Wasmtime GitHub notifications bot (Nov 30 2020 at 05:13): Wasmtime GitHub notifications bot (Nov 30 2020 at 05:13):abrown opened PR #2458 from improve-interpreter to main:
Previously, getting or setting a value in a frame of the Cranelift interpreter involved a hash table lookup. Since the interpreter statically knows the number of slots necessary for each called frame, we can use a vector instead and save time on the hash lookup. This also has the advantage that we have a more stable ABI for switching between interpreted and code.
<!--
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 (Nov 30 2020 at 05:13):abrown requested fitzgen for a review on PR #2458.
Wasmtime GitHub notifications bot (Nov 30 2020 at 09:04):bjorn3 created PR Review Comment:
Maybe use
SecondaryMapinstead?
Wasmtime GitHub notifications bot (Nov 30 2020 at 09:04):bjorn3 submitted PR Review.
Wasmtime GitHub notifications bot (Nov 30 2020 at 18:26):fitzgen submitted PR Review.
Wasmtime GitHub notifications bot (Nov 30 2020 at 18:26):fitzgen submitted PR Review.
Wasmtime GitHub notifications bot (Nov 30 2020 at 18:26):fitzgen created PR Review Comment:
Is this helper carrying its weight? It only saves five characters and doesn't do anything different (like unwrap or provide default values, etc).
Wasmtime GitHub notifications bot (Nov 30 2020 at 18:29):fitzgen submitted PR Review.
Wasmtime GitHub notifications bot (Nov 30 2020 at 18:29):fitzgen created PR Review Comment:
Although having a newtype for indices would be nice, it isn't clear this is a pure win, since
SecondaryMapwill check if it needs to grow the underlying vec on each insertion since it lazily adds capacity (which we can reserve up front, but I'm not sure LLVM can always optimize those checks away).
Wasmtime GitHub notifications bot (Nov 30 2020 at 20:44):abrown updated PR #2458 from improve-interpreter to main:
Previously, getting or setting a value in a frame of the Cranelift interpreter involved a hash table lookup. Since the interpreter statically knows the number of slots necessary for each called frame, we can use a vector instead and save time on the hash lookup. This also has the advantage that we have a more stable ABI for switching between interpreted and code.
<!--
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 (Nov 30 2020 at 20:44):abrown submitted PR Review.
Wasmtime GitHub notifications bot (Nov 30 2020 at 20:44):abrown created PR Review Comment:
Gone!
Wasmtime GitHub notifications bot (Nov 30 2020 at 23:41):abrown merged PR #2458.
Last updated: Oct 23 2024 at 20:03 UTC