Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable direct dispatch for WASI #299

Merged
merged 1 commit into from Mar 10, 2024
Merged

Conversation

rockwotj
Copy link
Contributor

@rockwotj rockwotj commented Mar 10, 2024

Direct dispatch is faster than a "normal" switch clause and it's disabled for WASI, although it works fine in wasmtime, so I'm not sure why it was disabled.

Background reading on computed gotos for interpreter loops: https://eli.thegreenplace.net/2012/07/12/computed-goto-for-efficient-dispatch-tables

Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@rockwotj
Copy link
Contributor Author

Difference running this program:

function fibonacci(num) {
  if (num <= 1) return 1;

  return fibonacci(num - 1) + fibonacci(num - 2);
}

console.log(fibonacci(35));

With DIRECT_DISPATCH=1

$ hyperfine --warmup=2 -N 'wasmtime run --dir . build/qjs t.js'
Benchmark 1: wasmtime run --dir . build/qjs t.js
  Time (mean ± σ):      1.209 s ±  0.154 s    [User: 1.205 s, System: 0.004 s]
  Range (min … max):    1.138 s …  1.643 s    10 runs

With DIRECT_DISPATCH=0

$ hyperfine --warmup=2 -N 'wasmtime run --dir . build/qjs t.js'
Benchmark 1: wasmtime run --dir . build/qjs t.js
  Time (mean ± σ):      2.050 s ±  0.042 s    [User: 2.043 s, System: 0.006 s]
  Range (min … max):    2.024 s …  2.167 s    10 runs

@rockwotj
Copy link
Contributor Author

cc: @saghul since you added WASI support in #264

@rockwotj
Copy link
Contributor Author

rockwotj commented Mar 10, 2024

(never mind it defaults to release build the results are valid)

@saghul
Copy link
Contributor

saghul commented Mar 10, 2024

Hum, I remember testing it and things not working... perhaps it was with wasmer?

@rockwotj
Copy link
Contributor Author

Hrm I can give wasmer a try. Maybe it needs to be a compile time switch then?

@saghul
Copy link
Contributor

saghul commented Mar 10, 2024

There are other things that didn't work with wasmer, so I'm just ok landing this TBH.

@saghul saghul merged commit 33e38be into quickjs-ng:master Mar 10, 2024
36 checks passed
@rockwotj
Copy link
Contributor Author

Clang did add support for this, so maybe emscripten could be removed too: https://bugs.llvm.org/show_bug.cgi?id=42498

@rockwotj rockwotj deleted the faster-wasi branch March 10, 2024 10:39
@saghul
Copy link
Contributor

saghul commented Mar 10, 2024

I guess it depends on the emsdk version?

We don't quite have a way to e2e test emscripten, but if you verify it with quickjs-emscripten then I'd be happy to land a patch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants