Skip to content

perf: resolve RSpec test file paths without allocating per example#344

Open
connorshea wants to merge 1 commit into
KnapsackPro:mainfrom
connorshea:perf/rspec-file-path-resolution
Open

perf: resolve RSpec test file paths without allocating per example#344
connorshea wants to merge 1 commit into
KnapsackPro:mainfrom
connorshea:perf/rspec-file-path-resolution

Conversation

@connorshea

Copy link
Copy Markdown

Description

AI Disclosure: This was generated using Claude Code with Opus 5. It has been reviewed and tested by me manually.

RSpecAdapter.file_path_for runs on every test example (in Queue Mode, it runs twice per example) and was allocating more objects than necessary to check candidate paths. We can instead check it inline while still keeping the laziness.

We built a MatchData and a few other things for every call of parse_file_path and id_path?. We don't need to do that here, because we can instead look for the last [ in the path and then match on the string from there onward, without needing to allocate almost anything.

parse_file_path now finds [ with rindex and validates it with an allocation-free Regexp#match? starting at that offset, and id_path? is now an end_with? + match?.

The only functional change in this code is that a path with a linebreak (which shouldn't happen anyway) would previously cause a NoMethodError, and is now ignored/kept instead. I don't think that's a problem because it doesn't ever happen, but figured I'd note it.

Also wanted to note that I had Claude fuzz the new regex to ensure equivalent behavior on 250k values (some generated to match real paths, some fully random values).


This saves a tiny bit of time, but the main benefit of this change is reducing the # of object allocations because it helps avoid triggering GC in large suites, which speeds things up. This isn't going to be hugely significant on its own, but I think it's worth doing.

Per-call, Ruby 3.4.9, arm64-darwin25, 500 RSpec example ids, benchmark-ips (3s, 1s warmup)

Method Objects before Objects after µs before µs after Speedup
file_path_for 13 1 0.926 0.206 4.5x
parse_file_path (id path) 6 1 0.531 0.153 3.5x
parse_file_path (plain path) 5 0 0.493 0.040 12.4x
id_path? (id path) 6 0 0.541 0.089 6.1x
id_path? (plain path) 5 0 0.511 0.038 13.5x

Scaled to a suite (one file_path_for per example, NOT Queue Mode)

Examples Time before Time after Objects before Objects after
10,000 9.1 ms 2.0 ms 130,007 10,007
100,000 94.6 ms 20.6 ms 1,300,000 100,000

Scaled to a suite (Queue Mode: two file_path_for calls per example)

Examples Time before Time after Objects before Objects after
10,000 18.2 ms 4.0 ms 260,014 20,014
100,000 189.2 ms 41.2 ms 2,600,000 200,000

Checks

  • I added the changes to the UNRELEASED section of the CHANGELOG.md, including the needed bump (i.e., patch, minor, major)
  • I followed the architecture outlined below for RSpec in Queue Mode:
    • Pure: lib/knapsack_pro/pure/queue/rspec_pure.rb contains pure functions that are unit tested.
    • Extension: lib/knapsack_pro/extensions/rspec_extension.rb encapsulates calls to RSpec internals and is integration and E2E tested.
    • Runner: lib/knapsack_pro/runners/queue/rspec_runner.rb invokes the pure code and the extension to produce side effects, which are integration and E2E tested.

`RSpecAdapter.file_path_for` runs for every test example and allocated an
array of four lambdas on every call, purely to check its candidate paths
lazily. Checking them inline keeps the same order and the same laziness.

`parse_file_path` and `id_path?` built a MatchData for every path - plus,
because the target string is unfrozen, a frozen copy of it, and an array
from `captures`. An id path always ends with `]` and the id characters
can never contain `[`, so the only possible split point is the last `[`
of the path. `parse_file_path` now finds it with `rindex` and validates
it with an allocation-free `Regexp#match?` starting at that offset;
`id_path?` is a plain `match?`.

Verified equivalent to the old regexp on 550 hand-picked paths and 50k
fuzzed inputs over the interesting alphabet, including paths whose file
name itself contains brackets and trailing brackets that are not ids.
Those cases are now covered by specs.

  file_path_for    13 allocations / 2.1 us  -> 1 / 0.46 us
  parse_file_path   6 allocations / 0.97 us -> 0 / 0.10 us (no id)
  id_path?          6 allocations / 1.05 us -> 0 / 0.09 us (no id)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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