[wasm] Exclude ref.test from Wasm GC use counter [v8/v8 : main]
Jakob Kummerow (Gerrit)
Jakob Kummerow voted and added 3 comments![]( "Open in Gerrit")
Votes added by Jakob Kummerow
| Code-Review | +1 |
3 comments
Debatable, but I guess I'm OK with this.
uses of ref.test to avoid artificially inflating use counts.That wording is a bit strong, don't you think? The status quo counts all GC-prefixed instructions (which came with the GC proposal!), whereas this CL "artifically deflates" the count by introducing a rather arbitrary definition of usage that's so insignificant that we choose not to count it.
GetUseCounterMap().clear();Not necessary; we don't support running multiple tests in the same process anyway.
Related details
- Thomas Lively
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Matthias Liedtke (Gerrit)
Matthias Liedtke added 1 comment![]( "Open in Gerrit")
uses of ref.test to avoid artificially inflating use counts.That wording is a bit strong, don't you think? The status quo counts all GC-prefixed instructions (which came with the GC proposal!), whereas this CL "artifically deflates" the count by introducing a rather arbitrary definition of usage that's so insignificant that we choose not to count it.
I'd go one step further, I disagree with this. If we have another implementation that does the same but uses `br_on_cast`, would we drop that instruction as well? At which operation would we draw the line? Is usage of `i31ref` enough to qualify for "wasm-gc" even though there isn't any GC involved for it in V8? Would we need at least a struct or array type? What if that type is defined but not used?
I think these counters should reflect "uses a feature of this proposal", if you define two memories, then your module uses multi-memory even if you never access the second memory.
Related details
- Thomas Lively
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jakob Kummerow (Gerrit)
Jakob Kummerow voted and added 1 comment![]( "Open in Gerrit")
Votes added by Jakob Kummerow
| Code-Review | +0 |
1 comment
In light of the amount of skepticism (mostly expressed offline) I'd like to understand better what the motivation is before landing this. What are we hoping to get out of the WasmGC use counter? Would it make sense to introduce a separate counter with a different scope?
Related details
- Thomas Lively
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thomas Lively (Gerrit)
Thomas Lively added 2 comments![]( "Open in Gerrit")
In light of the amount of skepticism (mostly expressed offline) I'd like to understand better what the motivation is before landing this. What are we hoping to get out of the WasmGC use counter? Would it make sense to introduce a separate counter with a different scope?
The motivation is that we look at trends in Wasm GC usage and it looks really great, but I have a strong suspicion that a lot of the usage is actually just Python. What I'd really like to know is the usage trends for modules produced from Wasm GC toolchains, i.e. not Python.
What do you both think about a new use counter that only looks for allocation instructions, e.g. struct.new and array.new? I think that will be a better match for "real Wasm GC usage" in the interesting sense.
uses of ref.test to avoid artificially inflating use counts.Matthias LiedtkeThat wording is a bit strong, don't you think? The status quo counts all GC-prefixed instructions (which came with the GC proposal!), whereas this CL "artifically deflates" the count by introducing a rather arbitrary definition of usage that's so insignificant that we choose not to count it.
I'd go one step further, I disagree with this. If we have another implementation that does the same but uses `br_on_cast`, would we drop that instruction as well? At which operation would we draw the line? Is usage of `i31ref` enough to qualify for "wasm-gc" even though there isn't any GC involved for it in V8? Would we need at least a struct or array type? What if that type is defined but not used?
I think these counters should reflect "uses a feature of this proposal", if you define two memories, then your module uses multi-memory even if you never access the second memory.
- Jakob Kummerow
- Matthias Liedtke
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thomas Lively (Gerrit)
Thomas Lively abandoned this change.
Thomas Lively abandoned this change![]( "Open in Gerrit")
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |