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

Allow host functions to call Wasm functions #590

Merged
merged 23 commits into from
Dec 23, 2022
Merged

Conversation

Robbepop
Copy link
Member

@Robbepop Robbepop commented Dec 7, 2022

Closes #572

@paritytech-cicd-pr
Copy link

paritytech-cicd-pr commented Dec 7, 2022

BENCHMARKS

NATIVEWASMTIME
BENCHMARKMASTERPRDIFFMASTERPRDIFFWASMTIME OVERHEAD
execute/
bare_call_0
1.05ms 1.63ms 🔴 54.66% 1.02ms 1.26ms 🔴 23.91% 🟢 -22%
execute/
bare_call_0/typed
539.14µs 964.88µs 🔴 77.58% 462.54µs 696.41µs 🔴 50.60% 🟢 -28%
execute/
bare_call_1
1.05ms 1.66ms 🔴 57.41% 1.25ms 1.60ms 🔴 27.71% 🟢 -3%
execute/
bare_call_16
2.11ms 2.76ms 🔴 30.22% 4.89ms 4.81ms 🔴 -1.50% 🟡 74%
execute/
bare_call_16/typed
1.51ms 1.86ms 🔴 23.10% 2.05ms 2.36ms 🔴 14.74% 🟢 27%
execute/
bare_call_1/typed
610.89µs 1.11ms 🔴 79.56% 771.01µs 1.12ms 🔴 45.98% 🟢 1%
execute/
bare_call_4
1.20ms 1.81ms 🔴 50.90% 2.01ms 2.30ms 🔴 14.29% 🟢 27%
execute/
bare_call_4/typed
641.85µs 1.13ms 🔴 77.12% 865.13µs 1.19ms 🔴 37.31% 🟢 5%
execute/
br_table
630.29µs 1.09ms 🔴 72.94% 995.87µs 1.37ms 🔴 37.69% 🟢 26%
execute/
count_until
651.17µs 739.93µs 🔴 13.66% 2.17ms 2.16ms ⚪ -0.17% 🔴 192%
execute/
factorial_iterative
309.40µs 321.63µs 🔴 4.11% 919.15µs 909.27µs 🟢 -1.18% 🔴 183%
execute/
factorial_recursive
610.41µs 643.57µs 🔴 5.38% 1.33ms 1.88ms 🔴 42.40% 🔴 193%
execute/
fib_iterative
1.54ms 1.43ms 🟢 -7.29% 4.79ms 4.98ms 🔴 4.00% 🔴 247%
execute/
fib_recursive
5.75ms 5.72ms ⚪ -0.53% 12.21ms 12.04ms 🟢 -1.35% 🔴 110%
execute/
global_bump
955.29µs 1.04ms 🔴 8.91% 3.32ms 3.16ms 🟢 -4.86% 🔴 204%
execute/
global_const
801.34µs 831.08µs ⚪ 3.50% 2.49ms 2.48ms ⚪ -0.74% 🔴 198%
execute/
host_calls
28.92µs 28.57µs ⚪ -1.16% 42.42µs 44.01µs 🔴 3.83% 🟡 54%
execute/
memory_fill
1.30ms 1.39ms 🔴 6.70% 4.20ms 4.27ms 🔴 1.79% 🔴 206%
execute/
memory_sum
1.33ms 1.28ms 🟢 -3.87% 4.34ms 4.20ms 🟢 -3.45% 🔴 229%
execute/
memory_vec_add
2.74ms 2.78ms 🔴 1.24% 8.84ms 8.72ms 🟢 -1.54% 🔴 214%
execute/
recursive_is_even
1.13ms 1.11ms 🟢 -1.49% 2.20ms 2.25ms 🔴 2.20% 🔴 103%
execute/
recursive_ok
143.26µs 142.83µs ⚪ -0.41% 304.60µs 315.09µs 🔴 3.44% 🔴 121%
execute/
recursive_scan
178.83µs 180.26µs ⚪ 0.68% 390.02µs 419.47µs 🔴 7.50% 🔴 133%
execute/
recursive_trap
14.12µs 13.98µs 🟢 -1.19% 30.47µs 31.15µs 🔴 2.08% 🔴 123%
execute/
regex_redux
544.79µs 549.26µs ⚪ 0.78% 1.55ms 1.56ms ⚪ 0.35% 🔴 183%
execute/
rev_complement
522.17µs 518.09µs ⚪ -0.94% 1.55ms 1.58ms 🔴 2.09% 🔴 206%
execute/
tiny_keccak
369.79µs 375.07µs 🔴 1.37% 1.29ms 1.26ms 🟢 -2.10% 🔴 237%
execute/
trunc_f2i
920.76µs 911.64µs ⚪ -1.07% 2.64ms 2.62ms ⚪ -0.49% 🔴 188%
instantiate/
wasm_kernel
58.27µs 60.07µs 🔴 5.06% 90.72µs 72.50µs 🟢 -20.31% 🟢 21%
translate/
erc1155
206.56µs 209.08µs 🔴 1.56% 405.39µs 422.06µs 🔴 3.58% 🔴 102%
translate/
erc20
100.28µs 104.08µs 🔴 3.96% 198.89µs 207.51µs 🔴 4.51% 🟡 99%
translate/
erc721
144.87µs 148.03µs 🔴 2.26% 289.17µs 300.07µs 🔴 3.78% 🔴 103%
translate/
spidermonkey
0.00ns 0.00ns ⚪ -0.06% 0.00ns 0.00ns 🔴 4.31% 🟢 0%
translate/
wasm_kernel
3.77ms 3.86ms 🔴 2.19% 7.67ms 7.87ms 🔴 2.80% 🔴 104%

