Repository navigation
A walk following a call finds its callee's values without scanning the program - #7404
Conversation
…e program widen_boxed_array_sources and array_src_walk follow a call into the value of the method it binds to: method_value_leaves collects the method's returns, and method_has_other_body asks whether a subclass or a second definition can answer instead. The first scanned every ReturnNode of the program and the second every scope, once per call the walk follows. On a large machine-generated program (289k lines, about 3,000 returns and 2,000 methods) the walks follow hundreds of thousands of calls per round, and these two scans were most of their time. The returns now come from a chain per scope (comp_sret_first, beside the CallNode chain comp_scall_first), built in node order, so the values and their order, and the cut at the caller's capacity, are the ones the scan gave. method_has_other_body remembers its answer per scope while scope shape is fixed (the scope-index epoch and the scope and class counts). Both fall back to the scans while the scope index is not frozen. The generated C is unchanged.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 11 minutes. View limit detailsLimit details: You’ve used all 8 included reviews currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Counts on the 289k-line program the issue describes, to add to the description.
|
2 participants
Fixes #7403
What changes
method_value_leavestakes a method'sreturns from a chain per scope,comp_sret_first/comp_sret_nextinsrc/compiler.c, the same shape as the existing CallNode chaincomp_scall_first. The chain is built in node order and rebuilt when the node table's version or the scope count moves. It is used only while the scope index is frozen (the inference fixpoint and after), since before that a node's scope can still move; otherwise the scan runs as before. The returns, their order and the cut at the caller's capacity are the ones the scan gave.method_has_other_bodyremembers its answer per scope while the scope index is frozen, keyed by the scope-index epoch and the scope and class counts (the inputs of its scan: the scopes' names, kinds and classes, and the class hierarchy).A similar per-scope grouping of
ReturnNodes is added insidepromote_shared_stored_stringsby #7386 (an_returns_by_scope, rebuilt per run); the two could be merged later.Why the results do not change
spinel -cwas run on master d38099f and on this change (based on it) from the same worktree, with the same input paths, in both overflow modes, on everytest/*.rb(5,791 files × raise/promote = 11,582 compilations): the generated C is identical except in the 4 tests that embed the compiler's own revision inRUBY_DESCRIPTION(frozen_chilled_builtin_strings,object_scoped_ruby_constants,ruby_description_shape,symbol_id2name_ruby_desc_minmax, both modes), and the refusals are the same.Before / after
spinel -c --int-overflow=promoteon master ab9b925 and on this change, one after the other, counting the calls of the two helpers and the iterations of their scans (a local counter, not part of this change):ReturnNodes visitedEach iteration of the scans is cheap, so at these sizes the time is within noise. The scans grow with (calls followed × program size), and on a 289k-line program (about 3,000 returns and 2,000 methods) they were 918 of the 2,300 samples that
widen_boxed_array_sourcestook at 180 s into the compilation (with #7377, #7378 and #7384), the walk itself being about 18% of all samples at that point.(Timings are from a shared machine in low-power mode; the two runs of each program were taken back to back.)
Testing
make gate:The stamp line above names the
origin/masterthe clone had fetched when the gate finished (ab9b925); the tree the gate ran was based on 92510d6, as described below.SPINEL_INT_OVERFLOW=promote make test: 5909 pass, 15 fail, 13 error. Every failure is one master e5e8f79 already had; none is new.The gate ran on a branch based on 92510d6 that combines this change with the other compile-time fixes from the same investigation (#7377, #7378, #7384, #7375, #7386, #7388, #7390, #7392, #7396, #7398, #7400, #7402). This branch was then rebased onto ab9b925 without conflicts.