Skip to content

Sort the result cache's package dependencies before writing them - #6370

Open
SanderMuller wants to merge 1 commit into
phpstan:2.2.xfrom
SanderMuller:result-cache-deterministic
Open

Sort the result cache's package dependencies before writing them#6370
SanderMuller wants to merge 1 commit into
phpstan:2.2.xfrom
SanderMuller:result-cache-deterministic

Conversation

@SanderMuller

@SanderMuller SanderMuller commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Two identical runs of the same project produce result caches of the same size that differ in 466436 bytes out of 75.3 MB. Every differing byte is inside packageDependencies, which is the one section save() writes without ordering it first, so its order follows the order the workers happened to finish in.

ksort($errors);
ksort($locallyIgnoredErrors);
ksort($linesToIgnore);
ksort($unmatchedLineIgnores);
ksort($collectedData);
ksort($invertedDependencies);   // + sort() on each dependentFiles list
ksort($exportedNodes);
// packageDependencies: written as it came

With the sort, the two runs differ in 2 bytes: two digits of the lastFullAnalysisTime integer, which is meant to differ. That is also what identifies the cause rather than assuming it, since nothing else moved.

Why it is worth having

A run reading the file back does not care. Two things do, both measured on two cold runs of a 4524-file project.

Diffing two caches to find out why one was not reused, which is the pro-tip in @staabm's 2023 result cache post. Plain diff refuses the framed format that ships in 2.2.13 (Binary files ... differ), so this is diff -a:

diff -a lines sha256 that skips the lastFullAnalysisTime frame
2.2.x 3932 different
this PR 2 equal

And content hashing, which is the part a tool cannot retrofit cheaply: without the sort the file cannot be content-addressed at all, with it a consumer reads that one frame's header for its length, skips it and hashes the rest.

What is not a reason, since I measured it: deduplicating a cache in an incremental store. The ordering churn is localised, 129 of 18381 4K blocks on 2.2.x against 1 with the sort, so about half a megabyte per run out of 75 MB. Not worth code on its own.

Cost, measured on the real data shape from that project (4524 files, 2792 package names): median 0.454 ms per save as the array comes out of the run, 0.861 ms if it is shuffled first as a worst case. That project's cold run is about 30 s.

Fails-before / passes-after

On one build with only the sort removed: 466436 differing bytes against 2, 3932 diff -a lines against 2, and the timestamp-skipping hash goes from different to equal.

e2e/result-cache-deterministic-order covers it in CI. Four files are split across two parallel jobs, so the section reaches the main process interleaved and no worker completion order can put it in ascending order by accident, and each file declares its two packages in reverse alphabetical order. assert-sorted.php reports every entry section that is out of key order and every file whose package list is out of order.

Both halves of the change are needed and both are covered: removing the whole hunk fails on the key order and on all four package lists, removing only sort($packages) fails on the four package lists. The package-list half does not depend on how the scheduler composes jobs, so it still fails without the sort even when the whole project runs as a single job.

The naive test would have been coverage in name only: on a fixture-sized project two runs are already byte-identical without the fix, so "the two files match" passes on 2.2.x. What distinguishes the two builds at that scale is the ordering itself, which is what the fixture asserts.

Suite green, self-analysis clean, phpcs clean.

Every other section of the file is ordered before it is written; this one
followed the order the workers happened to finish in, so two identical runs
of a 4524-file project produced caches of the same size differing in 9848
lines. A run reading the file back does not care, but it means a result
cache cannot be hashed, deduplicated or compared between machines.

The packages of a single file need the same treatment: they are collected in
the order that file's dependencies are reflected, so a file naming Mailer
before Logger records their packages in that order.

The e2e fixture makes both observable without depending on a race. Four
files are split into two parallel jobs, so the section reaches the main
process interleaved and no worker completion order can put it in ascending
order by accident; and each file declares the two packages in reverse
alphabetical order. Dropping either sort fails the assertion on every run,
naming what is out of order.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@SanderMuller
SanderMuller force-pushed the result-cache-deterministic branch from fb89904 to 2252ebd Compare September 4, 2026 21:22
@staabm

staabm commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

I am not sure its worth sorting the results or whether a diff/debug-tool should do it before comparing

@SanderMuller

Copy link
Copy Markdown
Contributor Author

Good question, and it made me go and check which of the reasons actually survive. One of the three I had in mind does not, so here they are with numbers. Two cold runs of the same 4524-file project, 75.3 MB cache, same size every time.

Diffing, which is your point. You are right that a tool could canonicalise first. Worth knowing that plain diff already refuses the framed format from 2.2.13 (Binary files ... differ, about 25k NUL bytes per MB), so this is diff -a territory either way:

diff -a run1 run2 | grep -c '^[<>]'
2.2.x        3932
with sort       2

That is Ondřej's pro-tip from your 2023 post, diffing the cache to find out why it was not reused. It works today, in any shell, without a tool existing. A tool that sorts first would work too, it just has to be written first and then parse every frame and know which sections are order-insensitive.

Content hashing, which a tool cannot retrofit cheaply. Without the sort you cannot content-address the file at all. With it, two runs are identical apart from lastFullAnalysisTime, so a consumer reads that frame's header for its length, skips it and hashes the rest. Ten lines, no further knowledge of the format. I checked both directions: skipping the timestamp, 2.2.x hashes differ and the sorted ones are equal.

Deduplication, which I had in the description and which does not hold up. I measured what an incremental store or transfer would actually rewrite between two runs, and the ordering churn is localised:

              4K blocks   64K blocks
2.2.x         129/18381    10/1149     ~0.5 MB of 75 MB
with sort       1/18381     1/1149     ~0.1 MB

So about half a megabyte per run. Not worth carrying code for, and I will drop that claim from the description.

Cost of the sort, on the real array from that project (4524 files, 2792 package names): median 0.454 ms as it comes out of the run, 0.861 ms if I shuffle it first as a worst case, against a cold run of about 30 s.

So the case is the diff line count and the hashability, not storage. If neither is worth 0.5 ms to you and Ondřej, I am happy to close it, that is a fair reading of the same numbers.

Separately, the description said there was no e2e fixture, which is out of date since I pushed one. Updating it now.

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.

2 participants