Link to pipeline

@codecov-commenter
Copy link

codecov-commenter commented Dec 7, 2022

Codecov Report

Merging #590 (c453d2a) into master (f82ed77) will increase coverage by 0.06%.
The diff coverage is 86.30%.

@@            Coverage Diff             @@
##           master     #590      +/-   ##
==========================================
+ Coverage   80.70%   80.76%   +0.06%     
==========================================
  Files          79       80       +1     
  Lines        6364     6416      +52     
==========================================
+ Hits         5136     5182      +46     
- Misses       1228     1234       +6     
Impacted Files Coverage Δ
crates/wasmi/src/engine/config.rs 65.51% <40.00%> (-5.32%) ⬇️
crates/wasmi/src/engine/mod.rs 85.21% <86.27%> (+1.68%) ⬆️
crates/wasmi/tests/e2e/v1/host_calls_wasm.rs 100.00% <100.00%> (ø)

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

@Robbepop Robbepop changed the title WIP: Allow host functions to call Wasm functions Allow host functions to call Wasm functions Dec 22, 2022
@Robbepop Robbepop marked this pull request as ready for review December 22, 2022 18:56
@Robbepop
Copy link
Member Author

The PR is no longer in WIP phase and is ready to be tested.

One remaining thing left to do is to implement stack limits that are based on the offsets of parent call stacks to properly guard against requiring more stack space than has been preconfigured.
For example, with the current implementation it is possible to circumvent call and value space limitations set in StackLimits via Config by calling host and Wasm functions to reset the stacks in between.

When this issue is resolved this PR is good to go.

@Robbepop
Copy link
Member Author

Robbepop commented Dec 23, 2022

The PR is no longer in WIP phase and is ready to be tested.

One remaining thing left to do is to implement stack limits that are based on the offsets of parent call stacks to properly guard against requiring more stack space than has been preconfigured. For example, with the current implementation it is possible to circumvent call and value space limitations set in StackLimits via Config by calling host and Wasm functions to reset the stacks in between.

When this issue is resolved this PR is good to go.

Since this fix is non trivial and also not super important I decided to do that in another PR and merge this PR as is.

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.

Allow called host functions to execute Wasm functions via the wasmi::Engine
3 participants