Use direct table access over dynCall in invoke_xx functions

[sbc100]

This change starts the transition away from dynCall and towards
direct use of the wasm table.

In particular this change replaces the use of dynCall in invoke_xx
functions where possible. dynCall is still needed for functions with
i64 in thier signature when WASM_BIGINT is not enabled. When
WASM_BIGINT is enabled this change removes all use of dynCall by
invoke_xx functions.

[kripken]

Nice work!

[sbc100]

I'm a little worried about the cost of using the Array.slice for each invoke_xx call.

I have a couple of implementation options:

1. Use ES6 ...rest operator (we currently can't do this because we require all our code to be ES5 still.. sigh):

function dynCallDirect(ptr, ...args) {                                           
  return wasmTable.get(ptr).apply(null, args);                                   
} 

2. Use ES5 and Array.slice

function dynCallDirect(ptr) {                                                    
  return wasmTable.get(ptr).apply(null, Array.prototype.slice.call(arguments, 1));
} 

3. Stamp out a different func for each signature to avoid variadic function:

function dynCallDirect_vi(ptr, a1)...
function dynCallDirect_vii(ptr, a1, a2)...
etc...

[sbc100]

I guess I should go with option 3 since switch to ES6 is not something I want to take on right now :)

And Array.slice is costly? Can someone confirm the cost of this?

[kripken]

Array.slice does an allocation. It's not a huge cost, but it's good to avoid it, as there will be cases where it matters.

How about this, we use similar code patterns in other places:

var dynCallDirectArray = [];
function dynCallDirect(ptr) {
  dynCallDirectArray.length = arguments.length - 1;
  for (var i = 1; i < arguments.length; i++) {
    dynCallDirectArray[i - 1] = arguments[i];
  }
  return wasmTable.get(ptr).apply(null, dynCallDirectArray);
}

[sbc100]

Array.slice does an allocation. It's not a huge cost, but it's good to avoid it, as there will be cases where it matters.

How about this, we use similar code patterns in other places:

var dynCallDirectArray = [];
function dynCallDirect(ptr) {
  dynCallDirectArray.length = arguments.length - 1;
  for (var i = 1; i < arguments.length; i++) {
    dynCallDirectArray[i - 1] = arguments[i];
  }
  return wasmTable.get(ptr).apply(null, dynCallDirectArray);
}

But compare that to:

function dynCallDirect(ptr, ...args) {                                           
  return wasmTable.get(ptr).apply(null, args);                                   
} 

Doesn't it make your you want to cry a little inside?

[kripken]

@sbc100 yeah... ES6 is nice!

We've said at some point we should write ES6 and use babel to downcompile it...

[sbc100]

@sbc100 yeah... ES6 is nice!

We've said at some point we should write ES6 and use babel to downcompile it...

I know we have discussed before. Perhaps we can push it forward this time.

I know I probably sound like stuck record but can we not just consider ES6 -> ES5 conversion something that can easily be done downstream of us?

I would imagine that teams targeting old engines already have babel in their pipeline and it would be a little odd for us to embed it in addition. I suppose you can say the same for minification and closure, but we run those is specialized ways so they are little different.

Perhaps I should try to land this change as ES5 and push for ES6 separately in the coming weeks? If nothing else it would be nice code size win I imagine for most users.

[sbc100]

OK I think all the issues with this are now sorted. I've avoided ES6 for now. This change doesn't remove all the dynCall usage but is removes a lot of it.

I have followup changes to binaryen to limit the emitting of dynCall.

[sbc100]

I've reduced the scope of this change significantly such that it only covers invoke_xx functions.

Since we have one invoke function for each signature this allows me to avoid using Array.slice, or spread, or any kind of allocation.

[sbc100]

PTAL, I hope this good to land now.

As a followup PR, I plan to start limiting the dynCalls generated by wasm-emscripten-finalize.

@awtcode

[kripken]

It is a little unfortunate to always export the table in standalone mode, but I can't think of a good idea to avoid that.

[kripken]

Suggested change

	if (args && args.length) {
	if (args) {

an empty array is truthy in JS (but not in python)

[sbc100]

This code is pre-existing, plus it seems like we need both check here.. we want to check for args being defined and having a length.

If we remove the first check we will get Cannot read property 'length' of undefined.. if we remove the second check as you suggest when the empty array will go down the wrong path (since as you say its truthy).

[sbc100]

I'm going to land this as is for the above reasons.. if you think i'm wrong we can fix after.

[kripken]

Oh, right, I was mistaken about args.length, lgtm.

[kripken]

Suggested change

	if (args && args.length) {
	if (args) {