Improve collate performance - #1244
Merged
sferik merged 3 commits intoJul 30, 2026
Merged
Conversation
Measured with `ruby benchmarks/collate.rb` (160 resultsets, 1836 files,
147,875 lines, 8,205 branch conditions, branch coverage enabled):
phase before after
merge 8.44s 5.54s -34.4%
total 9.21s 6.27s -32.0%
`BranchesCombiner.combine` dominated the merge either way, but its share
of it fell from 59% to 43%.
Collating N resultsets folds them pairwise, and both branch and method
combining derive a merge identity from every key of *both* sides on every
one of the N-1 merges. The accumulator's keys therefore had an identity
derived for them N-1 times, always to the same answer. `RubyDataParser`
already memoises the string *parse*, but each call still allocated a fresh
location array and then hashed it as a `Hash` key, and `Array#hash` is not
memoised.
Every fold also rebuilt more than it needed to: `combine` stored a
replacement `[tuple, count]` pair for every key it had just merged, and
`merge_branches` allocated a new pair instead of updating the existing one.
So:
- Memoise the derived identity, keyed by the raw key, so a hit skips the
parse cache lookup as well as the derivation.
- Update the accumulated pairs in place and drop the redundant write-back.
Notes:
- The cache is bounded by the project's branch and method count, the same
argument that lets `RubyDataParser`'s parse cache live without eviction.
- Cached identities are frozen. They are shared across calls now, and every
caller only ever uses them as a hash key.
- `tuple_identity` and `source_identity` keep their signatures but are no
longer the memoised entry point - they are the plain derivation the cache
calls on a miss.
- Pairs are only mutated where `combine` built them itself, so nothing a
caller still holds can be reached. `LinesCombiner`'s deliberately
non-mutating contract is untouched.
- Both the string and array forms of a key still resolve to equal
identities, so a resultset that has been through JSON merges with an
in-process one exactly as before.
- Verified no behaviour change: merging all 160 resultsets with branch and
method coverage enabled produces a byte-identical result before and
after, in insertion order and canonically sorted alike.
Follows on from memoising the identities themselves. Measured with
`ruby benchmarks/collate.rb` (160 resultsets, 1836 files, 147,875 lines,
8,205 branch conditions, branch coverage enabled):
phase before after
merge 5.54s 4.47s -19.3%
total 6.27s 5.41s -13.7%
Cumulatively the merge is now 4.47s against 8.44s before either change,
-47%.
An identity exists only to be a key of `combine`'s working hash, and
`Array#hash` is not memoised, so keying on the five-element location tuple
rehashed all five elements on every lookup - for every key of both sides of
every one of the N-1 merges. Interning each distinct identity to a
sequential Integer makes those lookups hash an immediate instead.
Notes:
- Two keys are given the same id exactly when their identities compare
equal, so this is transparent to merging: differing branch ids still
collapse (issue simplecov-ruby#1233), and the string and array forms of one key still
merge together.
- The ids are an internal detail. They are never exposed - `combine`
returns `merged.values`, so the interned keys are dropped on the way out
and the retained tuple is still the first one seen.
- `tuple_identity` and `source_identity` are unchanged and still return the
location tuple; interning happens between them and the working hash.
- Adds a second bounded table per combiner, sized by the count of distinct
identities rather than of distinct keys.
- Verified no behaviour change: merging all 160 resultsets with branch and
method coverage enabled produces a byte-identical result before and
after, in insertion order and canonically sorted alike.
sferik
added a commit
that referenced
this pull request
Jul 30, 2026
The combiner specs covered array-with-array and string-with-string merges but not the mixed case, where an in-process result merges with a resultset read back from JSON and the same key arrives in both forms. That case is exactly the invariant the interning layer introduced in #1244 has to preserve (two keys share an id exactly when their identities compare equal), and a regression here would silently double-count branches and methods. Pin it with one spec per combiner.
sferik
added a commit
that referenced
this pull request
Jul 30, 2026
BranchesCombiner and MethodsCombiner carried identical copies of the interning cache introduced in #1244, differing only in which identity derivation they call. Move the cache construction into a small factory that takes the derivation as a block. Call sites are unchanged and each combiner keeps its own independent cache and id table, since the factory builds fresh ones per call. No behavior change. The combine specs, including the new mixed-form ones, pass, and a run of the collate benchmark against the same fixture produces byte-identical output at unchanged speed.
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
2 participants
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.
After upgrading from 0.22 to 1.0.2 our 231k line rails coverage step on CI, split across 160 files from 160 test nodes, increased significantly from ~90sec to nearly 10 minutes. To combat this we implemented parallel collate actions with
Process.fork(subsequent PR if helpful), bring runtime down to 2m 16s as a workaround allowing us to merge the upgrade and gather real data.Looking at the performance of
SimpleCov.collatethere are some easier performance wins.This AI-assisted PR does 3 things broken out by commit:
SimpleCov.collateperformance in runtime and memory consumptionTests of these commits on our codebase reduces parallel collate from 2m 16s => 26s with identical outputs.
I'm happy to split these commits into separate PRs or add more metrics to test their impact.