Skip to content

winch: Epoch-based interruption#9737

Merged
saulecabrera merged 4 commits intobytecodealliance:mainfrom
saulecabrera:winch-epoch
Dec 6, 2024
Merged

winch: Epoch-based interruption#9737
saulecabrera merged 4 commits intobytecodealliance:mainfrom
saulecabrera:winch-epoch

Conversation

@saulecabrera
Copy link
Copy Markdown
Member

Closes #8091

This commit introduces support for epoch interruption to Winch. The heuristics around epoch check emission are identical to Cranelift's except for the fact that the current implementation doesn't introduce a local-based cache for the current epoch deadline. This is an intentional decision given Winch's focus on compilation performance. However, if needed in the future, knobs could be introduced to optionally introduce a local cache at the cost of reduced compilation performance.

@saulecabrera saulecabrera requested review from a team as code owners December 4, 2024 23:55
@saulecabrera saulecabrera requested review from fitzgen and removed request for a team December 4, 2024 23:55
@github-actions github-actions bot added fuzzing Issues related to our fuzzing infrastructure winch Winch issues or pull requests labels Dec 5, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 5, 2024

Subscribe to Label Action

cc @fitzgen, @saulecabrera

Details This issue or pull request has been labeled: "fuzzing", "winch"

Thus the following users have been cc'd because of the following labels:

  • fitzgen: fuzzing
  • saulecabrera: winch

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

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.

Looks great! Just a couple nitpicks inline.

Also I know that most of the testing will be covered by fuzzing and the existing epoch tests, but it may make sense to throw in a single disas test with epochs enabled as well that contains a some ifs, blocks, and a loop just to show that we insert epoch checks only on the loop headers and body prologue. Could also be used to see that deadlines are actually cached, if we ever get around to doing that here.

saulecabrera added a commit to saulecabrera/wasmtime that referenced this pull request Dec 6, 2024
This commits ports the suggestions made in bytecodealliance#9737
to fuel checks.

Namely

* Prefer the usage of `*_reg` over `*_var`
* Add a couple of disassembly tests
@saulecabrera
Copy link
Copy Markdown
Member Author

@fitzgen agreed with all the suggestions,thanks! I've also opened #9737, which applies the suggestions in this PR to fuel checks.

saulecabrera added a commit to saulecabrera/wasmtime that referenced this pull request Dec 6, 2024
This commits ports the suggestions made in bytecodealliance#9737
to fuel checks.

Namely

* Prefer the usage of `*_reg` over `*_var`
* Add a couple of disassembly tests
Closes bytecodealliance#8091

This commit introduces support for epoch interruption to Winch. The
heuristics around epoch check emission are identical to Cranelift's
except for the fact that the current implementation doesn't introduce
a local-based cache for the current epoch deadline. This is an
intentional decision given Winch's focus on compilation performance.
However, if needed in the future, knobs could be introduced to
optionally introduce a local cache at the cost of reduced compilation
performance.
@saulecabrera saulecabrera added this pull request to the merge queue Dec 6, 2024
Merged via the queue into bytecodealliance:main with commit 9b9824f Dec 6, 2024
@saulecabrera saulecabrera deleted the winch-epoch branch December 6, 2024 17:37
saulecabrera added a commit to saulecabrera/wasmtime that referenced this pull request Dec 6, 2024
This commits ports the suggestions made in bytecodealliance#9737
to fuel checks.

Namely

* Prefer the usage of `*_reg` over `*_var`
* Add a couple of disassembly tests
github-merge-queue bot pushed a commit that referenced this pull request Dec 6, 2024
This commits ports the suggestions made in #9737
to fuel checks.

Namely

* Prefer the usage of `*_reg` over `*_var`
* Add a couple of disassembly tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fuzzing Issues related to our fuzzing infrastructure winch Winch issues or pull requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

winch: Epoch based interruption

2 participants