The default bug view has changed. See this FAQ.

IonMonkey: Don't try to load Ion code for disabled JSScripts.

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: sstangl, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Created attachment 630017 [details] [diff] [review]
Patch.

kraken-1.1's audio-beat-detection is about twice as slow as the competition. I measured that slightly more than half (484 of 780ms) of our total execution time is spent in the function at audio-beat-detection-data:2147, which is too long to be compiled by Ion. However, we always enter it from Ion.

This first simple patch doesn't go through checking whether Ion code has been compiled for scripts that have already been disabled. Brings test from 780 to 750ms, about 4%.
Attachment #630017 - Flags: review?(dvander)
Comment on attachment 630017 [details] [diff] [review]
Patch.

Review of attachment 630017 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ion/CodeGenerator.cpp
@@ +561,5 @@
> +            Label notPrimitive;
> +            masm.branchTestPrimitive(Assembler::NotEqual, JSReturnOperand, &notPrimitive);
> +            masm.loadValue(Address(StackPointer, unusedStack), JSReturnOperand);
> +            masm.bind(&notPrimitive);
> +        }

You might be able to share this too (it's repeated) but I don't know if its worth it.
Attachment #630017 - Flags: review?(dvander) → review+
Nice improvement. 

I was wondering if it would make sense to keep track of scripts that get disabled?
When script A gets compiled and script B has not compiled yet, the extra code (causing the slowdown will get added). Eventualy when Ion tries to compile B, but doesn't succeed (Ion decides it is to long or try is used ...) the extra code in script A will remain. Causing the slowdown ...

What about recompiling script A when script B is set to ION_DISABLED_SCRIPT?
(Reporter)

Comment 3

5 years ago
(In reply to Hannes Verschore from comment #2)
> What about recompiling script A when script B is set to ION_DISABLED_SCRIPT?

The overhead of running the checking instructions is likely less than the cost of invalidating and recompiling script A. We tend to re-JIT functions pretty frequently anyway (even with the patch preserving code on GC), so callsites will pretty rapidly converge on the right configuration without additional help.
(Reporter)

Comment 4

5 years ago
http://hg.mozilla.org/projects/ionmonkey/rev/ff28f1696c5a
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.