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

2021 05 17 wasmer #773

Merged
merged 43 commits into from
May 25, 2021
Merged

2021 05 17 wasmer #773

merged 43 commits into from
May 25, 2021

Conversation

thedavidmeister
Copy link
Contributor

@thedavidmeister thedavidmeister commented May 17, 2021

relies on a wasmer memory leak fix - wasmerio/wasmer#2327

implements holochain/holochain-wasmer#56

sadly the benchmarks are about 15-30% slower but still nominal

i have no idea why, maybe the introduction of the rwlock?

upstream benchmarks are much faster so it is weird for sure

[nix-shell:~/holochain]$ cargo bench --bench bench
   Compiling holochain_wasm_test_utils v0.0.1 (/home/thedavidmeister/holochain/crates/test_utils/wasm)
   Compiling hdk v0.0.101-alpha.0 (/home/thedavidmeister/holochain/crates/hdk)
   Compiling holochain_state v0.0.1 (/home/thedavidmeister/holochain/crates/holochain_state)
   Compiling holochain_conductor_api v0.0.1 (/home/thedavidmeister/holochain/crates/holochain_conductor_api)
   Compiling holochain_cascade v0.0.1 (/home/thedavidmeister/holochain/crates/holochain_cascade)
   Compiling holochain_test_wasm_common v0.0.1 (/home/thedavidmeister/holochain/crates/test_utils/wasm_common)
   Compiling holochain v0.0.100 (/home/thedavidmeister/holochain/crates/holochain)
    Finished bench [optimized] target(s) in 3m 28s
     Running unittests (target/release/deps/bench-7397c2657d614708)
WARNING: HTML report generation will become a non-default optional feature in Criterion.rs 0.4.0.
This feature is being moved to cargo-criterion (https://github.com/bheisler/cargo-criterion) and will be optional in a future version of Criterion.rs. To silence this warning, either switch to cargo-criterion or enable the 'html_reports' feature in your Cargo.toml.

Gnuplot not found, using plotters backend
Benchmarking wasm_call_n/1: Warming up for 3.0000 sFixturator seed: 2652401984165948502
wasm_call_n/1           time:   [3.8134 ms 4.0124 ms 4.3292 ms]                           
                        thrpt:  [230.99   B/s 249.23   B/s 262.23   B/s]
                 change:
                        time:   [+26.943% +33.354% +45.135%] (p = 0.00 < 0.05)
                        thrpt:  [-31.099% -25.012% -21.225%]
                        Performance has regressed.
Found 10 outliers among 100 measurements (10.00%)
  7 (7.00%) high mild
  3 (3.00%) high severe
wasm_call_n/1000        time:   [4.0591 ms 4.3137 ms 4.7952 ms]                              
                        thrpt:  [203.66 KiB/s 226.39 KiB/s 240.58 KiB/s]
                 change:
                        time:   [+25.949% +33.700% +54.408%] (p = 0.00 < 0.05)
                        thrpt:  [-35.237% -25.206% -20.603%]
                        Performance has regressed.
Found 6 outliers among 100 measurements (6.00%)
  5 (5.00%) high mild
  1 (1.00%) high severe
Benchmarking wasm_call_n/1000000: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 9.8s, or reduce sample count to 50.
wasm_call_n/1000000     time:   [95.882 ms 96.424 ms 96.955 ms]                                
                        thrpt:  [9.8363 MiB/s 9.8904 MiB/s 9.9463 MiB/s]
                 change:
                        time:   [+13.214% +14.746% +16.388%] (p = 0.00 < 0.05)
                        thrpt:  [-14.081% -12.851% -11.671%]
                        Performance has regressed.

     Running unittests (target/release/deps/bench-43b3edd4c1facd76)
WARNING: HTML report generation will become a non-default optional feature in Criterion.rs 0.4.0.
This feature is being moved to cargo-criterion (https://github.com/bheisler/cargo-criterion) and will be optional in a future version of Criterion.rs. To silence this warning, either switch to cargo-criterion or enable the 'html_reports' feature in your Cargo.toml.

Gnuplot not found, using plotters backend
simple_bench/client/request                                                                            
                        time:   [70.974 us 73.119 us 75.528 us]
                        change: [+13.499% +16.380% +19.298%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)
  4 (4.00%) high mild
simple_bench/client/signal                                                                             
                        time:   [2.2957 us 2.3275 us 2.3604 us]
                        change: [+2.1030% +4.0911% +6.1584%] (p = 0.00 < 0.05)
                        Performance has regressed.
simple_bench/client/response                                                                            
                        time:   [70.769 us 72.636 us 74.496 us]
                        change: [+11.698% +14.636% +17.594%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 6 outliers among 100 measurements (6.00%)
  6 (6.00%) high mild

@maackle maackle marked this pull request as draft May 17, 2021 22:37
@syrusakbary
Copy link

this appears to work - but i know there's a memory leak in it that showed up in benchmarking the wasmer crate independently

The memory leak was recently introduced in master, is not present in the latest wasmer 1.0.2 published in crates.io :)

@thedavidmeister
Copy link
Contributor Author

@syrusakbary must be a different leak because 1.0.2 did not help - this fix wasmerio/wasmer#2327 seems to be working for me

@thedavidmeister thedavidmeister marked this pull request as ready for review May 21, 2021 16:08
@thedavidmeister thedavidmeister changed the title WIP: 2021 05 17 wasmer 2021 05 17 wasmer May 21, 2021
Copy link
Collaborator

@maackle maackle left a comment

Choose a reason for hiding this comment

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

Looks good but I'd like to see all the host fn access definitions collapsed back down to one line per fn

crates/holochain/src/core/ribosome/real_ribosome.rs Outdated Show resolved Hide resolved
@thedavidmeister thedavidmeister merged commit f1ab775 into develop May 25, 2021
@jost-s jost-s deleted the 2021-05-17-wasmer branch January 18, 2023 15:33
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