Project

General

Profile

Actions

Feature #22250

closed

Add an only: keyword to Coverage.peek_result

Feature #22250: Add an only: keyword to Coverage.peek_result

Added by sferik (Erik Berlin) 18 days ago. Updated 15 days ago.

Status:
Feedback
Assignee:
-
Target version:
-
[ruby-core:126431]

Description

I am a maintainer of SimpleCov and working on adding a feature that attributes coverage to individual tests (see https://github.com/simplecov-ruby/simplecov/pull/1277). This feature works by work by diffing Coverage.peek_result snapshots around each test. That means thousands of peek_result calls per test run, but each call only needs the line counters, since per-test attribution is computed from line execution count deltas.

peek_result always builds the full result for every measured type. With branch and method coverage enabled, building the branch and method structures dominates the cost, so each snapshot is an order of magnitude more expensive than the lines the caller actually reads. On a real test suite measuring lines, branches, and methods, per-test tracking causes the test suite to run in ~30 seconds vs. ~3 seconds when it's disabled. Nearly all of this additional time is spent in peek_result, building results that are immediately discarded. Line-only filtering is significantly cheaper because the line result is a flat array dup, while branch and method results are rebuilt hashes.

Proposal

Allow callers to request only the types they need:

Coverage.start(lines: true, branches: true, methods: true)
Coverage.peek_result(only: :lines)
# => {"file.rb" => {lines: [1, 2, nil]}, ...}

Coverage.peek_result(only: [:lines, :branches])
# => {"file.rb" => {lines: [1, 2, nil], branches: {...}}, ...}

only: should accept one of :lines, :oneshot_lines, :branches, or :methods, or an Array of them. Each file's hash then contains only the requested keys, and the result construction for the other measured types is skipped entirely (the branch structure walk and the method table iteration never run). Requesting a type that is not being measured should raise a RuntimeError, consistent with the existing "coverage measurement is not enabled" error. An unknown type should raise an ArgumentError.

A patch with tests and documentation is at https://github.com/sferik/ruby-lang/tree/coverage-peek-result-only

The change is confined to ext/coverage/coverage.c. The existing coverage_peek_result_i iteration takes a filter mask (defaulting to current_mode, so the unfiltered path is byte-for-byte the current behavior), and Coverage.result reuses the same internal function with the full mask. I am happy to open a pull request if this direction is acceptable.

Updated by sferik (Erik Berlin) 18 days ago Actions #1

  • Tracker changed from Bug to Feature
  • Backport deleted (3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN)

Updated by mame (Yusuke Endoh) 15 days ago Actions #2

  • Status changed from Open to Closed

Applied in changeset git|d43737d2a8a66c4eaae5035695ee585ef8dc452c.


Cache a template of the branch coverage result [Bug #22250]

Coverage.peek_result rebuilt the nested branch coverage hash on every
call, and hashing its array keys (Array#hash via the recursion guard)
dominated the cost. Build { base_key => { target_key => counter_index } }
once per file and cache it; each peek dups it and fills in the counters.

40k branch sites: 40 ms -> 3.5 ms per peek. The key arrays are now frozen
and shared between results.

Co-Authored-By: Claude Fable 5

Updated by mame (Yusuke Endoh) 15 days ago Actions #3 [ruby-core:126457]

  • Status changed from Closed to Feedback

I have sped up peek_result: https://github.com/ruby/ruby/pull/18406 (already merged). It caches a template of the branch/method coverage result structures, and each peek_result just dups the template and fills in the counters.

I reproduced your setup by running rack's test suite (1242 tests) with your SimpleCov track_tests branch, measuring lines+branches+methods (on my machine):

  • no coverage measurement: 4.4 s
  • with per-test tracking (peek_result around every test): 27.6 s → 8.4 s with the change

peek_result still builds and returns the whole nested data structure, so it is not blazingly fast, but would this be acceptable for your use case?

As for the only: keyword itself, I'm not very positive: if you want per-test coverage but building the branch coverage result is too slow for that, measuring line coverage only seems reasonable, and I'm not sure it is worth a new API to support measuring per-test line coverage and whole-run branch coverage in a single run.

Rather, I think the fundamental solution would be an API that returns (or destructively updates) a flat array of counters, together with a method that tells what each counter means:

  • Coverage.peek_counters #=> an array of counters
  • Coverage.counter_info(idx) #=> [:line, lineno] or [:branch, base_location, target_location]

But I'm not sure whether this is the way to go.

Actions

Also available in: PDF Atom