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 ofVMBuiltinFunctionsArray
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 allVMContext
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 thoughVMContext
initialization is now simply setting one
pointer instead of doing amemcpy
from one location to another.With some macro-magic this commit also replaces the previous
implementation with one that's moreconst
-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.
-->
fitzgen submitted PR review.
cfallin submitted PR review.
cfallin created PR review comment:
Rather than
new()
, can we call thisstatic_instance()
or something like that? As written it was somewhat concerning to readVMBuiltinFunctionsArray::new()
in the initialization path -- it looked like an allocation of an owned thing.
cfallin submitted PR review.
alexcrichton updated PR #3741 from builtins-nocopy
to main
.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Aha looks like I can do one better and have
const INIT: VMBuiltinFunctionsArray = ...
!
bjorn3 submitted PR review.
bjorn3 created PR review comment:
This function doesn't return an actual pointer, right?
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Correct, this is just how the type-checking with the macro worked out.
alexcrichton merged PR #3741.
Last updated: Dec 23 2024 at 13:07 UTC