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

Update wasm3 to 0.5.0 #739

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Update wasm3 to 0.5.0 #739

wants to merge 15 commits into from

Conversation

axic
Copy link
Member

@axic axic commented Feb 22, 2021

We still need a patch for the start function, see wasm3/wasm3#202. And also a memory leak issue: wasm3/wasm3#203.

At least we could get rid of wasm3/wasm3#130, wasm3/wasm3#129, and wasm3/wasm3#145.

@axic axic marked this pull request as draft February 22, 2021 23:42
@codecov
Copy link

codecov bot commented Feb 22, 2021

Codecov Report

Merging #739 (91bab22) into master (3581284) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 91bab22 differs from pull request most recent head 8761680. Consider uploading reports for the commit 8761680 to get more accurate results

@@            Coverage Diff             @@
##           master     #739      +/-   ##
==========================================
- Coverage   99.27%   99.27%   -0.01%     
==========================================
  Files          88       88              
  Lines       13296    13158     -138     
==========================================
- Hits        13200    13062     -138     
  Misses         96       96              
Flag Coverage Δ
rust 98.47% <0.00%> (-0.01%) ⬇️
spectests 90.00% <0.00%> (+0.07%) ⬆️
unittests 99.21% <0.00%> (-0.01%) ⬇️
unittests-32 99.31% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/fizzy/capi.cpp 95.41% <0.00%> (-0.28%) ⬇️
lib/fizzy/execute.cpp 99.29% <0.00%> (-0.01%) ⬇️
bindings/rust/src/lib.rs 98.76% <0.00%> (-0.01%) ⬇️
lib/fizzy/execute.hpp 100.00% <0.00%> (ø)
lib/fizzy/instructions.cpp 100.00% <0.00%> (ø)
lib/fizzy/execution_context.hpp 100.00% <0.00%> (ø)
test/unittests/execute_test.cpp 100.00% <0.00%> (ø)
test/unittests/capi_execute_test.cpp 100.00% <0.00%> (ø)
test/unittests/execute_floating_point_test.cpp 99.71% <0.00%> (+<0.01%) ⬆️

cmake/ProjectWasm3.cmake Outdated Show resolved Hide resolved
@axic
Copy link
Member Author

axic commented Feb 24, 2021

This is not ready to merge, as I would wait for a wasm3 release, but @chfast @gumb0 I think it is ready for review.

test/utils/wasm3_engine.cpp Outdated Show resolved Hide resolved
test/utils/wasm3_engine.cpp Outdated Show resolved Hide resolved
test/utils/wasm3_engine.cpp Outdated Show resolved Hide resolved
test/utils/wasm3_engine.cpp Outdated Show resolved Hide resolved
if (result == m3Err_none)
return {false, ret_valid ? ret_value : std::optional<uint64_t>{}};

std::vector<const void*> argPtrs;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you want ultimate speed you can use stack-allocated fixed-size array. Just bail out if too much arguments provided.

Copy link
Member Author

Choose a reason for hiding this comment

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

fizzy/test/utils/wasm3_engine.cpp:147:24: error: variable length arrays are a C99 feature [-Werror,-Wvla-extension]
    const void* argPtrs[args.size()];

Or not sure what you've meant?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah you meant like allocating an array with like 32 items, and fail if args.size() is larger.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if this makes a lot of speed difference, but pushed a version.

test/utils/wasm3_engine.cpp Outdated Show resolved Hide resolved
test/utils/wasm3_engine.cpp Outdated Show resolved Hide resolved
test/utils/wasm_engine.hpp Outdated Show resolved Hide resolved
test/utils/wasm_engine.cpp Outdated Show resolved Hide resolved
@axic axic changed the title Update wasm3 to newer version Update wasm3 to 0.4.9 Mar 12, 2021
@axic axic marked this pull request as ready for review March 25, 2021 00:49
@axic
Copy link
Member Author

axic commented Apr 9, 2021

My plan is we release 0.8.0, then merge this and release 0.8.1. Then it is simple to show benchmarks for both, if needed.

const uint32_t offset = static_cast<uint32_t>(stack[0]);
const uint32_t length = static_cast<uint32_t>(stack[1]);
const uint32_t offset = static_cast<uint32_t>(stack[1]);
const uint32_t length = static_cast<uint32_t>(stack[2]);
stack[0] = fizzy::test::adler32({reinterpret_cast<uint8_t*>(mem) + offset, length});
Copy link
Member Author

Choose a reason for hiding this comment

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

The stack order has changed. Now the return value is on the bottom.

@axic
Copy link
Member Author

axic commented Mar 3, 2023

Merging this is still blocked on benchmarks running out of stack space or something:

23/23 Test  #5: fizzy/smoketests/bench/benchmarks .....................Subprocess aborted***Exception:   0.32 sec
2023-03-03T00:12:35+00:00
Running /home/builder/build/bin/fizzy-bench
Run on (36 X 3203.04 MHz CPU s)
CPU Caches:
  L1 Data 32 KiB (x18)
  L1 Instruction 32 KiB (x18)
  L2 Unified 1024 KiB (x18)
  L3 Unified 25344 KiB (x1)
