Skip to content

The string budget's gate reads the measured mark share, whatever SPINEL_GC_OBJ_BUDGET pins - #8460

Merged
matz merged 1 commit into
matz:masterfrom
namespaceMarcello:gc-str-gate-measured-share
Oct 11, 2026
Merged

matz merged 1 commit into
matz:masterfrom
namespaceMarcello:gc-str-gate-measured-share

Conversation

@namespaceMarcello

@namespaceMarcello namespaceMarcello commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

What this changes

A follow-up to #8444: the two review findings it was merged with, both real, and a correction to one sentence of its description.

The string budget's gate read what SPINEL_GC_OBJ_BUDGET made of the mark share, not the share. sp_str_retune gated the share of the object old generation on sp_gc_obj_alpha1024. That is the object budget's decision: obj sets it to 0, walk to 1, and under fixed it stays at its initial 1, because the object retune returned before measuring anything. So pinning the object budget pinned the string budget too:

run before after
test/gc_str_budget_objects.rb, SPINEL_GC_OBJ_BUDGET=obj 224 collections, old obj in str trigger 0.00 MB 44 collections, 5.80 MB, as the default
test/gc_str_budget_still.rb, SPINEL_GC_OBJ_BUDGET=walk 33 collections, as SPINEL_GC_STR_BUDGET=walk 106, as the default
the same, SPINEL_GC_OBJ_BUDGET=fixed SPINEL_GC_THRESHOLD_OBJ_KB=65536 17 collections 86 (17 with SPINEL_GC_STR_BUDGET=walk beside it)

The measurement has its own variable now (sp_gc_mark_share1024, static), written before those knobs are applied, and fixed leaves sp_gc_retune_object after the measurement instead of at the top. Each knob pins its own budget. With neither set the two values are the same number, so nothing else moves: the default, walk and str arms of both programs print the same [gc] lines as on master.

old obj in str trigger kept a share the trigger no longer carried. A string sweep that reclaims under a quarter doubles the budget as it stands and puts no share in, but the reading stayed at what the last productive sweep had left. It is zero after such a sweep now. test/gc_str_budget_kept.rb (new) goes through that branch: 2.80 MB before, 0.00 after. Its kept Strings are 3 KB each because Strings the slab takes never reached the branch here: kept alive in ten different ways, every sweep of them read as 40% reclaimed. I have not chased why.

gc-str-budget-test holds both: four more runs under SPINEL_GC_OBJ_BUDGET and the new program, read against str (the share was carried) and against zero (the reading was cleared). With lib/sp_alloc.c put back as #8444 left it, four of the new checks fail; of 17 mutations of the final code 16 are red, and the green one is the share at 1/4, which the leg leaves free on purpose. The other six collector legs pass.

A correction to #8444's description. It said that a scheduled string major skipped when it has nothing to free "would also take away the gc_str_budget_still cost". Measured since, it does not: with those majors skipped the program still peaks at 56 MB. The peak is one wide cycle at the change of phase, when the first string sweep after the heap is built reads the mark share of the last collection of the building phase; it is not the cycles after the full marks. Wide cycles among the string retunes of that program: 13 of 86 with the schedule as master has it, 6 of 237 with the scheduled majors skipped, 2 of 323 with full collections kept out of the gate altogether, and 56 MB each time against 26 MB under SPINEL_GC_STR_BUDGET=str. So that cost stands as the description gave it, and nothing I have tried takes it away.

Counts and megabytes above are from one laptop under Docker (gcc 14.2, linux-x86_64); none of them is a time.

make gate (on this branch merged with current master)

The gate ran on this branch as it stands, on master ad681d651 (the Gate: trailer). Master is at 00802fd5e as this opens, 34 commits later; none of them touches lib/sp_alloc.c or lib/sp_gc.c, and the branch merges cleanly. On that merged tree I ran again the three string-budget tests and the seven collector legs (gc-str-budget-test, gc-obj-budget-test, gc-str-major-test, gc-threshold-test, gc-phases-test, gc-stress-test, gc-minor-test), all passing. I did not run the whole gate a second time.

Tests: 6831 pass, 0 fail, 0 error
gate-test-shared: 6829 pass, 2 known failures, 0 new failures
scale-test: boxed Hash store work at 2x the methods is 1.95x (limit 2.20)
scale-test: boxed-receiver alias work at 2x the writes is 1.86x (limit 2.20)
scale-test: instance_eval forwarding work at 2x the wrappers is 1.71x (limit 2.50)
scale-test: work at 4x the program is 5.00x (linear 4.00, limit 5.20)
scale-test: work at 4x the program, compiled to C, is 6.27x (limit 6.90)
scale-test: call-shape work at 4x the units, compiled to C, is 4.15x (linear 4.00, limit 4.50)
gate: ALL GREEN
  • New tests have .expected files that match CRuby 4.0 run with --enable-frozen-string-literal
  • Values past 2^31 are marked # spinel: int64 (none: the new program stays under 2^31)
  • If optcarrot's generated C changed: callgrind numbers, checksum 59662 (the generated C does not change: the diff is in lib/)
  • Depends on: #

