|new Function| code isn't Ion-compilable

RESOLVED DUPLICATE of bug 646597

Status

()

Core
JavaScript Engine
RESOLVED DUPLICATE of bug 646597
5 years ago
5 years ago

People

(Reporter: kael, Unassigned)

Tracking

22 Branch
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 736663 [details]
Slow version of test case (using |new Function|)

Emscripten-compiled code that uses embind to enable JS <-> C++ interop seems to be taking a severe performance hit due to its use of |new Function|. Specifically, it looks like the functions it generates using |new Function| don't get ion-compiled and as a result, they're stuck in JM land where they get less optimizations applied and inlining never seems to happen.

Taking the code normally generated with |new Function| and manually inserting it into the page causes it to get Ion-compiled and inlined as expected, effectively halving the runtime for the test.

I believe this could be caused by bug #646597 - specifically, these functions are probably not compile-and-go.
(Reporter)

Comment 1

5 years ago
Created attachment 736664 [details]
Fast version of test case (not using |new Function|)
Depends on: 646597

Comment 2

5 years ago
I see the "if (!script->compileAndGo)" abort in CheckScript (Ion.cpp), and functions compiled by new Function aren't compile-and-go, so that does seem to be the case.

Does anyone know one of either of (1) new Function can't be compileAndGo (2) why Ion can't compile !compileAndGo?

ISTR cdleary trying to make 'new Function' compile-and-go a while ago but I can't find the bug.
Summary: use of |new Function| prevents embind/emscripten functions from being inlined → |new Function| code isn't Ion-compilable
(In reply to Luke Wagner [:luke] from comment #2)
> Does anyone know one of either of (1) new Function can't be compileAndGo (2)
> why Ion can't compile !compileAndGo?

See bug 646597.

Comment 4

5 years ago
Oh wow, I hadn't seen that.  Should this just be a dup of that bug then?
Yeah, once bug 646597 is fixed these functions will be Ion-compilable.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 646597
(Reporter)

Comment 6

5 years ago
The reason I think this and the other bug report (about JSIL suffering badly from this same issue) probably shouldn't just be dups is that we have no way to know that compile-and-go is the ONLY reason these functions don't get ion-compiled. Though it seems plausible that it might be. Does that make sense?

The goal is for both use cases (generating invoker stubs, marshalling code, etc. on the fly with |new Function| ) to be as fast as generating all that noise offline in a compiler, so that you can send less code over the wire and spend less time in startup. Once that's possible I think it makes sense to close/dup this and do the same for my other similar report.

Comment 7

5 years ago
It seems likely these issues will be fixed.  Compile-and-go is one of the few artifacts of Function-compilation after the fact.  If, after bug 646597 lands, you re-test and find a new issue, we can just file a new bug.

Comment 8

5 years ago
Tested the change that just recently landed ( https://hg.mozilla.org/mozilla-central/rev/ed26fdbe8444 ) and can confirm that this really helps us with emscripten! I am seeing +300% performance increase in some cases!

Thanks!
You need to log in before you can comment on or make changes to this bug.