Minimize lazy allocation of the GC store

[alexcrichton]

This commit is an effort to minimize the number of entrypoints which might lazily allocate a GC store. The is currently done through StoreOpaque::gc_store_mut but this method is very commonly used meaning that there are many many places to audit for lazily allocating a GC store. The reason that this needs an audit is that lazy allocation is an async operation right now that must be on a fiber and is something I'm looking to fix as part of #11262.

This commit performs a few refactorings to achieve this:

• gc_store_mut is renamed to ensure_gc_store. This is intended to be an async function in the future and clearly demarcates where lazy allocation of a GC store is occurring.

• require_gc_store{,_mut} is now added which is a pure accessor of the GC store with no lazy allocation. Most locations previously using gc_store_mut are updated to use this instead.

Documentation is added to store methods to clearly indicate which ones are allocating and which ones should only be called in a context where allocation should already have happened.

[github-actions]

Subscribe to Label Action

cc @fitzgen

This issue or pull request has been labeled: "wasmtime:api", "wasmtime:ref-types"

Thus the following users have been cc'd because of the following labels:

• fitzgen: wasmtime:ref-types

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

[alexcrichton]

@fitzgen ok so turns out there were more places than expected that really do lazily allocate a GC store, notably dealing with i31ref and null references. Best I can think of for handling this is to either:

1. Support allocating a GC store without a GC heap so things like write_gc_ref if the reference is i31ref or null. We'd then have ensure_gc_store (sync, just allocates some data structures) and ensure_gc_heap (async, allocates a heap) where both return &mut GcStore. Just ensure_gc_store wouldn't allocate a heap so we'd still skip it with i31ref and null references.
2. Change callers of write_gc_ref to internally check for null references and i31ref an directly perform writes. This would basically require that all GC implementations don't need barriers for i31ref and null and would require more runtime checking in the host when performing those writes.

Do you have other ideas and/or a preference about how might be best to resolve this?

[fitzgen]

Change callers of write_gc_ref to internally check for null references and i31ref an directly perform writes. This would basically require that all GC implementations don't need barriers for i31ref and null and would require more runtime checking in the host when performing those writes.

We already bake in the assumption that i31refs don't need barriers in various places, because they aren't actually GC heap objects, so they aren't managed by the GC runtime. Same for null pointers. Although writing an i31ref or null reference into a field of a non-null object does need to call GC barriers.

Anyways, we have some of these checks in the GcStore::* barrier methods, and we could pull them out to being StoreOpaque::* barrier methods perhaps.

Also, if we have a non-i31ref, non-null reference, then we must have already allocated the GC store. And if we don't have one of those, then we don't need barriers, so we don't need to allocate a GcStore lazily to run barriers.

So I think that the StoreOpaque::* barrier methods shouldn't ever need to lazily allocate a GcStore. Basically something like

impl StoreOpaque {
    pub(crate) fn clone_gc_ref(&mut self, gc_ref: &GcRef) -> GcRef {
        if gc_ref.is_i31() {
            gc_ref.copy_i31()
        } else {
            self.gc_store
                .as_mut()
                .expect("non-null, non-i31 gc ref means we must have a gc store")
                .clone_gc_ref(gc_ref)
    }
}

[alexcrichton]

Ok I think everything should be handled now. Mind taking another look @fitzgen?

[fitzgen]

Suggested change

	self.result.module.needs_gc_heap |= match table.ref_type.heap_type.top() {
	WasmHeapTopType::Extern | WasmHeapTopType::Exn | WasmHeapTopType::Any => {
	true
	}
	WasmHeapTopType::Func | WasmHeapTopType::Cont => false,
	};
	self.result.module.needs_gc_heap |= table
	.ref_type
	.is_vmgcref_type_and_points_to_object();

[alexcrichton]

I ended up going with is_vmgcref_type since the method here is on wasmtime_environ types and not wasmtime types (therefore is_vmgcref_type_and_points_to_object not naively available). I tried using is_vmgcref_type_not_i31 but due to the way table initialization works it requires the GC store to be present right now so I switched it to is_vmgcref_type. Should be easy enough to fix in the future if we really want, but I figure it's not too important to be too optimal here and keeping the same code between the externref/exnref/anyref clauses is nice

[fitzgen]

LOL how did this sneak by????