Skip to content
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

5.2 multicore markdelay #3029

Merged
merged 4 commits into from
Oct 16, 2024
Merged

Conversation

stedolan
Copy link
Contributor

@stedolan stedolan commented Sep 11, 2024

This patch extends the existing single-domain-only markdelay patch to work with multiple domains, and (in separate commits) re-enables all the parallel tests and adds a new parallel test.

The idea is to add a new phase, Phase_sweep_main, prior to Phase_sweep_and_mark_main, in which only sweeping and not marking can be done. Since the roots have not yet been marked, it is safe to give new allocations the UNMARKED status during this phase, allowing them to be collected a cycle earlier than otherwise. Also, caml_modify need not do darkening during this phase. This was implemented for single-domain programs in #2348, and this PR extends it to multi-domain programs.

Since domains freely share new allocations, all must agree on phase transitions. So, when any domain finishes sweeping, it requests a transition to Phase_sweep_and_mark_main. Instead of being acted upon directly, this sets a flag to trigger the phase transition at the next minor GC. (The phase transition involves marking roots, which must be done with the minor heap empty, and the simplest way to achieve this is to wait until the next point when the minor heap naturally empties)

@stedolan stedolan force-pushed the 5.2-multicore-markdelay branch from e3fd82b to 5e85fe9 Compare September 11, 2024 12:09
@stedolan stedolan marked this pull request as ready for review September 11, 2024 12:17
@stedolan stedolan force-pushed the 5.2-multicore-markdelay branch from 5e85fe9 to 69cd4ed Compare September 11, 2024 12:47
@mshinwell mshinwell deleted the branch ocaml-flambda:main September 20, 2024 14:27
@mshinwell mshinwell closed this Sep 20, 2024
@mshinwell mshinwell reopened this Sep 23, 2024
@mshinwell mshinwell changed the base branch from merge-5.2-staging to main September 23, 2024 06:38
@mshinwell
Copy link
Collaborator

(This was automatically closed by Github when 5.2-merge-staging was merged. I've re-opened and changed the base to main.)

Copy link
Contributor

@NickBarnes NickBarnes left a comment

Choose a reason for hiding this comment

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

This is all OK, but definitely needs some more comments in a few places (and possibly some better names for things), as commented.

ocaml/runtime/major_gc.c Outdated Show resolved Hide resolved
ocaml/runtime/major_gc.c Show resolved Hide resolved
ocaml/runtime/major_gc.c Outdated Show resolved Hide resolved
ocaml/runtime/major_gc.c Outdated Show resolved Hide resolved
ocaml/runtime/major_gc.c Show resolved Hide resolved
ocaml/runtime/minor_gc.c Show resolved Hide resolved
  - Domains now begin marking simultaneously at some minor GC during the major cycle

  - Before marking, the write barrier is disabled and fresh allocations are UNMARKED

  - Change to ephemeron logic: per-cycle ephemeron data structure reset is now done
    at marking start rather than at the start of the cycle. (In particular, this makes
    orphaned work simpler to deal with)

(cherry picked from commit f376410)
Includes several systhreads tests that rely on the interaction between
systhreads, domains and backup threads.

(cherry picked from commit c31eebd)
@stedolan stedolan force-pushed the 5.2-multicore-markdelay branch from 69cd4ed to 5d2b2dc Compare October 14, 2024 15:44
@stedolan
Copy link
Contributor Author

I rebased to current main and added another commit with review feedback. There are no functional changes in the new commit - it's entirely shuffling code around, renaming, and adding comments.

Copy link
Collaborator

@mshinwell mshinwell left a comment

Choose a reason for hiding this comment

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

On behalf of Nick Barnes

@stedolan stedolan merged commit f937ddd into ocaml-flambda:main Oct 16, 2024
16 checks passed
ncik-roberts pushed a commit that referenced this pull request Oct 21, 2024
* Markdelay support for multi-domain programs

  - Domains now begin marking simultaneously at some minor GC during the major cycle

  - Before marking, the write barrier is disabled and fresh allocations are UNMARKED

  - Change to ephemeron logic: per-cycle ephemeron data structure reset is now done
    at marking start rather than at the start of the cycle. (In particular, this makes
    orphaned work simpler to deal with)

* Re-enable parallel tests

Includes several systhreads tests that rely on the interaction between
systhreads, domains and backup threads.

* Add a new test (tests/parallel/churn.ml) stressing cross-domain promotion
lukemaurer pushed a commit that referenced this pull request Oct 23, 2024
* Markdelay support for multi-domain programs

  - Domains now begin marking simultaneously at some minor GC during the major cycle

  - Before marking, the write barrier is disabled and fresh allocations are UNMARKED

  - Change to ephemeron logic: per-cycle ephemeron data structure reset is now done
    at marking start rather than at the start of the cycle. (In particular, this makes
    orphaned work simpler to deal with)

* Re-enable parallel tests

Includes several systhreads tests that rely on the interaction between
systhreads, domains and backup threads.

* Add a new test (tests/parallel/churn.ml) stressing cross-domain promotion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants