Inline context.{get,set} in components#13194
Open
alexcrichton wants to merge 2 commits intobytecodealliance:mainfrom
Open
Inline context.{get,set} in components#13194alexcrichton wants to merge 2 commits intobytecodealliance:mainfrom
context.{get,set} in components#13194alexcrichton wants to merge 2 commits intobytecodealliance:mainfrom
Conversation
This commit reimplements the `context.{get,set}` intrinsics in the
component model, introduced in the component-model-async and
component-model-threading proposals. The intent of these intrinsics in
WASIp3, for example, are intended to replace the `global`s used for the
stack pointer and TLS base in previous modules, for example. The
implementation of loading from a `global` is a single load instruction,
whereas the previous implementation of `context.get` was a full libcall,
which is significantly more expensive. The goal of this PR is to ensure
that the transition to using `context.get` and `context.set` for
high-performance uses retains the same performance as the WASIp2
constructs.
Specifically the storage for `context.{get,set}` slots have been moved
into the `VMStoreContext` structure which has a known layout to compiled
code. There still remains storage within each `GuestThread` because
there's only one store, and the idea is that whenever threads are
switched between the switch operation is slightly more expensive now
where it has to update and maintain the state in the store. The
rationale for this is that it'll be far more often that these values are
accessed rather than threads being swapped between.
The implementation chosen in this commit is to model the
`context.{get,set}` intrinsics as `UnsafeIntrinsic`s. This is a bit of a
shoehorn where they're not actually unsafe, but all of the plumbing and
support for `UnsafeIntrinsic` is effectively exactly what these want. To
avoid duplicating lots of infrastructure that's where these now reside.
The `concurrent.rs` implementation has been updated to save/restore
context from the store, and this additionally updates a few other switch
points to ensure that the store never switches away or to a deleted
thread. This niche situation happened in a few scenarios with no impact
from before, but with the switching implementation having to access
threads it became load-bearing that these must be valid.
The end result is that with `-Cinlining` the `context.{get,set}`
instructions are two instructions instead of a full libcall. One
instruction is loading `VMStoreContext`, which is GVN-able and
hoist-able, while the other is the actual load/store. This is the same
as the performance of the stack pointer being in an imported global,
for example.
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 reimplements the
context.{get,set}intrinsics in the component model, introduced in the component-model-async and component-model-threading proposals. The intent of these intrinsics in WASIp3, for example, are intended to replace theglobals used for the stack pointer and TLS base in previous modules, for example. The implementation of loading from aglobalis a single load instruction, whereas the previous implementation ofcontext.getwas a full libcall, which is significantly more expensive. The goal of this PR is to ensure that the transition to usingcontext.getandcontext.setfor high-performance uses retains the same performance as the WASIp2 constructs.Specifically the storage for
context.{get,set}slots have been moved into theVMStoreContextstructure which has a known layout to compiled code. There still remains storage within eachGuestThreadbecause there's only one store, and the idea is that whenever threads are switched between the switch operation is slightly more expensive now where it has to update and maintain the state in the store. The rationale for this is that it'll be far more often that these values are accessed rather than threads being swapped between.The implementation chosen in this commit is to model the
context.{get,set}intrinsics asUnsafeIntrinsics. This is a bit of a shoehorn where they're not actually unsafe, but all of the plumbing and support forUnsafeIntrinsicis effectively exactly what these want. To avoid duplicating lots of infrastructure that's where these now reside.The
concurrent.rsimplementation has been updated to save/restore context from the store, and this additionally updates a few other switch points to ensure that the store never switches away or to a deleted thread. This niche situation happened in a few scenarios with no impact from before, but with the switching implementation having to access threads it became load-bearing that these must be valid.The end result is that with
-Cinliningthecontext.{get,set}instructions are two instructions instead of a full libcall. One instruction is loadingVMStoreContext, which is GVN-able and hoist-able, while the other is the actual load/store. This is the same as the performance of the stack pointer being in an imported global, for example.