Skip to content

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

Merged
pchickey merged 2 commits into
mainfrom
pch/engine_weak
Jan 19, 2024
Merged

wasmtime: add EngineWeak which has ::upgrade, created by Engine::weak#7797
pchickey merged 2 commits into
mainfrom
pch/engine_weak

Conversation

@pchickey
Copy link
Copy Markdown
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
Copy Markdown
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
@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