compiler/wasm: memoize in function (not callsite) #3169
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
srenatus
force-pushed
the
sr/wasm/memoize-in-function-not-callsite
branch
from
893bba2 to
d85df7f
Compare
|
😬 forgot about something: the args length check. |
srenatus
force-pushed
the
sr/wasm/memoize-in-function-not-callsite
branch
3 times, most recently
from
1fee90a to
8da631c
Compare
|
Also forgot that we can drop the memo around call_indirect, and thereby the elem_to_func mechanism. ✨ |
srenatus
force-pushed
the
sr/wasm/memoize-in-function-not-callsite
branch
2 times, most recently
from
b1ed3f6 to
d0191f1
Compare
|
✔️ The |
As soon as a function is called twice, there should be a little bit
to gain from having the memoization happen in the function body.
Run on a big bundle, this results in the following change in opcode
counts:
Total opcodes: 184881 -> 183719
Opcode counts:
local.get: 46920 -> 46760 -
i32.const: 24152 -> 23706 -
local.set: 14097 -> 14097 =
call: 12547 -> 12061 -
end: 10911 -> 11015 +
br_if: 11062 -> 10826 -
block: 8987 -> 9092 +
So, the change isn't dramatic at all. But less is more, so I guess
we could still do this.
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
srenatus
force-pushed
the
sr/wasm/memoize-in-function-not-callsite
branch
from
d0191f1 to
ac39e2f
Compare
srenatus
commented
Feb 22, 2021
| c.appendInstr(instruction.GetLocal{Index: c.local(fn.Return)}) | ||
| c.appendInstr(instruction.Call{Index: c.function(opaMemoizeInsert)}) | ||
| } | ||
| c.appendInstr(instr) |
There was a problem hiding this comment.
💭 Currently, if this instruction is Return, we could just ignore it. The last value on the stack is the function's return value, with our without that statement.
tsandall
approved these changes
Feb 23, 2021
This was referenced Mar 9, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As soon as a function is called twice, there should be a little bit
to gain from having the memoization happen in the function body.
Run on a big bundle, this results in the following change in opcode
counts:
So, the change isn't dramatic at all. But less is more, so I guess
we could still do this.