Stream: git-wasmtime

Topic: wasmtime / PR #2458 Optimize access to interpreter frame ...


view this post on Zulip 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.

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

view this post on Zulip Wasmtime GitHub notifications bot (Nov 30 2020 at 05:13):

abrown requested fitzgen for a review on PR #2458.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 30 2020 at 09:04):

bjorn3 created PR Review Comment:

Maybe use SecondaryMap instead?

view this post on Zulip Wasmtime GitHub notifications bot (Nov 30 2020 at 09:04):

bjorn3 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 30 2020 at 18:26):

fitzgen submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 30 2020 at 18:26):

fitzgen submitted PR Review.

view this post on Zulip 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).

view this post on Zulip Wasmtime GitHub notifications bot (Nov 30 2020 at 18:29):

fitzgen submitted PR Review.

view this post on Zulip 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 SecondaryMap will 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).

view this post on Zulip 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.

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

view this post on Zulip Wasmtime GitHub notifications bot (Nov 30 2020 at 20:44):

abrown submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 30 2020 at 20:44):

abrown created PR Review Comment:

Gone!

view this post on Zulip Wasmtime GitHub notifications bot (Nov 30 2020 at 23:41):

abrown merged PR #2458.


Last updated: Dec 23 2024 at 12:05 UTC