Load Average: 13.59, 16.22, 17.43
***WARNING*** Library was built as DEBUG. Timings may be affected.
-------------------------------------------------------------------------------------------------------------
Benchmark                                                   Time             CPU   Iterations UserCounters...
-------------------------------------------------------------------------------------------------------------
fizzy/parse/arith                                        51.2 us         51.2 us          333 rate=1092.81k/s size=56
fizzyc/parse/arith                                       30.2 us         30.2 us          443 rate=1.8543M/s size=56
 wabt/parse/arith                                        44.5 us         44.5 us          312 rate=1.25833M/s size=56
wasm3/parse/arith                                        1.15 us         1.15 us        12107 rate=48.6236M/s size=56
fizzy/instantiate/arith                                  36.7 us         36.7 us          385
fizzyc/instantiate/arith                                 45.1 us         45.1 us          354
 wabt/instantiate/arith                                  80.4 us         79.9 us          137
wasm3/instantiate/arith                                  4.04 us         4.04 us         3286
fizzy/execute/arith/addition                            0.630 us        0.630 us        26441
fizzyc/execute/arith/addition                           0.524 us        0.524 us        27211
 wabt/execute/arith/addition                             3.45 us         3.44 us         4017
wasm3/execute/arith/addition                            0.149 us        0.149 us       138437
fizzy/execute/arith/division_by_zero               ERROR OCCURRED: 'Trapped'
fizzyc/execute/arith/division_by_zero              ERROR OCCURRED: 'Trapped'
 wabt/execute/arith/division_by_zero               ERROR OCCURRED: 'Trapped'
wasm3/execute/arith/division_by_zero               ERROR OCCURRED: 'Trapped'
fizzy/execute/arith/memory_initialization_failure  ERROR OCCURRED: 'Memory initialization failed'
fizzyc/execute/arith/memory_initialization_failure ERROR OCCURRED: 'Memory initialization failed'
 wabt/execute/arith/memory_initialization_failure  ERROR OCCURRED: 'Memory initialization failed'
wasm3/execute/arith/memory_initialization_failure  ERROR OCCURRED: 'Memory initialization failed'
fizzy/execute/arith/unexcepted_result              ERROR OCCURRED: 'Unexpected result value: 1'
fizzyc/execute/arith/unexcepted_result             ERROR OCCURRED: 'Unexpected result value: 1'
 wabt/execute/arith/unexcepted_result              ERROR OCCURRED: 'Unexpected result value: 1'
wasm3/execute/arith/unexcepted_result              ERROR OCCURRED: 'Unexpected result value: 1'
fizzy/execute/arith/expected_memory_shorter        ERROR OCCURRED: 'Result memory is shorter than expected'
fizzyc/execute/arith/expected_memory_shorter       ERROR OCCURRED: 'Result memory is shorter than expected'
 wabt/execute/arith/expected_memory_shorter        ERROR OCCURRED: 'Result memory is shorter than expected'
wasm3/execute/arith/expected_memory_shorter        ERROR OCCURRED: 'Result memory is shorter than expected'
fizzy/execute/arith/missing_function               ERROR OCCURRED: 'Function "sub" not found'
fizzyc/execute/arith/missing_function              ERROR OCCURRED: 'Function "sub" not found'
 wabt/execute/arith/missing_function               ERROR OCCURRED: 'Function "sub" not found'
wasm3/execute/arith/missing_function               ERROR OCCURRED: 'Function "sub" not found'
fizzy/execute/arith/incorrect_result_value         ERROR OCCURRED: 'Incorrect result value, expected: 5, got: 4'
fizzyc/execute/arith/incorrect_result_value        ERROR OCCURRED: 'Incorrect result value, expected: 5, got: 4'
 wabt/execute/arith/incorrect_result_value         ERROR OCCURRED: 'Incorrect result value, expected: 5, got: 4'
wasm3/execute/arith/incorrect_result_value         ERROR OCCURRED: 'Incorrect result value, expected: 5, got: 4'
fizzy/parse/invalid                                ERROR OCCURRED: 'Parsing failed'
fizzyc/parse/invalid                               ERROR OCCURRED: 'Parsing failed'
 wabt/parse/invalid                                ERROR OCCURRED: 'Parsing failed'
wasm3/parse/invalid                                     0.981 us        0.981 us        14528 rate=39.7592M/s size=39
fizzy/instantiate/invalid                          ERROR OCCURRED: 'Instantiation failed'
fizzyc/instantiate/invalid                         ERROR OCCURRED: 'Instantiation failed'
 wabt/instantiate/invalid                          ERROR OCCURRED: 'Instantiation failed'
wasm3/instantiate/invalid                                3.22 us         3.22 us         3205
fizzy/execute/invalid/add                          ERROR OCCURRED: 'Instantiation failed'
fizzyc/execute/invalid/add                         ERROR OCCURRED: 'Instantiation failed'
 wabt/execute/invalid/add                          ERROR OCCURRED: 'Instantiation failed'


96% tests passed, 1 tests failed out of 23

Total Test time (real) =   0.37 sec

The following tests FAILED:
	  5 - fizzy/smoketests/bench/benchmarks (Subprocess aborted)
Errors while running CTest

We did find some reason, increased something/update compiler to work it around, but it is back again.

@axic
Copy link
Member Author

axic commented Mar 19, 2023

Apparently we run into a wasm3 bug: wasm3/wasm3#425

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.

3 participants