Skip to content

Fixes to obscure mark-delay problems #3297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

NickBarnes
Copy link
Contributor

@NickBarnes NickBarnes commented Nov 21, 2024

When upstreaming #2348 / #2358 / #3029 to ocaml/ocaml#13580, two problems were detected by CI. This PR has fixes for each, in separate commits, and also a fix for a third problem spotted by @stedolan during review discussions.

The first is a pacing problem in which major GC work performed can, in some circumstances, fall further and further behind the estimated work required. See the discussion on ocaml/ocaml#13580 for more details. In a long-running process this could be problematic, leading to longer GC pauses. On 32-bit platforms the flawed estimation can lead to an overflow which stops GC altogether, fatally. The short-term fix in this PR applies a "catch-up" adjustment when it may be necessary. Work is in progress on replacing all the pacing code, which should result in a longer-term fix.

The second problem concerns adopting ephemerons which have been orphaned by a terminating domain. They are marked by the terminating domain, so survive that GC cycle. They are then adopted at the start of marking in the next cycle, when they should be unmarked (due to the major GC cycle colour-switch). However, at present all domains attempt to adopt orphans at the start of marking, without a subsequent barrier, so it is possible for the terminating domain to get past this point, mark the ephemerons and then orphan them, before some other domain attempts to adopt and picks up the ephemerons. The fact that the ephemerons have already been marked in this cycle is detected in the debug runtime by a failing assertion. It is unclear whether this difference in marking is harmless; the multicore major GC ephemeron marking system is complex and it seems unwise to change this behaviour. In this PR, orphan adoption is moved inside the barrier at the start of marking; it starts with a very cheap check (no_orphaned_work()) so this change is almost free in the common case and fairly cheap even if there are orphans.

This second problem was detected by the ocaml-multicore/multicoretests run automatically by CI on ocaml/ocaml#13580 due to the run-multicoretests label.

The third problem is due to the fact that marking is requested in one slice, but then actually begun on the next minor GC. If insufficient allocation takes place to trigger a minor GC, successive major slices may occur without the major GC progressing. This is now detected on the following slice: if no sweeping is available and yet marking has not started, a minor GC is requested.

@mshinwell
Copy link
Collaborator

Given that these seem to be three unrelated bugs, I think it's probably best if we have three separate PRs for them.

@mshinwell mshinwell added runtime bug Something isn't working OCaml 5 labels Nov 22, 2024
@NickBarnes
Copy link
Contributor Author

OK, I'll do that next week (along with a fix to a finalisers problem which one of these fixes exposes).

Copy link
Contributor

@stedolan stedolan 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, thanks!

@NickBarnes
Copy link
Contributor Author

Given that these seem to be three unrelated bugs, I think it's probably best if we have three separate PRs for them.

Done. #3331 / #3332 / #3333. Closing this.

@NickBarnes NickBarnes closed this Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working OCaml 5 runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants