Stream: git-wasmtime

Topic: wasmtime / PR #4829 Cranelift: Deduplicate ABI signatures...

Wasmtime GitHub notifications bot (Aug 31 2022 at 01:19):

fitzgen requested cfallin for a review on PR #4829.

Wasmtime GitHub notifications bot (Aug 31 2022 at 01:19):

fitzgen opened PR #4829 from dedupe-abi-signatures to main:

This commit creates the SigSet type which interns and deduplicates the ABI
signatures that we create from ir::Signatures. The ABI signatures are now
referred to indirectly via a Sig (which is a cranelift_entity ID), and we
pass around a SigSet to anything that needs to access the actual underlying
SigData (which is what ABISig used to be).

I had to change a couple methods to return a SmallInstVec instead of emitting
directly to work around what would otherwise be shared and exclusive borrows of
the lowering context overlapping. I don't expect any of these to heap allocate
in practice.

This does not remove the often-unnecessary allocations caused by
ensure_struct_return_ptr_is_returned. That is left for follow up work.

This also opens the door for further shuffling of signature data into more
efficient representations in the future, now that we have SigSet to store it
all in one place and it is threaded through all the code. We could potentially
move each signature's parameter and return vectors into one big vector shared
between all signatures, for example, which could cut down on allocations and
shrink the size of SigData since those SmallVecs have pretty large inline
capacity.

Overall, this refactoring gives a 1-7% speedup for compilation on
pulldown-cmark:

compilation :: cycles :: benchmarks/pulldown-cmark/benchmark.wasm

  Δ = 8754213.66 ± 7526266.23 (confidence = 99%)

  dedupe.so is 1.01x to 1.07x faster than main.so!

  [191003295 234620642.20 280597986] dedupe.so
  [197626699 243374855.86 321816763] main.so

compilation :: cycles :: benchmarks/bz2/benchmark.wasm

  No difference in performance.

  [170406200 194299792.68 253001201] dedupe.so
  [172071888 193230743.11 223608329] main.so

compilation :: cycles :: benchmarks/spidermonkey/benchmark.wasm

  No difference in performance.

  [3870997347 4437735062.59 5216007266] dedupe.so
  [4019924063 4424595349.24 4965088931] main.so

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Aug 31 2022 at 16:36):

fitzgen updated PR #4829 from dedupe-abi-signatures to main.

Wasmtime GitHub notifications bot (Aug 31 2022 at 18:13):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 31 2022 at 18:13):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 31 2022 at 18:13):

cfallin created PR review comment:

Is there any way that we can stuff the SigSet into the VCode? It seems like maybe it is held separately here because of borrow-checker issues but maybe we can work around that if so.

Wasmtime GitHub notifications bot (Aug 31 2022 at 18:42):

fitzgen updated PR #4829 from dedupe-abi-signatures to main.

Wasmtime GitHub notifications bot (Aug 31 2022 at 18:43):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Aug 31 2022 at 18:43):

fitzgen created PR review comment:

Done! Required a little bit of plumbing accessors through so things that have a Lower could access the Lower's VCodeBuilder's VCode's SigSet but it was ultimately a nice clean up.

Wasmtime GitHub notifications bot (Aug 31 2022 at 18:43):

fitzgen has enabled auto merge for PR #4829.

Wasmtime GitHub notifications bot (Aug 31 2022 at 19:23):

fitzgen updated PR #4829 from dedupe-abi-signatures to main.

Wasmtime GitHub notifications bot (Aug 31 2022 at 20:39):

fitzgen merged PR #4829.

Last updated: Dec 23 2024 at 12:05 UTC