feat: use second table to visit members

[HerrCai0907]

visit members in large projects will generate large switch case, it can be optimized to call_indirect when reference-types enabled.

[MaxGraey]

What do you think to use compileVisitMembersWithCallIndirect only for large amount of members (for example >= 16) for optimized for speed builds and always use it (without a threshold) for optimized for size builds?

[HerrCai0907]

I think we cannot get any benefit from old implement in runtime which supports new features. So maybe emit call_indirect is better in each cases (more clear, easier to further opt, ...)

[MaxGraey]

As far as I know, switch-case is usually faster than dynamic dispatching. Rust even has a whole package for this, https://crates.io/crates/enum_dispatch. Or have you already tried to evaluate the performance, and it turned out to be the same as jump table / switch-case?

[HerrCai0907]

It is not switch case vs dynamic dispatching. It is switch case based dynamic dispatching vs table lookup based dynamic dispatching. The major task is dispatching to actually visitor function.
old implement is

switch (obj.rtid) {
  case 0:
    call fn0;
  case 1:
   call fn1;
  ...
}

new implement is

jump_table = [fn0, fn1, ...]
call_indirect jump_table[obj.rtid];

[MaxGraey]

I understand. The problem is that call_indirect is not a very cheap operation even without wasm vm. In Wasm engines bounds check operation + long indirect jump is usually performed. In switch-case approach it is a very fast deterministic jump table. So I would first of all test the performance and how much it regresses if you always use call_indirect

[MaxGraey]

I can't reproduce benchmark. I bootstraped compiler with this PR (with enabled reference-types feature) and main branch .wasm files. Optimize via wasm-opt as you suggested but when I try run js bench file it can't find __visit_members. All builds in debug btw.

    ins.instance.exports["__visit_members"](ptr, 0);
                                           ^

TypeError: ins.instance.exports.__visit_members is not a function

However rest of exports is present:

...
  __new: [Function: 50],
  __pin: [Function: 2389],
  __unpin: [Function: 2390],
  __collect: [Function: 2391],
  __rtti_base: Global [WebAssembly.Global] {},
  memory: Memory [WebAssembly.Memory] {},
  __setArgumentsLength: [Function: 2805],
  _initialize: [Function: 2806],
  setTarget: [Function: 5291],
  setRuntime: [Function: 5292],
  setNoAssert: [Function: 5293],
  setExportMemory: [Function: 5294],
  
  ...

And as I remember __visit_members is not exported from module and that's expected. So I'm confused...

[MaxGraey]

Can you prepare a worked benchmark and share it with GitHub gist for example?

[MaxGraey]

When we tall about performance, we should not keep eyes in switch and call_indirect itself.
Here is diff between old and new.
old

It's typical codegen for any switch statement in wasm. Btw size of compiler (.wasm)

- with switch-case:    1,824,783 bytes (main)
- with indirect_call:  1,822,204 bytes (this pr)

So with indirect call we just shrink ~0.14% for non-optimized build.
For an optimized build, it will be even less

[HerrCai0907]

Sorry, I forgot one step, in bootstrap wat, manual export __visit_members.

[HerrCai0907]

So with indirect call we just shrink ~0.14% for non-optimized build.

It is not a module level change, it is a function level change. which means we save 2.5kB wasm code by optimizing one function.

[HerrCai0907]

perf.zip

preprocessed wat file for benchmarkting

[github-actions]

This PR has been automatically marked as stale because it has not had recent activity. It will be closed in one week if no further activity occurs. Thank you for your contributions!

[github-actions]

This PR has been automatically closed due to lack of recent activity, but feel free to reopen it as long as you merge in the main branch afterwards.