Skip to content

Reduce clones of Arc<HostFunc> during instantiation#4051

Merged
alexcrichton merged 1 commit into
bytecodealliance:mainfrom
alexcrichton:less-arc-clone
Apr 19, 2022
Merged

Reduce clones of Arc<HostFunc> during instantiation#4051
alexcrichton merged 1 commit into
bytecodealliance:mainfrom
alexcrichton:less-arc-clone

Conversation

@alexcrichton

Copy link
Copy Markdown
Member

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.

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.
@github-actions github-actions Bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Apr 19, 2022
@github-actions

Copy link
Copy Markdown

Subscribe to Label Action

cc @peterhuene

Details This issue or pull request has been labeled: "wasmtime:api"

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

  • peterhuene: wasmtime:api

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

Learn more.

@cfallin cfallin left a comment

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.

LGTM; this should be a nice improvement! Appreciate the detailed safety notes here around the raw hostfunc pointers.

@alexcrichton
alexcrichton merged commit 3394c2b into bytecodealliance:main Apr 19, 2022
@alexcrichton
alexcrichton deleted the less-arc-clone branch April 19, 2022 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wasmtime:api Related to the API of the `wasmtime` crate itself

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants