Stream: git-wasmtime

Topic: wasmtime / PR #4204 Add a first-class `StoreId` type to W...

Wasmtime GitHub notifications bot (Jun 01 2022 at 18:46):

alexcrichton opened PR #4204 from store-id to main:

This commit adds a StoreId type to uniquely identify a store
internally within Wasmtime. This hasn't been created previously as it
was never really needed but I've run across a case for its usage in the
component model so I've gone ahead and split out a commit to add this type.

While I was here in this file I opted to improve some other
miscellaneous things as well:

• Notes were added to the Index impls that unchecked indexing could be
  used in theory if we ever need it one day.

• The check in Index for the same store should now be a bit lighter on
  codegen where instead of having a panic!() in the codegen for each
  Index there's now an out-of-line version which is #[cold]. This
  should improve codegen as calling a function with no arguments is
  slighly more efficient than calling the panic macro with one string argument.

• An assert! guarded with a cfg(debug_assertions) was changed to a
  debug_assert!.

• Allocation of a StoreId was refactored to a method on the StoreId
  itself.

<!--

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 (Jun 01 2022 at 18:47):

alexcrichton requested fitzgen for a review on PR #4204.

Wasmtime GitHub notifications bot (Jun 01 2022 at 18:57):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jun 01 2022 at 18:57):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jun 01 2022 at 18:57):

cfallin created PR review comment:

s/stores/store/

Wasmtime GitHub notifications bot (Jun 01 2022 at 18:57):

cfallin created PR review comment:

I think this can be a Relaxed fetch_add, because the sequence in which IDs are allocated doesn't matter (every ID is as good as any other and is opaque), so there's no need to synchronize this allocation step with respect to any other loads/stores. I wrote a similar justification over here for CompiledModuleId, I guess we could just point to that as justification.

Wasmtime GitHub notifications bot (Jun 01 2022 at 18:57):

cfallin created PR review comment:

"... inserted into a store" --> add "and stores are never shrunk, so an index once valid is always valid", just for extra clarity?

Wasmtime GitHub notifications bot (Jun 01 2022 at 19:02):

alexcrichton updated PR #4204 from store-id to main.

Wasmtime GitHub notifications bot (Jun 01 2022 at 19:13):

alexcrichton updated PR #4204 from store-id to main.

Wasmtime GitHub notifications bot (Jun 01 2022 at 19:46):

alexcrichton merged PR #4204.

Last updated: Nov 22 2024 at 17:03 UTC