Cache GC subtype checks per-store#13860
Conversation
Avoids taking a lock on the engine's type registry. Doesn't really affect single-threaded performance, but for a mid-sized Kotlin component I see performance improve from 4050 RPS to 17680 RPS.
| /// FIXME(#13484) this field is a temporary workaround for a "true | ||
| /// solution" where supertypes are stored inline in a compiled-code-visible | ||
| /// location which means that a libcall isn't needed at all (nor | ||
| /// synchronization) to determine subtype/supertype relationships. | ||
| subtype_check_cache: HashMap<u64, bool, NopHasher>, |
There was a problem hiding this comment.
FWIW, even with inline supertype arrays, we will still need to consult the type registry in the limit: for example, when a module defines a non-final type and is checking whether another type defined outside that module (perhaps a last-minute host-defined type or a type in a module that wasn't even loaded into the engine until after the first module was instantiated) is a subtype of the non-final type.
Not sure a cache will still be worth it at that point; just noting that inline supertype arrays won't Solve All The Problems
There was a problem hiding this comment.
Agreed yeah, and I would agree that in that case the cache here should still get deleted. The main purpose here is to avoid horizontal scaling bottlenecks for idiomatic code, which the inline supertype arrays would solve.
| /// caches results store-locally to avoid contention on the engine's type | ||
| /// registry lock. See the documentation of the `subtype_check_cache` field | ||
| /// for details. | ||
| #[cfg(feature = "gc")] |
There was a problem hiding this comment.
This shouldn't be necessary inside this GC-specific file, no?
* Defer DRC drops in host-side `write_gc_ref` (#13842) * Defer DRC drops in host-side `write_gc_ref` This commit is an update to the DRC collector to avoid immediately dropping GC references in `write_gc_ref`. This is done today by threading contextual information such as `ExternRefHostDataTable` all the way down to `write_gc_ref` and `drop_gc_ref` hooks, but a refactoring that I'm planning to do is going to make it significantly more complicated to thread all necessary contextual information through to these hooks. Specifically I'm hoping to store `TraceInfo` outside of stores and instead inside of `Module` and `RegisteredType` to avoid the need for per-type work done during module instantiation. Threading this context through these hooks is effectively ergonomically a no-go. The strategy then taken in this commit is to change the DRC allocator, the only allocator we have that needs this information during these barriers. The DRC allocator now defers full deallocation of GC allocations to a later point in time where contextual information is available (e.g. during a GC itself). This means that host-initiated writes/drops are no longer guaranteed to actually run destructors immediately (same as with the copying collector). Internally the DRC heap already has a stack of references to decrement, and previously it was only needed during a decrement operation and now it's instead modified to persist between GC barriers through to a GC itself. * Restructure to always use the `dec_ref_stack` This updates the processing of on-stack roots to additionally get moved onto the `dec_ref_stack` in addition to host-initiated overwrites that reach a refcount of 0. * fix: use portable WASMTIME_ALIGNOF in val.h static assertions (#13837) The alignment static_assert checks in val.h used __alignof (GCC extension) which returns preferred alignment rather than ABI alignment. On i686 SysV ABI these differ for uint64_t (8 vs 4), causing spurious compile failures. Replace with a WASMTIME_ALIGNOF macro that selects the correct operator per compiler: alignof (C11/C++11) for GCC/Clang and C++, __alignof for MSVC C mode which does not support alignof. The macro is #undef'd immediately after use. * Add a tunable knob for the GC heap initial size (#13841) * Add Config::gc_heap_initial_size and -O gc-heap-initial-size Setting an initial heap size > 0 avoids frequent collect-then-grow cycles during instance startup. In testing with a mid-sized Kotlin component, this improves `wasmtime serve` throughput from 240 RPS to 1550 RPS when processing a single request at a time, and from 168 to 1809 RPS for concurrency == 20. * Move initial GC heap size to a tunable Also add a small smoke test plus fuzzing integration. * Drop copying collector note * Add a test for init > reservation * Fix fuzz tests --------- Co-authored-by: Till Schneidereit <till@tillschneidereit.net> * Fix missing rounding in gc initial size when fuzzing (#13855) This fixes and oversight from #13841 where the clamping done there during fuzzing was insufficient because the clamping happened pre-rounding which didn't match what Wasmtime did internally. * Remove `ensure_trace_info` methods (#13843) * Isolate more GC-specific data to `store/gc.rs` This is an attempt to move more information that's only needed for GC-enabled Wasmtime to `gc`-feature-gated files. This moves some methods, types, etc, from `store.rs` to `store/gc.rs` * Store `TraceInfo` in registered type information This commit updates the `RegisteredType` and `TypeCollection` abstractions to inherently store within them `TraceInfo` used for GC types. This was previously calculated per-instantiation and per-store when modules were instantiated or types were inserted into the store. No consumers of this information currently exist, but the goal of this commit is to enable the next commit to use it. * Remove `ensure_trace_info` methods This commit removes all `ensure_trace_info` methods from all GC collectors, the GC store, etc. The goal of this commit is to accelerate instantiation of modules that use GC by avoiding using the read-write lock on the `TypeRegistry` stored within the engine. As shown in #13822 even in read-only situations this comes with a significant performance penalty. The strategy taken in this commit is to take an alternative route of handling trace information, empowered by the previous commit. Notably trace information is now all available at `Module`-creation time, for example, and need not be re-calculated for each store. The main difficulty is then looking up this trace information at runtime when a GC is performed. This commit implements functionality where `TraceInfos` is repurposed as a cache rather than a storage table. The cache stores where the trace information is located, and then trace information is looked up where it lies at-rest within a `Module` or `RegisteredType`. This means that the first time a type is traced within a store it requires a search to determine where the trace information is located. Right now this involves two locations: * If a store's `gc_host_alloc_types` maps contains the type index, then that's where the trace information is located. * Otherwise a module previously inserted into a store's `ModuleRegistry` must have trace information. The entire registry is searched and each module is consulted to determine if it has trace information for the type index in question. The `TraceInfos` cache is intended to amortize this cost of a lookup. This lookup is additionally mitigated in the copying collector where this is only required for "big structs" where their tracing information can't be stored inline in the object header itself. Overall it's expected that for the copying collector this change has little effect on typical GC performance itself. Additionally overall, however, this eliminates usage of the read/write lock in the `TypeRegistry` entirely during instantiation. Eliminating this lock acquisition was the goal of this commit, and this is expected to help improve parallel instantiation performance of GC-using modules. * Shuffle things around to resolve compile warnings prtest:full * Fix feature gates * Run apt-get installs in a loop * More feature fixes * Cache GC subtype checks per-store (#13860) * Cache GC subtype checks per-store Avoids taking a lock on the engine's type registry. Doesn't really affect single-threaded performance, but for a mid-sized Kotlin component I see performance improve from 4050 RPS to 17680 RPS. * Review comments --------- Co-authored-by: Till Schneidereit <till@tillschneidereit.net> --------- Co-authored-by: crowforkotlin <crowforkotlin@gmail.com> Co-authored-by: Till Schneidereit <till@tillschneidereit.net>
This is the final commit extracted from #13822 with some edits here. Notably:
store/gc.rsNopHasherused for trace info.