Skip to content

Allow static allocation of mutable values#178

Closed
chambart wants to merge 6 commits intoocaml:trunkfrom
chambart:global_mutables
Closed

Allow static allocation of mutable values#178
chambart wants to merge 6 commits intoocaml:trunkfrom
chambart:global_mutables

Conversation

@chambart
Copy link
Copy Markdown
Contributor

This patch modifies the runtime to allow multiple roots per compilation unit and turns some mutable allocations into structured constants. Those are recorded as roots for the GC.
The makeblock that can be statically allocated are those that are evaluated at most once. This allows some code to be allocation free (see the test https://github.com/ocaml/ocaml/blob/de65bc6e6e2e7a51a5d5b626a08f14f0d5fc16b1/testsuite/tests/asmcomp/staticalloc_mutable.ml).
In particular it allows user defined exception to behave exactly as compiler defined ones.

Note: #177 is included in this patch suite to avoid a increasing the cost of minor collections.

This allows the gc to handle multiple roots per compilation unit.
Before this patch, the unique root was the symbol of the compilation
unit. i.e. for a module `Module` the global symbol is `camlModule`.
Roots are now recorded in the `camlModule__mutable_globals` symbol
which is a null terminated array of pointers to roots.
Structured constants created with `~mutability:Mutable` are now
recorded as roots for the GC.
Turn mutable block allocation that can be evaluated only once into
structured constants. Those are recorded as mutable to allow the
GC to traverse them on major collections.
Turns
 set_oo_id (makeblock (object_tag, name, id))
into
 let var = makeblock (object_tag, name, id) in
 set_oo_id var;
 var

This allows exceptions to directly reference the block and
be statically allocated:
  exception Exn of int
  raise (Exn 3)
@mshinwell
Copy link
Copy Markdown
Contributor

I am in support of this patch being merged, once it is reviewed.

@alainfrisch
Copy link
Copy Markdown
Contributor

Couldn't we keep the benefit of static allocation of mutable global values, without changing the logic of having a single global root per unit? I'm thinking about simply adding more fields to the global block (which already contains more values than those exported in the interface).

@chambart
Copy link
Copy Markdown
Contributor Author

We could, but I kind of think that this is a bit cleaner. And more importantly, this is a part of a (work in progress) patch series that needs it.

@gasche
Copy link
Copy Markdown
Member

gasche commented Jul 26, 2015

This patch should be merge ready, but #177 (also included in this PR) seems problematic for now.

@mshinwell
Copy link
Copy Markdown
Contributor

Note that GPR #262 (a subset of this pull request) has now been merged.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

From what I can tell, this file change[bytecomp/translmod.ml) is unnecessary ( correct me if I am wrong).
Is it possible to leave it untouched? The old version is straightforward to be optimized and inlined in my JS backend, here an Strict binding is introduced which makes it a bit tricky to do a general optimization, of course, I can do an ad-hoc peephole, but it would be nice to avoid too many ad-hoc optimizations, thanks

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This has already changed in trunk, but not in that direction.
The change made in trunk makes it more difficult to allocates exceptions statically currently, but easier for Flambda.
In general I do not expect this PR to be merged and will do that separately in the flambda branch. The important part for flambda was already merged (the GC stuff).

@chambart chambart closed this Nov 16, 2015
chambart pushed a commit to chambart/ocaml-1 that referenced this pull request Oct 5, 2021
Refactor linear_to_cfg translation of Ladjust_trap_depth
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
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