emscripten-core / emscripten-core/emscripten
[perf] Unnecessary memory growth check repetitions
- Dominant language
- C++
- Stars
- 27.6k
- Forks
- 3.6k
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 105
Description
For background: I've been looking through https://github.com/WebAssembly/design/issues/1271 referenced from console warnings when building Wasm with threads, as well as implementation PR.
I think current Emscripten's implementation of tracking memory growth has implementation details that make it much slower than it has to be and could be made much faster even Wasm API design aside.
For example, I've looked at the generated JS of the benchmark from https://github.com/WebAssembly/design/issues/1271#issuecomment-477257958 and its performance profiles, and noticed that it's spending quite a lot of time in `GROWABLE_HEAP_*` functions.
On its own it's unsurprising, but upon closer look it seems to be not so much due to the slowness of functions themselves (where the only expensive part is the `wasmMemory.buffer` accessor) but rather due to number of times they are called, which significantly multiplies that constant-time cost.
For example, here is what the generated `AsciiToString` - one of the highest callees - looks like:
```js
function AsciiToString(ptr) {
var str = "";
while (1) {
var ch = GROWABLE_HEAP_U8()[ptr++ >> 0];
if (!ch) return str;
str += String.fromCharCode(ch);
}
}
```
Since it's expected to read an existing string, there is no reason to repeatedly check for memory growth for every single byte - the view can't get invalidated anyway, and we don't care about higher bytes even if memory grows.
Changing this single function to
```js
function AsciiToString(ptr) {
var str = "";
var heapU8 = GROWABLE_HEAP_U8();
while (1) {
var ch = heapU8[ptr++ >> 0];
if (!ch) return str;
str += String.fromCharCode(ch);
}
}
```
made the benchmark on my machine in latest stable Chrome go from 5.7s to 4.1s - 28% improvement.
There seems to be a lot of other functions (e.g. `readAsmConstArgs`) that do the same repetitive checks inside a loop, presumably due to the templating Emscripten uses for the `HEAP` variables that substitutes all such accessors with function calls.
Fixing few more suspects quickly brought performance down to 2.9s - almost 50% of the original time.
I don't know if it's possible to automatically check+update memory views only on edges where Wasm calls / returns to JS (e.g. by instrumenting imports + ASM_CONSTS), or whether this is something that needs to be changed manually on per-function basis, but I thought I'd post this anyway, as fixing such loops seems to be a very low-hanging fruit that could significantly improve performance of pthread + memory growth builds.
Contributor guide
Assessment
This issue has not been assessed yet.