1134552 - asm.js functions doesn't get inlined

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine: JIT, defect, P5)

Description

[Michał Wadas]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20150122214805

Steps to reproduce:

I have compared perfomance of following codes:

function Module() {
   'use asm';
   function add_integers(a,b) {
     a = a|0;
     b = b|0;
     return (a|0 + b|0)|0;
   }
   return {
      add_integers: add_integers
   };
}
    var add_asm = Module().add_integers;
    var add_normally = function(a,b) {
      a = a|0;
      b = b|0;
      return (a|0 + b|0)|0;
    };

for (var i = 0; i < 1000; i++) {
    add_asm(i,i+1);
}

 	

for (var i = 0; i < 1000; i++) {
    add_normally(i,i+1);
}



Actual results:

asm.js version was 1000 times slower.

Microbenchmark related: http://jsperf.com/asm-js-doesn-t-get-inlined


Expected results:

asm.js version should be at least as fast as normal JS version.

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

Thanks for opening an issue! Actually, it is pretty expected in this case: calling from non-asmjs into asmjs takes some time (namely, putting JS values directly into registers, do some checks, and so on), so the slowdown you're measuring is actually due to the asm.js call overhead.  However, if you spend more time inside the module itself, this overhead is negligible, for instance:


function SecondModule() {
   'use asm';
   function add_integers(a,b) {
     a = a|0;
     b = b|0;
     return (a|0 + b|0)|0;
   }
   function loop(n) {
       n = n|0;
       var i = 0;
       for (; (i | 0) < (n | 0); i = i + 1 | 0)
           add_integers(i, i + 1 | 0) | 0;
   }
   return {
      add_integers: loop
   };
}
SecondModule().add_integers(1000);

Ion (the normal super-optimizing VM part) doesn't have fast call paths into Odin (and cannot, at the moment, inline asm.js code), so I assume what happens here is that we jump from Ion into Baseline into Odin, then back into Baseline and into Ion, so this micro-benchmark measures time spent in the different kinds of trampolines.

Comment 2

[Michał Wadas]

I know it's expected - but it's unwanted.
That behavior that prevents eg. implementing fast complex number arithmetic using objects with methods backed by asm.js.

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Well, the whole story of asm.js is to try to spend most of the CPU time inside the asm.js module.  Any path from/to the outside is a slow path, by definition...

Also, I think your benchmark coincidentally has some issues, as it isn't measuring what it claims it does:

  (a | 0 + b | 0) == (a | (0 + b | 0)) == a|b

Not changing anything with respect to the inlining story, though, so not a real issue here.

Comment 4

[Boris Zbarsky [:bzbarsky]]

I do think there's a problem here in that code structured as above will actually run slower in Firefox than other browsers, no?

I wonder whether we can do some heuristic where if an asm.js function would be inlined we do in fact inline it...

Comment 5

[Boris Zbarsky [:bzbarsky]]

Though note, of course, that once inline it's no longer doing the asm.js stuff, so the entire use of asm.js in that situation is a red herring.

Comment 6

[Luke Wagner [:luke]]

Ion (and all type-specializing JITs these days) do fine with small snippets of scalar asm.js-like code, so users wanting to implement tiny number routines have no real need for "use asm"; they can just write it in an asm.js style, that uses |0 for integer arithmetic etc.  The point of "use asm" is to handle enormous pieces of code (which want AOT, caching, etc) and the representation that is optimized for these enormous pieces of code is what prevents inlining.

One day we may want to enable cross-asm.js-module inlining, but it would be a significant amount of work that didn't fundamentally enable any new use cases (since libraries of small routines can be written just as well w/o asm.js); it would mostly just help us on jsPerf benchmarks.  Perhaps usage patterns will change and so we will see the value of such a feature, but for now it seems like a lower priority.

On a side note, the Ion->asm.js path is particularly slow b/c there is not even an IC to call in.  The asm.js->Ion path is hotter and we do have an IC for that so it's about as fast as a normal Ion->Ion call (but slower of course than inlining).

Comment 7

[Boris Zbarsky [:bzbarsky]]

I guess my question is whether we can heuristically detect functions that would not benefit from "use asm" (e.g. short function, no calls inside) and just recompile them with normal Ion as needed...

Comment 8

[Sean Stangl [:sstangl]]

(In reply to Boris Zbarsky [:bz] from comment #7)
> I guess my question is whether we can heuristically detect functions that
> would not benefit from "use asm" (e.g. short function, no calls inside) and
> just recompile them with normal Ion as needed...

Probably not quite with normal Ion, since we wouldn't have any Baseline information. We would have to teach IonBuilder about the Odin typesystem, or use Odin itself in some different mode.

Comment 9

[Luke Wagner [:luke]]

It's also hard to 'just recompile them with normal Ion'.  At parse time, we see "use asm" and, if the source validates, we don't even create a JSScript for the module or nested functions (bytecode uses tons of memory and takes tons of time to create), so after asm.js validation is complete, there isn't any simple way to use the JITs.  That means, to do what it sounds like you're talking about, we'd need purely syntactic heuristic that effectively enables/disables Odin.  This sounds dangerous from a predictability standpoint if we want to do anything but the simplest heuristic (tiny, no loops no calls which I suppose would have caught the code in comment 0).

Perhaps instead, since we can assume people hand-writing asm.js are looking at the JS console for asm.js validation messages, we could track time spent in CallAsmJS (our trampoline function) and if we're seeing an inordinate number of calls (say, more than 10,000 calls in a second), issue a console warning.

Comment 10

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.

Comment 11

[Mayank Bansal]

WASM has filled in teh space of asm.js these days.
Closing this bug.