Skip to content

Faster minor GC#14571

Open
NickBarnes wants to merge 5 commits intoocaml:trunkfrom
NickBarnes:faster-oldify-upstream
Open

Faster minor GC#14571
NickBarnes wants to merge 5 commits intoocaml:trunkfrom
NickBarnes:faster-oldify-upstream

Conversation

@NickBarnes
Copy link
Contributor

This is a rewrite of the hot code in the minor GC, the function oldify_one (and, to some extent, oldify_mopup). Basically the fast path through oldify_one is now all inlined, does not touch global variables, and shared_heap.c makes the major heap free lists available to an inlined function in shared_heap.h, so that in the common case the minor GC can allocate inline.

There are five commits, of which the first four are other small PRs (#14492, #14568, #14569, #14570). The last commit is the main change here.

Upstreamed from OxCaml, which is somewhat different because OCaml has a different free-list implementation (#13616). I have measured the matching PR in OxCaml (oxcaml/oxcaml#5163) to make a 2-3% total runtime improvement in some real workloads. Basically any workload which does much minor GC promotion should see measurable speed gains from this PR.

@NickBarnes NickBarnes force-pushed the faster-oldify-upstream branch from 5355bf1 to 31ea764 Compare February 19, 2026 16:54
@gasche
Copy link
Member

gasche commented Feb 20, 2026

A minor meta-comment: the last commit where the action happens is a bit too large for comfort, it affects about 700 lines of very-tricky code. As a reviewer I would prefer to have several smaller commits to pace myself and be able to think about changes in isolation. It may very well be that the specific change being done here had to be done in one go, or that this is just a side-effect of your working habits for good reasons. But my impression is that https://github.com/oxcaml/oxcaml/pull/5163/commits has a more separated commit history, and that they were somehow squashed together during the (possibly non-trivial) rebase process. I would prefer to keep separate commits when possible, to make review easier.

Copy link
Member

@damiendoligez damiendoligez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Just a few remarks.

Comment on lines +246 to +247
* Promoted_hd (so that, e.g., major GC marking can work while the
* block is still on our oldify todo list). Have to do this here,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

major GC marking can work while the block is still on our oldify todo list

That doesn't sound very likely because major GC marking cannot be allowed to see the block before you have initialized all the scannable fields.

@tmcgilchrist
Copy link
Contributor

Basically any workload which does much minor GC promotion should see measurable speed gains from this PR.

@NickBarnes is there a sharable benchmark program I could use to reproduce your results? I haven't reviewed the code yet (it's on the list), however the explanation sounds compelling.

@gasche
Copy link
Member

gasche commented Mar 11, 2026

@NickBarnes when you have time it would be nice to address the review comments of @damiendoligez so that we can merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants