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

use rkyv deserialize to replace bincode on jit artifact #2279

Closed
wants to merge 1 commit into from

Conversation

ailisp
Copy link
Contributor

@ailisp ailisp commented Apr 30, 2021

Description

So this is applying similar technique to accelerate deserialize an jit artifact. I test by adding this in tests/compiler/serialize.rs, fn test_deserialize:

    let wat = std::fs::read("lib/c-api/tests/assets/qjs.wasm").unwrap();

    let module = Module::new(&store, wat)?;
    let serialized_bytes = module.serialize()?;

    let headless_store = get_headless_store();

    let start = std::time::Instant::now();
    let deserialized_module = unsafe { Module::deserialize(&headless_store, &serialized_bytes)? };
    println!("deserialize takes {:?}", start.elapsed());

run it with

cargo test --features singlepass,test-singlepass,test-jit --test compilers serialize::test_deserialize --release -- --nocapture

Before it was 11-12ms, this PR make it 6-9ms, most of the time around 7ms. The optimization is visible, but we may achieve a faster one via Archive, that is replace all usage of SerializableModule and its fields, to ArchivedSerializableModule - this would make deserialize an O(1) mmap operation, but also add further intrusive changes based on this PR.

Review

  • Add a short description of the change to the CHANGELOG.md file

@syrusakbary
Copy link
Member

Hey @ailisp I've created actually a very similar PR yesterday #2250.
There is a bug on rkyv that I'm trying to debug with it's creator (as it only triggers in rkyv 0.6.0)

@ailisp
Copy link
Contributor Author

ailisp commented Apr 30, 2021

@syrusakbary Ah I'll close this in favor of your PR. i actually checked yesterday but happened to before your PR isn't happen and then start work 😂, let me know if i can help on review or the rkyv bug (maybe)

@ailisp ailisp closed this Apr 30, 2021
@syrusakbary
Copy link
Member

syrusakbary commented Apr 30, 2021

If you want to reproduce the bug, you need to:

  • just follow the instructions in the PR: Use rkyv for JIT Engine #2250. It works properly with rkyv 0.4.3
  • Revert this commit 4c54d86
  • Run again the command: ./target/release/wasmer run lib/c-api/tests/assets/qjs.wasm --disable-cache -- -h

@djkoloski
Copy link

The bug in question has been fixed as of 0.6.1. rkyv issue: rkyv/rkyv#116

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