Skip to content

Keep coverage source positions off AST nodes when Coverage isn't in use - #9795

Merged
headius merged 1 commit into
jruby:masterfrom
sferik:coverage-node-positions
Oct 15, 2026
Merged

headius merged 1 commit into
jruby:masterfrom
sferik:coverage-node-positions

Conversation

@sferik

@sferik sferik commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

This is the follow-up @enebo described in #9751 (comment). The branch and method coverage work added a source span to every node, plus a few extra positions on conditionals, loops, when/in clauses and safe-navigation calls. All of that lived in fields, so every node got 16 bytes bigger and an IfNode went from 32 to 80, whether or not anyone had called Coverage.start.

With this change a node holds a single reference to a record with its positions, and the parser only fills it in when Coverage needs it. Every other parse leaves it null.

Sizes in bytes per instance (compressed oops), measured from field offsets:

Class 10.1.2.0 master this PR
Node 24 40 24
CallNode 40 72 40
FCallNode 32 56 40
IfNode 32 80 40
WhileNode 32 72 32
WhenNode 32 64 40
IterNode 40 56 40
LambdaNode 40 56 40
DefnNode 40 64 48
LocalVarNode 32 48 32
FixnumNode 32 48 32
SymbolNode 24 48 32

The reference is a compressed pointer, so it costs 4 bytes, and objects are aligned to 8. A class either had the padding to take it or grew by 8: of the 128 node classes, half are the size they were in 10.1.2.0 and half are 8 bytes over.

Node also keeps two flags the coverage work added (a backtrace-only newline, and parenthesized) in one byte now instead of a boolean each. That leaves the last byte of its 24 free, and the JVM puts a subclass's own flag there. This brought nine classes back to their 10.1.2.0 size, CallNode, AttrAssignNode, ArgumentNode and WhileNode among them.

Cc: @headius @enebo

@sferik
sferik force-pushed the coverage-node-positions branch from 62e4507 to 89efeb1 Compare October 10, 2026 10:25
@headius

headius commented Oct 10, 2026

Copy link
Copy Markdown
Member

Yeah good stuff, this is the result that we wanted.

The agent is a bit wrong here in that the reference will be a "compressed pointer" on all modern JVMs, so 4 bytes instead of 8. Worth us doing another audit of all nodes to find more shrinking opportunities, but adding 4 bytes across the board to enable full coverage support is pretty manageable.

I'll review and merge.

Branch and method coverage report where things are in the source, so
the parser records a span on every node, and more on some: where the
else of a conditional is, the parentheses around an expression, the
statement of a modifier loop. All of it sat in fields of the nodes,
which made every node 16 bytes larger, and a conditional or a loop up
to 40, whether or not Coverage was in use. An AST is large in almost
any application and much of it lives as long as the code does.

As suggested in
jruby#9751 (comment),
a node now refers to a record holding its positions, and the parser
only records them when Coverage has a use for them. Any other parse
leaves that one reference null.

- NodeSpan holds a node's span and its parentheses. Each production
  handing a node along widens its span, which replaces the record.
  Parsing CRuby 4.0's standard library with positions recorded takes
  about 5% more CPU time that way than with a mutable holder (0.200s
  against 0.191s, the median of seven interleaved rounds, and 0.185s
  with no positions recorded).
- What a class of node keeps besides its span goes in a record of its
  own inside the NodeSpan, so it takes no field of the node: a
  SourcePosition for the else of a when or in clause, the end of a
  safe navigation and the arrow of a lambda, a SourceSpan for the
  body of a modifier loop, and a Branch for a conditional.
- Branch coverage needs all of these. Method coverage only needs to
  know where definitions and blocks start and end, so that is all a
  parse records for it.
- What decides the code that is compiled stays on the node, as flags:
  that an expression is parenthesized, that a conditional is a
  modifier, and what MRI folds of a literal condition. The first
  shares a byte with the flag for a newline that is only there for
  backtraces, rather than each being a boolean. That leaves the last
  byte of a node's own 24 free, and the JVM puts a flag of the
  node's class there, such as whether a call is lazy, instead of
  starting another 8 bytes for it.
- A block and a method definition have their end line in a field of
  their own again, since every parse needs it.

A parse decides what to record when it starts. If branch coverage
starts before it ends, the file is measured with what was recorded,
and so reports no branches.

Instance sizes in bytes (compressed oops), before the coverage work,
on the branch this one is based on, and now:

    Node            24 -> 40 -> 24
    CallNode        40 -> 72 -> 40
    FCallNode       32 -> 56 -> 40
    IfNode          32 -> 80 -> 40
    WhileNode       32 -> 72 -> 32
    WhenNode        32 -> 64 -> 40
    IterNode        40 -> 56 -> 40
    LambdaNode      40 -> 56 -> 40
    DefnNode        40 -> 64 -> 48
    LocalVarNode    32 -> 48 -> 32
    FixnumNode      32 -> 48 -> 32
    SymbolNode      24 -> 48 -> 32

The reference takes 4 bytes, being a compressed pointer, and objects
are aligned to 8. So a class either had the padding to take it or
grew by 8: of the 128 classes of node, 64 are the size they were
before coverage and 64 are 8 bytes over.

With positions recorded, the branches and lines Coverage reports for
the 727 files of CRuby 4.0's standard library are what they were, and
Coverage.line_stub gives the same result for each of them whether or
not positions are recorded. Tests check which nodes carry positions
in each coverage mode, so that a parse without Coverage keeps none.
@sferik
sferik force-pushed the coverage-node-positions branch from 89efeb1 to b8e6de8 Compare October 11, 2026 13:34
@sferik

sferik commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor Author

@headius I’ve updated the commit message and PR description to say the reference is a 4-byte compressed pointer. That also showed a little more room: packing two of Node’s flags into one byte brings nine classes, including CallNode, back to their 10.1.2.0 size. Now half of the 128 node classes match 10.1.2.0 and half are 8 bytes over.

@headius headius added this to the JRuby 10.1.3.0 milestone Oct 15, 2026
@headius
headius merged commit 3fe18b0 into jruby:master Oct 15, 2026
135 checks passed
@headius

headius commented Oct 15, 2026

Copy link
Copy Markdown
Member

Much cleaner, thanks!

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.

2 participants