Skip to content

Gc.set now changes the minor heap size of all domains#11142

Draft
sabine wants to merge 1 commit intoocaml:trunkfrom
sabine:gc_set_changes_minor_heap_size_of_all_domains
Draft

Gc.set now changes the minor heap size of all domains#11142
sabine wants to merge 1 commit intoocaml:trunkfrom
sabine:gc_set_changes_minor_heap_size_of_all_domains

Conversation

@sabine
Copy link
Copy Markdown
Contributor

@sabine sabine commented Mar 29, 2022

Currently, every domain has its own minor heap size that it can modify within the boundary of the minor heap reservation. However, this semantics of Gc.set may be confusing to the user.

This PR proposes to adjust the semantics of Gc.set and simplify the minor heap allocation code (with the intent of keeping minor heap allocation code complexity as low as we can for the 5.00 release):

  1. Gc.set updates the minor heap size for all domains.
  2. Consequently: the minor heap size of all domains is identical. So, instead of maintaining it in Caml_state, we make the minor heap size global.
  3. As the minor heap reservation is split into equal parts of the same size (as established in Reserve only caml_minor_heap_max_wsz * Max_domains for the minor heap #11082), we do not need to keep track of minor_heap_area_start and minor_heap_area_end in dom_internal. Since access to these boundaries is not performance-critical, we can compute them from the minor heap size and domain index instead.
  4. We add the missing freeing (decommitting) of the minor heap on domain_terminate() under the protection of the domain lock. This establishes the invariant that only active domains have a minor heap allocated (committed) within the minor heap reservation of all domains.

In contrast to #11082, setting a smaller minor heap size also stops the world and makes a new minor heap reservation, as we adjust the minor heap size of all domains.

This patch resolves the FIXME from #11082 relating to the duplicate reallocation of the leader domain's minor heap.

As this patch implements the user-visible extension of Gc.set semantics for 5.00, we propose a shared Changes entry for #11082 and this PR.

@sabine sabine marked this pull request as draft March 29, 2022 10:43
@sabine sabine force-pushed the gc_set_changes_minor_heap_size_of_all_domains branch 2 times, most recently from b17f970 to a723b64 Compare March 29, 2022 10:58
@sabine sabine marked this pull request as ready for review March 29, 2022 11:15
@sabine sabine force-pushed the gc_set_changes_minor_heap_size_of_all_domains branch from a723b64 to 703c990 Compare March 29, 2022 11:17
@sabine sabine marked this pull request as draft March 29, 2022 11:19
Every call to `Gc.set` that modifies the minor heap size causes
a stop-the-world section, in order to adjust the minor heaps of all
domains.

On `domain_terminate()`, we free the minor heap to maintain the
invariant "only active domains have a minor heap allocated".

Resulting simplifications: we no longer keep track of individual
domains' minor heap size - instead, we have a global variable
`caml_minor_heap_wsz` for the size of the minor heap, and
every domain allocates a minor heap of this size.

This patch resolves the FIXME from ocaml#11082 relating to the duplicate
reallocation of the leader domain's minor heap.
@sabine sabine force-pushed the gc_set_changes_minor_heap_size_of_all_domains branch from 703c990 to f573882 Compare March 29, 2022 13:35
@gasche
Copy link
Copy Markdown
Member

gasche commented Mar 29, 2022

Here is what I understand about the proposed implementation (I wasn't involved in writing the code of this one PR and could review the implementation) is that:

  1. It's a nice simplification of the code.
  2. But it comes at the cost of deciding that all domains must use the same minor heap size, which I'm not sure we want.

On the current trunk, we would reserve the same memory size for all domains, but each domain can allocate a different size. Here all domains must allocate the same size.

I could see this being an issue in heterogeneous workflows when the "main domain", doing most of the work, uses an in-fact-fairly-large minor heap, but we still want "auxiliary domains" around to, say, offload serialization/logging tasks, and for those we don't want a large minor heap.
(Another way to think of this: if you don't care about latency, you can set a large minor heap, where the limit for "how large" is: an amount of space you don't mind wasting one half unused on average. But if you have to use N times the memory for N domains, you can't necessarily afford a large minor heap for your main domain anymore. For example Coq uses a 256 Mio minor heap, if the cost of having 7 worker domains ready to offload some work is to have 2Gio of minor heap, that may be too much.)

Note that it would be possible to provide the feature of this PR, that is an ability to set all the minor heap size for all domains at once, and also the minor heap of future domains (which I think we want in any case), while preserving the ability to set different minor heap sizes for specific domains. But then:

  • the code simplifications of this PR are gone
  • we need to figure out an UI to say this (whereas: both trunk behavior and the PR use only Gc.set in a 4.x-compatible way, because they don't try to offer two different options.)

@sabine
Copy link
Copy Markdown
Contributor Author

sabine commented Apr 4, 2022

You're correct: whether we want all the simplifications of this PR is not necessarily a given - the current design (reserving, and then allocating a minor heap up to the size of the per-domain reservation) allows for use cases where the memory load is not evenly distributed.

As I understand it, extending the UI for minor heap resizing is a post-5.00 feature.

we need to figure out an UI to say this (whereas: both trunk behavior and the PR use only Gc.set in a 4.x-compatible way, because they don't try to offer two different options.)

Indeed, and it's not immediately obvious what a good extended UI looks like. I opened a discussion at #11156.

For the 5.00 release, the simplifications are attractive in the sense that they reduce code complexity, for the time being. But indeed, when we want to support heterogenous workloads on the minor heap by letting every domain control its own minor heap allocation size, we will have to add most (but not all) of this complexity back.

I wanted to see what the differences were, and in that sense this experiment has been useful.

@dra27 dra27 mentioned this pull request Apr 20, 2022
16 tasks
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.

2 participants