Written by Claude Code (AI assistant) on behalf of @namespaceMarcello, who directs this work.

Summary by CodeRabbit

  • Bug Fixes
    • String-budget behavior now uses the measured mark share independently of the object-budget setting. Unproductive string sweeps report a zero share.
  • Tests
    • Added coverage for object-budget modes and fixed-budget behavior, including checks that these settings do not alter the string budget’s gate.
    • Added a workload to verify collection counts and measured shares with retained strings.
  • Documentation
    • Clarified how string and object budgets use measured shares and operate independently.

…EL_GC_OBJ_BUDGET pins

Two review findings on matz#8444, both real.

The share of the object old generation was gated by sp_gc_obj_alpha1024,
and that is not the measurement: it is what SPINEL_GC_OBJ_BUDGET made of it
for the object budget. `obj` sets it to zero, so the default string budget
turned into `str`; `walk` sets it to one, and under `fixed` it stayed at its
initial one because sp_gc_retune_object returned before measuring anything,
so the default string budget turned into `walk`. On
test/gc_str_budget_objects.rb `obj` collected 224 times with no share, where
the default collects 44 times; on test/gc_str_budget_still.rb `walk` collected
33 times where the default collects 106, and with the object budget held at a
64 MB floor the default and `walk` both collected 17 times.

The measurement has its own variable now, written before those knobs are
applied, and `fixed` leaves the object retune after it instead of at the
top. Each knob pins its own budget. With neither set the two values are the
same number, so nothing else moves: the default, `walk` and `str` arms of
both programs report the same `[gc]` lines as before.

The other one is the reading. A string sweep that reclaims under a quarter
doubles the budget as it stands and puts no share in, but `old obj in str
trigger` kept the figure the last productive sweep had left. It is zero
after such a sweep now. test/gc_str_budget_kept.rb goes through that branch
(records first, then Strings past the slab's size, all kept): 2.80 MB
before, 0.00 after.

gc-str-budget-test holds both: four runs under SPINEL_GC_OBJ_BUDGET (`obj`
on the first program, `walk` and a held budget on the second, the last one
against `walk` under the same held budget), and the new program read against
`str` for the share and against zero for the reading. Put back as it was,
lib/sp_alloc.c fails four of the new checks; 17 mutations of the final code,
16 red, and the green one is the share at 1/4, which the leg leaves free on
purpose.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Gate: green tree 74292b1 master ad681d6 (linux-x86_64 gcc-14.2.0) tests 6831/0
@github-actions github-actions Bot added the gate: verified The head commit's Gate trailer names the tree its merge with master gives label Oct 11, 2026
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cf32c234-ffa8-4251-9b2a-25078f97bed6

📥 Commits

Reviewing files that changed from the base of the PR and between 00802fd and 51a3ee7.


📒 Files selected for processing (5)
  • Makefile
  • docs/thread.md
  • lib/sp_alloc.c
  • test/gc_str_budget_kept.rb
  • test/gc_str_budget_kept.rb.expected

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.



📝 Walkthrough

Walkthrough

The GC now retains the measured mark share separately from the object-budget mode. The string budget uses that share, and an unproductive sweep resets the reported object-share bytes. New workload and Makefile checks cover these cases.

Changes

GC string budget

Layer / File(s) Summary
Retain measured mark share
lib/sp_alloc.c
The object retune records the measured mark share before applying object-budget settings. A fixed object budget no longer skips that measurement.
Apply measured share to string budget
lib/sp_alloc.c, docs/thread.md
The string-budget calculation uses the measured share regardless of the object-budget mode. An unproductive sweep resets the reported object-share bytes to zero. The documentation describes these behaviors.
Exercise budget modes and kept strings
test/gc_str_budget_kept.rb, test/gc_str_budget_kept.rb.expected, Makefile
The new workload retains 8,000 strings of 3,000 characters. Makefile checks compare collection counts across budget modes and verify that the final measured share is zero.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix


Merge Risk: ⚪ Minimal · up to 51a3e

The string-budget gate now uses the measured mark share independently of the object-budget setting, and unproductive sweeps report zero share. Added regression cases cover the relevant budget modes and retained strings; no concrete blocker remains before normal checks.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (3 skipped: 3… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and specifically describes the main change: the string-budget gate now uses the measured mark share independently of object-budget pinning.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (3 skipped: 3 unsupported.)


  • Fix all pre-merge checks with AI
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@matz
matz merged commit 0fc287c into matz:master Oct 11, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gate: verified The head commit's Gate trailer names the tree its merge with master gives

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants