Reduce clones of Arc<HostFunc> during instantiation#4051
Merged
Conversation
This commit implements an optimization to help improve concurrently creating instances of a module on many threads simultaneously. One bottleneck to this measured has been the reference count modification on `Arc<HostFunc>`. Each host function stored within a `Linker<T>` is wrapped in an `Arc<HostFunc>` structure, and when any of those host functions are inserted into a store the reference count is incremented. When the store is dropped the reference count is then decremented. This ends up meaning that when a module imports N functions it ends up doing 2N atomic modifications over the lifetime of the instance. For embeddings where the `Linker<T>` is rarely modified but instances are frequently created this can be a surprising bottleneck to creating many instances. A change implemented here is to optimize the instantiation process when using an `InstancePre<T>`. An `InstancePre` serves as an opportunity to take the list of items used to instantiate a module and wrap them all up in an `Arc<[T]>`. Everything is going to get cloned into a `Store<T>` anyway so to optimize this the `Arc<[T]>` is cloned at the top-level and then nothing else is cloned internally. This continues to, however, preserve a strong reference count for all contained items to prevent them from being deallocated. A new variant of `FuncKind` was added for host functions which is effectively stored via `*mut HostFunc`. This variant is unsafe to create and manage and has been documented internally. Performance-wise the overall impact of this change is somewhat minor. It's already a bit esoteric if this atomic increment and decrement are a bottleneck due to the number of concurrent instances being created. In my measurements I've seen that this can reduce instantiation time by up to 10% for a module that imports two dozen functions. For larger modules with more imports this is expected to have a larger win.
Subscribe to Label Actioncc @peterhuene DetailsThis issue or pull request has been labeled: "wasmtime:api"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
cfallin
approved these changes
Apr 19, 2022
cfallin
left a comment
Member
There was a problem hiding this comment.
LGTM; this should be a nice improvement! Appreciate the detailed safety notes here around the raw hostfunc pointers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit implements an optimization to help improve concurrently
creating instances of a module on many threads simultaneously. One
bottleneck to this measured has been the reference count modification on
Arc<HostFunc>. Each host function stored within aLinker<T>iswrapped in an
Arc<HostFunc>structure, and when any of those hostfunctions are inserted into a store the reference count is incremented.
When the store is dropped the reference count is then decremented.
This ends up meaning that when a module imports N functions it ends up
doing 2N atomic modifications over the lifetime of the instance. For
embeddings where the
Linker<T>is rarely modified but instances arefrequently created this can be a surprising bottleneck to creating many
instances.
A change implemented here is to optimize the instantiation process when
using an
InstancePre<T>. AnInstancePreserves as an opportunity totake the list of items used to instantiate a module and wrap them all up
in an
Arc<[T]>. Everything is going to get cloned into aStore<T>anyway so to optimize this the
Arc<[T]>is cloned at the top-level andthen nothing else is cloned internally. This continues to, however,
preserve a strong reference count for all contained items to prevent
them from being deallocated.
A new variant of
FuncKindwas added for host functions which iseffectively stored via
*mut HostFunc. This variant is unsafe to createand manage and has been documented internally.
Performance-wise the overall impact of this change is somewhat minor.
It's already a bit esoteric if this atomic increment and decrement are a
bottleneck due to the number of concurrent instances being created. In
my measurements I've seen that this can reduce instantiation time by up
to 10% for a module that imports two dozen functions. For larger modules
with more imports this is expected to have a larger win.