Repository navigation
Release completed statement temporaries before exit-hook workloads - #35
Conversation
284km
left a comment
There was a problem hiding this comment.
Reviewed a4eb486 against 7ac8c4c. No actionable findings introduced by this change.
The reset sits after top-level execution and before shutdown callbacks. I checked fb_wm_stmt's allocation intervals and the surrounding root paths: the final value is already in gc_hold, while globals, captured environments, pending exceptions, registered callbacks, and suspended Fiber frames retain their existing roots.
Validation:
- Fresh O2 build using the CI-pinned Mere
991f299bcompiler and mgz revision. - The new regression fails on the
23ce853baseline with normal GC in both index modes, then matches Ruby 4.0.9 on this head with indexing enabled/disabled, both normally and withMERE_RUBY_GC_STRESS=100. - Focused exit probes covering normal completion, an unrescued exception with attached values,
exit(7), and top-level return matched Ruby's stdout and exit status under GC stress in both index modes. These probes also exercised nested callback captures, globals/class state, and a suspended Fiber resumed during shutdown. - Array, Hash, and object results printed by
--eval-printsurvived shutdown collections and matched the baseline in both index modes under GC stress. - Four existing corpus programs covering exit hooks, exceptions, collection inside methods, and suspended Fiber values matched Ruby in both index modes.
- Two default benchmark runs per revision retained 20,000 errors on
23ce853and 84 on this head. Twenty subsequent collections took 0.809–0.854 s before and 0.0220–0.0226 s after; stdout matched Ruby. Other validation ran concurrently, so these are local workload measurements. - CI for this exact head reports 510/510 corpus programs matching Ruby and a successful Linux build. Broader CI jobs remain pending. I did not repeat the full corpus, Linux runs, or Unicode-suite measurements locally.
git diff --checkpassed.
284km
left a comment
There was a problem hiding this comment.
Follow-up review of a4eb486: the head is unchanged from my previous review, which had no actionable findings. That assessment still stands.
The existing exit-root regression was also rerun successfully with indexing ON/OFF on #36's updated head, which includes this exact change. The earlier focused exception, captured-value, suspended-Fiber, and result-print validation remains applicable; unchanged measurements were not repeated.
At this check, 27 of 28 CI jobs for this head have passed, with the last CRuby shard still pending and no failed jobs. This remains a separate CI completion condition.
The driver carries the final top-level statement's temporary-allocation interval into exit hooks. A test runner invoked from
at_exitcan consequently keep a large set of discarded setup objects alive for its entire run. Ruby's Unicode case suite retained roughly 190,000 conversion exceptions, making each later collection expensive.Start a fresh statement interval once the top-level body has finished. The result is already explicitly rooted in
gc_hold; globals, captured values, pending exceptions, and registered handlers keep their existing roots. No GC thresholds, test inputs, or CI limits change.Validation:
test_UTF-8_upcase_asciibody changed from approximately 121 to 7.5 seconds, with all 40,569 assertions passing. Initialization is separate; these are local measurements, not a whole-CI speedup.CI for this head passed all 29 jobs. All 758 CRuby test files were collected from 8 shards with no missing files or infrastructure errors. This confirms complete measurement, not full Ruby conformance. The local integration of #32–#36 with main
f7ee263also passed all 515 corpus programs with hash indexing ON and OFF. The full Unicode suite completed within the unchanged limit with the same 68 baseline failure/error reports. The TCP regressions and native compaction-deferral checks also passed.