Repository navigation
Avoid copying unchanged string pages during collection - #36
Conversation
284km
left a comment
There was a problem hiding this comment.
Reviewed e925f72 against its stacked base a4eb486 (#35). Found one Major issue: the mutation bit is cleared even when the native Vec runtime defers compaction. Once the pin is released, an otherwise unchanged page no longer gets retried, retaining overwritten storage until a later write. Please address this before merging.
Separating mutation tracking from byte-accounting dirtiness is appropriate, and the Ruby value regressions passed. The finding concerns reclamation after a deferred native compaction; details and a suggested regression are inline.
Validation:
- Fresh O2 head build with the CI-pinned Mere
991f299bcompiler and mgz revision; compared with the existing O2 build of the exact #35 base. - The new regression matched Ruby 4.0.9 on both revisions with indexing ON/OFF, normally and with
MERE_RUBY_GC_STRESS=100(eight runs). - Five related corpus programs covering collections inside methods, suspended Fiber values, string-page reuse, collected handles, and exit roots matched Ruby on the head with indexing ON/OFF (ten runs).
- A controlled native contract test using the exact generated Vec runtime and a forced pin predicate confirmed that
vec_compactcan return without changing the arena. Applying the new flag-clearing logic retained 8,392,720 bytes after unpinning; an unconditional retry reduced the page to 4,096 bytes. This isolates the runtime contract rather than claiming a measured Ruby-level leak from the supplied corpus. - Two default benchmark runs per revision matched Ruby's stdout. Timed collections were 0.044–0.054 s on #35 and 0.0322–0.0324 s on the head. Other validation ran concurrently; these are local workload measurements.
- CI for this exact head reports 510/510 corpus programs matching Ruby and a successful Linux build. Broader jobs remain pending. I did not repeat the entire corpus, Linux validation, or Unicode measurements locally.
git diff --checkpassed.
284km
left a comment
There was a problem hiding this comment.
Re-reviewed 8c85dd5 against the previously reviewed e925f72, retaining #35 as the stacked base. The previous Major finding is resolved. No new actionable findings.
The page's mutation flag is now cleared only when the production helper observes a strict capacity decrease. Native pin deferrals and equal-capacity copies conservatively remain pending, so a later collection can retry without another page write. This is compatible with the pinned compiler's unit-returning compaction API; equal-capacity copies may still be retried, as documented.
Validation:
- Fresh O2 build with the pinned Mere
991f299bcompiler and mgz revision. unittest/string_page_compact_check.pypassed using the actual production helper and generated runtime: both native early-return paths preserve pending work, unpinning reclaims the overwritten 8 MiB down to 4 KiB, and the live value remains intact.- The string-page corpus regression matched Ruby 4.0.9 with indexing ON/OFF, normally and with
MERE_RUBY_GC_STRESS=100. - Related collected-handle and #35 exit-root regressions matched Ruby in both index modes.
- CI for this exact head reports 510/510 corpus programs matching Ruby and a successful Linux build. Broader jobs remain pending.
git diff --checkpassed.
The deferred-compaction finding can be considered addressed. Full corpus, Linux execution, Unicode-suite timings, and benchmarks were not repeated locally.
Repeated collections compact every live string page even when no string on it has changed. Track mutations separately from byte-accounting dirtiness and skip pages whose changed storage has already been reclaimed. Completely dead pages still return to the pool.
The compiler returns unit even when a native pin defers compaction. Clear the mutation flag only when the page capacity decreases after the call; deferred and equal-capacity compactions conservatively remain pending. Runtime retirement continues to reclaim old arenas globally.
Validation:
The full Unicode file completes in 431 seconds under the unchanged 600-second limit: 422 tests and 1,831,104 assertions. All 64 failures and 4 errors match the baseline reports exactly. The combined #32–#36 implementation on main
f7ee263passes all 515 corpus programs with hash indexing ON and OFF; its Unicode run completes in 423 seconds with identical baseline failure/error reports. New-head CI is in progress.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.