Stream: git-wasmtime

Topic: wasmtime / PR #3741 Don't copy `VMBuiltinFunctionsArray` ...

Wasmtime GitHub notifications bot (Jan 28 2022 at 21:24):

alexcrichton opened PR #3741 from builtins-nocopy to main:

This is another PR along the lines of "let's squeeze all possible
performance we can out of instantiation". Before this PR we would copy,
by value, the contents of VMBuiltinFunctionsArray into each
VMContext allocated. This array of function pointers is modestly-sized
but growing over time as we add various intrinsics. Additionally it's
the exact same for all VMContext allocations.

This PR attempts to speed up instantiation slightly by instead storing
an indirection to the function array. This means that calling a builtin
intrinsic is a tad bit slower since it requires two loads instead of one
(one to get the base pointer, another to get the actual address).
Otherwise though VMContext initialization is now simply setting one
pointer instead of doing a memcpy from one location to another.

With some macro-magic this commit also replaces the previous
implementation with one that's more const-friendly which also gets us
compile-time type-checks of libcalls as well as compile-time
verification that all libcalls are defined.

Overall, as with #3739, the win is very modest here. Locally I measured
a speedup from 1.9us to 1.7us taken to instantiate an empty module with
one function. While small at these scales it's still a 10% improvement!

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Jan 28 2022 at 21:36):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Jan 28 2022 at 21:37):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jan 28 2022 at 21:37):

cfallin created PR review comment:

Rather than new(), can we call this static_instance() or something like that? As written it was somewhat concerning to read VMBuiltinFunctionsArray::new() in the initialization path -- it looked like an allocation of an owned thing.

Wasmtime GitHub notifications bot (Jan 28 2022 at 21:37):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jan 28 2022 at 21:40):

alexcrichton updated PR #3741 from builtins-nocopy to main.

Wasmtime GitHub notifications bot (Jan 28 2022 at 21:40):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jan 28 2022 at 21:40):

alexcrichton created PR review comment:

Aha looks like I can do one better and have const INIT: VMBuiltinFunctionsArray = ...!

Wasmtime GitHub notifications bot (Jan 28 2022 at 22:04):

bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Jan 28 2022 at 22:04):

bjorn3 created PR review comment:

This function doesn't return an actual pointer, right?

Wasmtime GitHub notifications bot (Jan 28 2022 at 22:24):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jan 28 2022 at 22:24):

alexcrichton created PR review comment:

Correct, this is just how the type-checking with the macro worked out.

Wasmtime GitHub notifications bot (Jan 28 2022 at 22:24):

alexcrichton merged PR #3741.

Last updated: Nov 22 2024 at 16:03 UTC