Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wasmtime: add EngineWeak which has ::upgrade, created by Engine::weak #7797

Merged
merged 2 commits into from
Jan 19, 2024

Conversation

pchickey
Copy link
Contributor

Engine is, internally, just an Arc, so this is trivial to implement - EngineWeak is a Weak.

This behavior is desirable because Engine::increment_epoch typically happens in a worker thread, which in turn requires additional machinery to discard the Engine once it is no longer needed. If instead the worker thread holds an EngineWeak, it can stop ticking when all consumers of the Engine have dropped it. This has been documented as a suggestion in increment_epoch.

For an example of additional machinery which is simplified by this change, see https://github.com/fastly/Viceroy/blob/25edee0700ec0b20b1b56db0a3a8d6f090397b3a/lib/src/execute.rs#L108-L116)

Engine is, internally, just an Arc<EngineInner>, so this is trivial to implement -
EngineWeak is a Weak<EngineInner>.

This behavior is desirable because `Engine::increment_epoch` typically
happens in a worker thread, which in turn requires additional machinery
to discard the `Engine` once it is no longer needed. If instead the
worker thread holds an `EngineWeak`, it can stop ticking when all
consumers of the `Engine` have dropped it. This has been documented as a
suggestion in `increment_epoch`.

For an example of additional machinery which is simplified by this change, see https://github.com/fastly/Viceroy/blob/25edee0700ec0b20b1b56db0a3a8d6f090397b3a/lib/src/execute.rs#L108-L116)

Co-authored-by: John Van Enk <vanenkj@gmail.com>
@pchickey pchickey requested a review from a team as a code owner January 19, 2024 18:18
@pchickey pchickey requested review from fitzgen and removed request for a team January 19, 2024 18:18
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sanity test for when it is expected to be upgradable and expected to not be?

@pchickey pchickey enabled auto-merge January 19, 2024 18:50
@pchickey pchickey added this pull request to the merge queue Jan 19, 2024
Merged via the queue into main with commit c736c71 Jan 19, 2024
20 checks passed
@pchickey pchickey deleted the pch/engine_weak branch January 19, 2024 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants