Use borsh to deserialize JIT Artifact

[ailisp]

Description

This has a little speed up on deserialization JIT Artifact, made a benchmark to store and load a 2MB lib/c-api/tests/assets/qjs.wasm from filesystem cache. To run the bench: cargo bench -p wasmer-cache --features singlepass. Before this PR it is:

store module in filesystem cache                                                                            
                        time:   [15.311 ms 15.356 ms 15.403 ms]
                        change: [-4.4021% -1.2939% +0.7777%] (p = 0.47 > 0.05)
                        No change in performance detected.
Found 2 outliers among 300 measurements (0.67%)
  2 (0.67%) high mild

load module in filesystem cache                                                                            
                        time:   [9.5429 ms 9.5611 ms 9.5803 ms]
                        change: [-1.2390% -0.9352% -0.6680%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 11 outliers among 300 measurements (3.67%)
  7 (2.33%) high mild
  4 (1.33%) high severe

After this PR:

store module in filesystem cache                                                                            
                        time:   [10.879 ms 10.938 ms 11.005 ms]
Found 25 outliers among 300 measurements (8.33%)
  12 (4.00%) high mild
  13 (4.33%) high severe

load module in filesystem cache                                                                            
                        time:   [6.0817 ms 6.0960 ms 6.1113 ms]
Found 6 outliers among 300 measurements (2.00%)
  3 (1.00%) high mild
  3 (1.00%) high severe

Review

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

[Hywan]

#[cfg_attr(feature = "enable-borsh", derive(BorshSerialize, BorshDeserialize))]?

[ailisp]

This is not under feature, because Serialize and Deserialize was always enabled, whereas crate like lib/compiler/Cargo.toml has optional serde, thus also optional borsh

[nlewycky]

Why? A SecondaryMap is a PrimaryMap without the ability to mutate the keys, which in turn is just a typesafe Vec where the index has a wrapper type instead to keep them distinct instead of plain integers. Is the problem not serialization but deserialization?

[ailisp]

PrimaryMap is internally just a vec, but SecondaryMap is more complex to serialize:
https://github.com/bytecodealliance/wasmtime/blob/285a7feb19c1ce768d68fd421eddd1ff6b093d34/cranelift/entity/src/map.rs#L200

More proper way is implement a borshserialize/deserialize similarly to its impl of serde serialize/deserialize

[MarkMcCaskey]

I'm generally in favor of conditional support for other types of serialization 👍

[MarkMcCaskey]

We shouldn't merge this while there's a git dependency as it will block us from doing releases on crates.io

[ailisp]

This commit haven't published, will keep this updated as we publish a new borsh

[ailisp]

@Hywan @MarkMcCaskey just added an benchmark, it's a 30% increase in a larger contract, might be interested to consider borsh. But deserialization is still not fast enough for our use case, so I'm going to try some mmap based serialization approach.

[Hywan]

it's a 30% increase in a larger contract

Do you mean that serialization is 30% faster?

[ailisp]

Both serialization (store_module) and deserialization (load_module) are 30% faster, possibly a little more because store_module = serialization + write_to_file and write_to_file part isn't changed. See bench numbers in PR description

…

On Thu, Feb 25, 2021 at 12:04 AM Ivan Enderlin ***@***.***> wrote: it's a 30% increase in a larger contract Do you mean that serialization is 30% faster? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2119 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADFFFCG7VIX5VJTPH6JUW7LTAYAARANCNFSM4XP44JAA> .

[ailisp]

close because #2190 is a faster approach