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

feat!: Grain implementation of memory allocator #530

Merged
merged 3 commits into from
Mar 7, 2021
Merged

Conversation

ospencer
Copy link
Member

Grain Implementation of malloc/free

This is a breaking change, as some runtime libraries were moved around.

This PR re-implements the Grain memory allocator in Grain. The allocator itself is nearly identical to the old version, so the interesting bits are the changes to how the Grain runtime works, and that effect on the new runtime libraries.

Compilation Modes

The Grain compiler sports two new compilation modes—Runtime and MemoryAllocation. The "normal" compilation mode is called Normal. Runtime mode and MemoryAllocation mode can be enabled for a module by adding the /* compilation-mode: runtime */ or /* compilation-mode: malloc */ block comments at the start of the file.

Runtime Mode

The essence behind Runtime mode is that garbage collection/memory allocation isn't set up yet (and that's probably what's currently being compiled!). As such, modules in Runtime mode do not implicitly depend on Pervasives or have garbage collection enabled. For any allocations that need to be done (just closures), that data exists in the runtime heap—Grain now reserves 4096 bytes of memory for the runtime. After some testing, the runtime currently uses 392 bytes of memory, so this amount should give us some breathing room.

MemoryAllocation Mode

This mode is the same as Runtime mode, with the addition that the pointer to the runtime heap exists in this module. (As such, Runtime mode imports that pointer from here.)

Normal Mode

This is the mode that we already know and love, but as a quick refresher—modules compiled in this mode implicitly depend on Pervasives and have their memory managed via the GC.

Runtime Libraries

There's a new runtime folder in stdlib where the runtime things will live, i.e. print, toString, numbers, etc.
The WasmI32/WasmI64/WasmF32/WasmF64 libraries have been moved from the unsafe stdlib directory to runtime/unsafe. Because malloc.gr needs to depend on wasmi32.gr and it depended on Pervasives for print, the conversion and print functionalities have been moved to runtime/unsafe/conv.gr and runtime/unsafe/utils.gr, which is the breaking change.
Additionally, there's now also runtime/debug.gr, where runtime debugging utilities will live (and I even got to use it to debug malloc!).

JS Runtime

The JS runtime got to be cleaned up a bit with no more need to manually load the memory allocator since the allocator is now just a regular Grain module that is depended on in the normal way.

@ospencer ospencer requested a review from a team February 13, 2021 17:48
@ospencer ospencer self-assigned this Feb 13, 2021
@@ -58,7 +58,7 @@
let consume_comments () =
let out_comments = !comments in
comments := [];
out_comments
List.rev(out_comments)
Copy link
Member Author

Choose a reason for hiding this comment

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

Turns out all of our comments were in reverse order.

Copy link
Member

Choose a reason for hiding this comment

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

🤦

Copy link
Member

Choose a reason for hiding this comment

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

🙃

@ospencer ospencer changed the title chore!: Grain implementation of memory allocator feat!: Grain implementation of memory allocator Feb 15, 2021
Copy link
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

Things I saw or was confused by!

compiler/src/codegen/compcore.re Show resolved Hide resolved
compiler/src/codegen/compcore.re Show resolved Hide resolved
compiler/src/codegen/compcore.re Show resolved Hide resolved
compiler/src/codegen/compcore.re Show resolved Hide resolved
compiler/src/codegen/compcore.re Show resolved Hide resolved
stdlib/runtime/debug.gr Show resolved Hide resolved
stdlib/runtime/malloc.gr Show resolved Hide resolved
stdlib/runtime/malloc.gr Outdated Show resolved Hide resolved
stdlib/runtime/malloc.gr Show resolved Hide resolved
stdlib/runtime/unsafe/utils.gr Outdated Show resolved Hide resolved
@peblair peblair self-requested a review February 25, 2021 17:16
Copy link
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

Looking great! Thanks for entertaining my questions! 🙇

Copy link
Member

@peblair peblair 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 awesome! Please review my comments before merging, but this looks great! 🚀


let round_allocation_size = (num_words: int): int => round_up(num_words, 4);
let heap_allocate = (wasm_mod, env, num_words: int) =>
Copy link
Member

Choose a reason for hiding this comment

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

I kind of wish we used an ADT here which was something like this:

type allocation_size =
  | AllocWords(int)
  | AllocBytes(int)

Even if we only have AllocWords as the only variant, I feel like it'd be more expressive/readable/"safe":tm:, since it will be clear at the use sites that the number we're passing into heap_allocate represents words rather than bytes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Could be a good future change, yeah.

@@ -429,7 +429,7 @@ and block = list(instr);
[@deriving sexp]
type import_type =
| MFuncImport(list(asmtype), list(asmtype))
| MGlobalImport(asmtype);
| MGlobalImport(asmtype, bool);
Copy link
Member

Choose a reason for hiding this comment

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

We should consider documenting these fields. Perhaps we should try to add documentation to an ADT definition whenever we update it?

Copy link
Member Author

Choose a reason for hiding this comment

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

In general, I suppose so, yeah. This one sorta makes sense because globals went from always being immutable to optionally being mutable. I think in general we could prefer the record-type variants since those are self-documenting.

@@ -58,7 +58,7 @@
let consume_comments () =
let out_comments = !comments in
comments := [];
out_comments
List.rev(out_comments)
Copy link
Member

Choose a reason for hiding this comment

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

🙃


let set_unit = ((name, source)) => current_unit := (name, source);
let current_unit = ref(("", "", Normal));
Copy link
Member

Choose a reason for hiding this comment

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

This has inspired me to open #547 .

"GRAIN$EXPORT$malloc"
).value;
return this._runtime.memoryManager.requiredExport("malloc")(
closure,
Copy link
Member

Choose a reason for hiding this comment

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

@ospencer To make sure I understand, this is needed because we now need to follow the Grain calling convention, yes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup.

@ospencer ospencer merged commit fd8faaa into main Mar 7, 2021
@ospencer ospencer deleted the grain-runtime branch March 7, 2021 14:36
@github-actions github-actions bot mentioned this pull request Apr 20, 2021
@github-actions github-actions bot mentioned this pull request May 31, 2022
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.

3 participants