perf(imaging): append to the cache outside the global lock, not inside it - #409
Merged
Conversation
This was referenced Jul 31, 2026
Merged
sebasmos
added a commit
that referenced
this pull request
Aug 4, 2026
…he-lock perf(imaging): append to the cache outside the global lock, not inside it
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For @MohShahin, who is seeing 1.7 calls/min against the 14-24/min the same arms managed when healthy.
The bottleneck
Every imaging runner did this:
All four workers serialise behind that append. On synced or network storage the write can block for seconds, so throughput caps at roughly one call per append no matter what
max_workerssays. That matches the symptom: a rate an order of magnitude below healthy, varying over time rather than steadily, on a lane where an earlier cache corruption was already traced to OneDrive.The dict update needs the lock. A single short line appended in
"a"mode does not.What this does not change
10 runners changed, mechanically and identically. 1145 passed, 7 skipped,
ruffclean.And it is not the whole answer
Putting the cache on genuinely local disk is still the first thing to try, and it needs no code. This PR removes the amplifier; the storage is the cause.
Two other things worth checking before spending more hours, neither of which this PR touches:
RetryBackend(..., tries=5, backoff=3.0)burns 3+6+12+24s on a throttled call, which alone floors four workers near 5/min. The signature is long stretches where the cache does not grow at all.Bigger lever than any of this
soloneeds ~4170 calls and is 34% done. The judge cell in #393 needsreferee_300/imaging_cascade.jsonl, which is 417 calls. Running that arm first gets the blocked cell in a tenth of the work.