Skip to content

perf(autoloader): optimize package list accumulation in loop#10341

Open
gr8man wants to merge 1 commit into
codeigniter4:developfrom
gr8man:perf/optimize-autoloader
Open

perf(autoloader): optimize package list accumulation in loop#10341
gr8man wants to merge 1 commit into
codeigniter4:developfrom
gr8man:perf/optimize-autoloader

Conversation

@gr8man

@gr8man gr8man commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Description
In Autoloader::loadComposerNamespaces(), array_merge() was being used inside a foreach loop to accumulate composer packages into $packageList:

foreach ($allData as $list) {
    $packageList = array_merge($packageList, $list['versions']);
}

Complexity Analysis:

  • Before (array_merge inside the loop):
    On each iteration $i$, array_merge allocates a completely new array and copies all existing elements from $packageList (which has size $i \times S$, where $S$ is the size of each package version list) plus the new ones.

    • Time Complexity: $\sum_{i=1}^{K} O(i \times S) = O(K^2 \cdot S) = O(N^2)$ (quadratic) where $N$ is the total number of packages across all lists.
    • Memory Complexity: $O(N)$ allocations and copies at each step, generating excessive garbage collection overhead.
  • After (Nested foreach loop with direct assignment):

    foreach ($allData as $list) {
        foreach ($list['versions'] as $packageName => $data) {
            $packageList[$packageName] = $data;
        }
    }

    The nested loop directly assigns elements to $packageList one by one. This preserves the exact behavior of array_merge() where duplicate string keys (package names) in subsequent array elements overwrite previous ones, while completely avoiding intermediate array copying.

    • Time Complexity: $\sum_{i=1}^{K} O(S) = O(K \cdot S) = O(N)$ (linear).
    • Memory Complexity: $O(1)$ additional overhead per element, performing in-place addition/overwrite of keys.

This PR shifts the package list building process from quadratic time $O(N^2)$ to linear time $O(N)$ while maintaining 100% behavioral compatibility with the original array_merge logic.

Additionally, a unit test was added to verify the package list auto-discovery flow inside loadComposerNamespaces.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide
  • We don't add entries for refactoring, as there are no real changes for end users. (No changelog entry needed)

@memleakd

Copy link
Copy Markdown
Contributor

Thanks for the PR. I have to say I respect the ambition here. Not every foreach gets a TED Talk.

The implementation itself looks fine to me, but I think the performance framing is doing more work than the code change needs. I'd trim the PR body quite a bit and frame this as a small internal cleanup.

I'd also consider dropping the new test. It reads a bit like assertComposerStillComposers(), while existing Composer discovery tests already cover that observable behavior.

@michalsn

Copy link
Copy Markdown
Member

Thanks, but I don't think this optimization is justified.

InstalledVersions::getAllRawData() typically returns a single dataset and returns more only when multiple independent Composer autoloaders are registered in the same process. Therefore, array_merge() is normally called only once and is likely faster than a PHP-level nested loop.

A benchmark showing a real-world improvement would be needed to justify this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants