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

Resumable Function Calls #598

Merged
merged 42 commits into from
Jan 4, 2023
Merged

Resumable Function Calls #598

merged 42 commits into from
Jan 4, 2023

Conversation

Robbepop
Copy link
Member

@Robbepop Robbepop commented Dec 31, 2022

Closes #537.

ToDo

  • Implement EngineExecutor::resume_func.
  • Add tests for new resumable function execution feature.
  • Write missing docs for methods.

This initial commit introduces the initial API surface and design of the resumable calls API for the new `wasmi` engine.

The implementation of the underlying resumable calls has not yet been done but should be fairly straight forward with the existing design.
@paritytech-cicd-pr
Copy link

paritytech-cicd-pr commented Dec 31, 2022

BENCHMARKS

NATIVEWASMTIME
BENCHMARKMASTERPRDIFFMASTERPRDIFFWASMTIME OVERHEAD
execute/
bare_call_0
1.31ms 1.33ms 🔴 2.17% 1.17ms 1.12ms 🔴 -4.11% 🟢 -16%
execute/
bare_call_0/typed
958.53µs 979.51µs 🔴 2.26% 651.02µs 723.44µs 🔴 11.09% 🟢 -26%
execute/
bare_call_1
1.40ms 1.41ms 🔴 1.53% 1.41ms 1.34ms 🔴 -4.81% 🟢 -5%
execute/
bare_call_16
2.36ms 2.45ms 🔴 3.93% 3.86ms 4.18ms 🔴 8.35% 🟡 71%
execute/
bare_call_16/typed
1.56ms 1.59ms 🔴 1.68% 2.03ms 2.37ms 🔴 16.71% 🟢 49%
execute/
bare_call_1/typed
1.05ms 1.08ms 🔴 2.49% 1.04ms 1.09ms 🔴 4.50% 🟢 0%
execute/
bare_call_4
1.47ms 1.69ms 🔴 14.78% 1.89ms 1.97ms 🔴 4.10% 🟢 17%
execute/
bare_call_4/typed
1.06ms 1.07ms ⚪ 0.36% 1.06ms 1.17ms 🔴 11.13% 🟢 9%
execute/
br_table
1.08ms 1.08ms ⚪ -0.08% 1.30ms 1.30ms ⚪ 0.03% 🟢 20%
execute/
count_until
711.38µs 646.76µs 🟢 -9.27% 2.16ms 2.16ms ⚪ -0.45% 🔴 234%
execute/
factorial_iterative
321.07µs 327.17µs 🔴 1.90% 907.25µs 882.35µs 🟢 -2.84% 🔴 170%
execute/
factorial_recursive
628.11µs 638.28µs 🔴 1.63% 1.36ms 1.55ms 🔴 13.84% 🔴 143%
execute/
fib_iterative
1.43ms 1.43ms ⚪ -0.12% 4.97ms 4.66ms 🟢 -6.53% 🔴 225%
execute/
fib_recursive
5.84ms 5.75ms ⚪ -1.19% 12.02ms 13.16ms 🔴 9.35% 🔴 129%
execute/
global_bump
921.36µs 982.40µs 🔴 6.64% 3.20ms 3.21ms ⚪ 0.11% 🔴 227%
execute/
global_const
717.76µs 717.78µs ⚪ 0.03% 2.47ms 2.51ms ⚪ 1.42% 🔴 249%
execute/
host_calls
28.99µs 29.38µs 🔴 1.51% 46.98µs 42.30µs 🟢 -9.91% 🟢 44%
execute/
memory_fill
1.39ms 1.31ms 🟢 -6.32% 4.23ms 4.12ms ⚪ -2.38% 🔴 216%
execute/
memory_sum
1.30ms 1.31ms ⚪ 0.67% 4.24ms 4.21ms ⚪ -0.45% 🔴 221%
execute/
memory_vec_add
2.72ms 2.66ms 🟢 -2.29% 8.84ms 8.56ms 🟢 -3.17% 🔴 222%
execute/
recursive_is_even
1.12ms 1.15ms 🔴 2.77% 2.20ms 2.37ms 🔴 7.61% 🔴 106%
execute/
recursive_ok
144.33µs 145.52µs ⚪ 0.63% 306.09µs 333.19µs 🔴 8.77% 🔴 129%
execute/
recursive_scan
177.28µs 179.58µs 🔴 1.34% 396.69µs 438.16µs 🔴 10.43% 🔴 144%
execute/
recursive_trap
13.98µs 14.70µs 🔴 4.65% 30.33µs 31.80µs 🔴 4.63% 🔴 116%
execute/
regex_redux
554.10µs 550.06µs ⚪ -0.70% 1.60ms 1.56ms 🟢 -2.90% 🔴 183%
execute/
rev_complement
514.49µs 510.53µs ⚪ -0.80% 1.57ms 1.52ms 🟢 -2.78% 🔴 198%
execute/
tiny_keccak
368.80µs 371.52µs ⚪ 0.81% 1.27ms 1.22ms 🟢 -3.67% 🔴 230%
execute/
trunc_f2i
912.71µs 913.81µs ⚪ 0.19% 2.63ms 2.64ms ⚪ 0.24% 🔴 189%
instantiate/
wasm_kernel
60.64µs 61.84µs ⚪ 3.30% 70.70µs 72.04µs ⚪ 1.21% 🟢 16%
translate/
erc1155
208.89µs 209.59µs ⚪ 0.44% 405.95µs 403.35µs ⚪ -0.75% 🟡 92%
translate/
erc20
101.61µs 102.80µs 🔴 1.42% 197.98µs 199.00µs ⚪ 0.76% 🟡 94%
translate/
erc721
146.75µs 148.55µs ⚪ 1.08% 289.19µs 287.05µs ⚪ -0.93% 🟡 93%
translate/
spidermonkey
0.00ns 0.00ns 🔴 1.77% 0.00ns 0.00ns ⚪ -0.75% 🟢 0%
translate/
wasm_kernel
3.83ms 3.84ms ⚪ 0.14% 7.74ms 7.68ms ⚪ -0.96% 🟡 100%

Link to pipeline

@codecov-commenter
Copy link

codecov-commenter commented Dec 31, 2022

Codecov Report

Merging #598 (7ed70a7) into master (3c62031) will increase coverage by 0.26%.
The diff coverage is 84.29%.

@@            Coverage Diff             @@
##           master     #598      +/-   ##
==========================================
+ Coverage   80.73%   81.00%   +0.26%     
==========================================
  Files          80       82       +2     
  Lines        6448     6628     +180     
==========================================
+ Hits         5206     5369     +163     
- Misses       1242     1259      +17     
Impacted Files Coverage Δ
crates/wasmi/src/engine/stack/values/vref.rs 95.71% <0.00%> (-2.82%) ⬇️
crates/wasmi/src/func/mod.rs 80.23% <66.66%> (-1.02%) ⬇️
crates/wasmi/src/engine/mod.rs 82.72% <79.31%> (-2.50%) ⬇️
crates/wasmi/tests/e2e/v1/resumable_call.rs 87.93% <87.93%> (ø)
crates/wasmi/src/engine/resumable.rs 96.00% <96.00%> (ø)
crates/wasmi/src/engine/stack/mod.rs 82.05% <100.00%> (+2.34%) ⬆️
crates/wasmi/src/engine/stack/values/mod.rs 85.07% <100.00%> (+0.94%) ⬆️
crates/wasmi/src/func/into_func.rs 95.45% <0.00%> (+2.27%) ⬆️
crates/wasmi/src/engine/stack/frames.rs 100.00% <0.00%> (+8.00%) ⬆️
crates/core/src/trap.rs 56.60% <0.00%> (+15.09%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

The host_func API is more powerful, yet decreases the size of the ResumableInvocation type.
This naive implementation introduces a lot of duplicated code in the engine that needs to be resolved before we merge this PR.
Robbepop added a commit that referenced this pull request Jan 3, 2023
Robbepop added a commit that referenced this pull request Jan 3, 2023
refactor FuncType verification

Carved out from PR #598.
EngineResources is a field in EngineExecutor and therefore does no longer need to be passed.
This experimental commit deduplicates logic between resumable and non-resumable calls in the Engine implementation. This is experimental as this might regress performance of non-resumable calls which would be pretty bad.
Using new led to panics at runtime since new constructor checks its inputs which we do not want or need in this particular instance.
The bug was that host function inputs were not properly dropped from the stack before feeding in the arguments that were supposed to replace the missing host results due to host trapping instead. Therefore the stack was getting bigger and bigger with every resumption.
We simply use the provided ctx parameter with which it is also possible to query the engine.
@Robbepop Robbepop marked this pull request as ready for review January 4, 2023 12:15
@Robbepop Robbepop changed the title WIP: Resumable Function Calls Resumable Function Calls Jan 4, 2023
This name fits better since on the surface it has nothing to do with resumable functions. However, tagging the origin of the trap is important for resumable calls.
It is incorrect since host functions are technically able to return Wasm trap codes.
Notifying users that this API is non standard and might not be available in other Wasm engines.
@Robbepop Robbepop merged commit 829ae5b into master Jan 4, 2023
@Robbepop Robbepop deleted the rf-resumable-funcs branch January 4, 2023 13:50
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.

Re-Implement resumable functions in wasmi
3 participants