Skip to content

fix: memoise key parsing in RubyDataParser to fix combiner performance regression#1239

Open
oleksii-leonov wants to merge 1 commit into
simplecov-ruby:mainfrom
syngenta:perf/memoize-coverage-key-parsing
Open

fix: memoise key parsing in RubyDataParser to fix combiner performance regression#1239
oleksii-leonov wants to merge 1 commit into
simplecov-ruby:mainfrom
syngenta:perf/memoize-coverage-key-parsing

Conversation

@oleksii-leonov

@oleksii-leonov oleksii-leonov commented Jul 22, 2026

Copy link
Copy Markdown

Measured on a real 66-worker CI merge (~21k files, ~46k branches, branch coverage enabled): the collate step (which merged 66 reports) regressed from ~29s under 0.22 to ~145s (5x slower) under 1.0.0.

A wall-clock stackprof attributes main time to Ripper#parse under RubyDataParser.parse_array_string.

Resultset JSON stores keys as stringified Ruby literals ("[:if, 0, 14, 4, 14, 30]"), and RubyDataParser walks them back into arrays with a Ripper parse. That parse sat on a hot path: BranchesCombiner and MethodsCombiner derive a merge identity from every key of both sides on every pairwise merge. Folding N resultsets therefore parsed each key string N-1 times.

This PR adds memoisation to RubyDataParser.call to avoid parsing strings that have already been parsed.

Notes:

  • The set of unique keys is bounded by the project's branch and method count, so the cache stays reasonably small and needs no eviction.
  • Cached arrays are frozen and shared. Every caller (both combiners, BranchBuilder, MethodBuilder, SourceFile::Method) destructures the parsed array without mutating it; freezing enforces that any future mutation surfaces as an error instead of silent cross-caller corruption.
  • Array inputs still pass through untouched and unfrozen (in-process results never hit the cache, exactly as before).

…e regression

Measured on a real 66-worker CI merge (~21k files, ~46k branches, branch coverage enabled): the collate step (which merged 66 reports) regressed from ~29s under `0.22` to ~145s (5x slower) under `1.0.0`.

A wall-clock stackprof attributes main time to `Ripper#parse` under `RubyDataParser.parse_array_string`.

Resultset JSON stores keys as stringified Ruby literals (`"[:if, 0, 14, 4, 14, 30]"`), and RubyDataParser walks them back into arrays with a Ripper parse. That parse sat on a hot path: `BranchesCombiner` and `MethodsCombiner` derive a merge identity from every key of *both* sides on *every* pairwise merge. Folding N resultsets therefore parsed each key string N-1 times.

This PR adds memoisation to `RubyDataParser.call` to avoid parsing strings that have already been parsed.

Notes:

- The set of unique keys is bounded by the project's branch and method count, so the cache stays reasonably small and needs no eviction.
- Cached arrays are frozen and shared. Every caller (both combiners, `BranchBuilder`, `MethodBuilder`, `SourceFile::Method`) destructures the parsed array without mutating it; freezing enforces that any future mutation surfaces as an error instead of silent cross-caller corruption.
- Array inputs still pass through untouched and unfrozen - in-process results never hit the cache, exactly as before.
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.

1 participant