boehmgc: Batch GC_generic_malloc_many() block allocation#547
Merged
Conversation
Add a patch to boehmgc that makes GC_generic_malloc_many() hand out up to GC_many_blocks heap blocks worth of objects per acquisition of the global GC allocation lock, instead of exactly one 4 KiB block. All evaluator threads replenish their thread-local free lists (Values, Envs, Bindings) through this function, so during parallel evaluation they otherwise serialize on the allocation lock: at 12 eval cores, batching 64 blocks reduces futex syscalls by 6x (3.58M to 0.58M) for 'nix search nixpkgs --no-eval-cache fizzbuzz', and reduces both kernel time and elapsed time at higher core counts. The batch size defaults to GC_MANY_BLOCKS_DEFAULT (set to 64 here) and can be overridden at runtime via the GC_MALLOC_MANY_BLOCKS environment variable (1-64, where 1 restores the upstream behavior). Assisted-by: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds a boehmgc patch that batches ChangesGC Allocation Batching
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packaging/patches/boehmgc-batch-malloc-many.patch (1)
37-37: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winApply the same batch-size clamp before splitting existing free lists.
The new-block path clamps
GC_many_blocksto1..GC_MANY_BLOCKS_MAX, but the existing free-list path uses the raw global. IfGC_MANY_BLOCKS_DEFAULTis changed to an out-of-range value, this path can hand out a much larger batch than the allocator path.♻️ Proposed fix
int GC_many_blocks = GC_MANY_BLOCKS_DEFAULT; + +static int GC_effective_many_blocks(void) +{ + int nblocks = GC_many_blocks; + if (nblocks < 1) nblocks = 1; + if (nblocks > GC_MANY_BLOCKS_MAX) nblocks = GC_MANY_BLOCKS_MAX; + return nblocks; +} ... - if ((word)my_bytes_allocd >= (word)GC_many_blocks * HBLKSIZE) { + if ((word)my_bytes_allocd >= (word)GC_effective_many_blocks() * HBLKSIZE) { ... - int nblocks = GC_many_blocks; + int nblocks = GC_effective_many_blocks(); int i; - - if (nblocks < 1) nblocks = 1; - if (nblocks > GC_MANY_BLOCKS_MAX) nblocks = GC_MANY_BLOCKS_MAX;Also applies to: 54-58
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packaging/patches/boehmgc-batch-malloc-many.patch` at line 37, The existing-free-list batching logic in the boehmgc patch still uses the raw GC_many_blocks value, so it can diverge from the new-block path when the default is out of range. Update the free-list splitting path that checks my_bytes_allocd against GC_many_blocks * HBLKSIZE to apply the same 1..GC_MANY_BLOCKS_MAX clamp used elsewhere before computing the batch size, keeping the behavior consistent in the allocator’s batch-handout logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packaging/patches/boehmgc-batch-malloc-many.patch`:
- Line 37: The existing-free-list batching logic in the boehmgc patch still uses
the raw GC_many_blocks value, so it can diverge from the new-block path when the
default is out of range. Update the free-list splitting path that checks
my_bytes_allocd against GC_many_blocks * HBLKSIZE to apply the same
1..GC_MANY_BLOCKS_MAX clamp used elsewhere before computing the batch size,
keeping the behavior consistent in the allocator’s batch-handout logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3bcc7734-92e4-415a-a85f-874e0d3b1311
📒 Files selected for processing (2)
packaging/dependencies.nixpackaging/patches/boehmgc-batch-malloc-many.patch
cole-h
reviewed
Jul 8, 2026
edolstra
force-pushed
the
batch-gc-malloc-many
branch
from
e5999e7 to
aa68c2e
Compare
cole-h
previously approved these changes
Jul 9, 2026
Declare GC_MANY_BLOCKS_DEFAULT, GC_MANY_BLOCKS_MAX and GC_many_blocks in private/gc_priv.h (following the GC_EXTERN convention for internal globals) instead of defining them in mallocx.c and repeating the constant 64 and an extern declaration in misc.c's environment variable parsing. Verified that the compile-time default still applies (futex calls drop 6.6x vs GC_MALLOC_MANY_BLOCKS=1) and the environment variable override still works. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
edolstra
force-pushed
the
batch-gc-malloc-many
branch
from
aa68c2e to
7ed5959
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packaging/patches/boehmgc-batch-malloc-many.patch (1)
46-58: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winReclaim-list path reads
GC_many_blocksunclamped, unlike the block path.Line 51 uses
(word)GC_many_blocks * HBLKSIZEdirectly, but the block-allocation path (lines 68-72) clamps a local copy to[1, GC_MANY_BLOCKS_MAX]. The two paths therefore disagree ifGC_many_blocksis ever outside that range — e.g. a compile-time-DGC_MANY_BLOCKS_DEFAULTset aboveGC_MANY_BLOCKS_MAX(the-Dvalue is never validated) or set to0(block path floors to 1, reclaim path would break after a single object). ClampingGC_many_blocksonce at initialization removes the divergence and lets you drop the per-call clamp in the block path.♻️ Clamp once at init (misc.c)
char * many_blocks_string = GETENV("GC_MALLOC_MANY_BLOCKS"); if (many_blocks_string != NULL) { int many_blocks = atoi(many_blocks_string); if (many_blocks > 0 && many_blocks <= GC_MANY_BLOCKS_MAX) GC_many_blocks = many_blocks; } + if (GC_many_blocks < 1) GC_many_blocks = 1; + if (GC_many_blocks > GC_MANY_BLOCKS_MAX) + GC_many_blocks = GC_MANY_BLOCKS_MAX;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packaging/patches/boehmgc-batch-malloc-many.patch` around lines 46 - 58, The reclaim-list branch in the GC allocation logic reads GC_many_blocks directly and can diverge from the block-allocation branch, which already clamps to a safe range. Update the initialization path that sets GC_many_blocks so it is clamped once to [1, GC_MANY_BLOCKS_MAX], then rely on that normalized value in the allocation code paths (including the reclaim-list check) to keep behavior consistent and remove the need for repeated per-call clamping.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packaging/patches/boehmgc-batch-malloc-many.patch`:
- Around line 46-58: The reclaim-list branch in the GC allocation logic reads
GC_many_blocks directly and can diverge from the block-allocation branch, which
already clamps to a safe range. Update the initialization path that sets
GC_many_blocks so it is clamped once to [1, GC_MANY_BLOCKS_MAX], then rely on
that normalized value in the allocation code paths (including the reclaim-list
check) to keep behavior consistent and remove the need for repeated per-call
clamping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 330a7df3-a161-45b4-a450-d3c872da57e7
📒 Files selected for processing (2)
packaging/dependencies.nixpackaging/patches/boehmgc-batch-malloc-many.patch
🚧 Files skipped from review as they are similar to previous changes (1)
- packaging/dependencies.nix
cole-h
approved these changes
Jul 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
This patches boehmgc to have
GC_generic_malloc_many()(which refills the thread-local allocation caches) hand out more objects per call. The upstream version does only one page per call, but that's not a lot (~170 values). This causes high contention on the allocation lock.Returning 64 pages each call reduces futex syscalls by 6x (3.58M to 0.58M) for
nix search nixpkgs --no-eval-cache fizzbuzz, and reduces elapsed time at 24 cores from 4.5s to 4.0s.Improvement over #546:
Context
Summary by CodeRabbit