Closed Bug 790628 Opened 9 years ago Closed 9 years ago

IonMonkey: GetElementMonitored where it is not needed.


(Core :: JavaScript Engine, defect)

Not set





(Reporter: paraboul, Unassigned)


(Blocks 1 open bug)


(Whiteboard: [ion:p2])


(2 files)

2.17 KB, application/octet-stream
533 bytes, application/octet-stream
Attached file ray.js
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20120904124322

Steps to reproduce:

Using the attached file on the shell.

Actual results:

There is a huge perf regression :

Anthonys-MacBook-Pro:shell anthonycatel$ ./js -b ray.js
runtime = 839.505 ms
Anthonys-MacBook-Pro:shell anthonycatel$ ./js -b --no-ion ray.js
runtime = 382.523 ms
Ps :
- 64bit OSX
- Release build (vanilla ./configure)
Weird I can't reproduce this on linux x64.

tom@tom-linux:~/inbound/js/src/build-opt$ ./js -b --no-ion ~/Downloads/ray.js 
runtime = 245.698 ms
tom@tom-linux:~/inbound/js/src/build-opt$ ./js -b ~/Downloads/ray.js 
runtime = 233.747 ms
Blocks: IonSpeed
Summary: IonMonkey huge perf regression → IonMonkey: huge perf regression
I'm using ca3fa3fbe62a.

Side note :

I'm getting 50ms if I change the line 24 with :

{width: 320, height: 240, data: new Uint8ClampedArray(320*240*4)}

instead of :

{width: 160, height: 120, data: new Uint8ClampedArray(160*120*4)}

I'm getting :

Anthonys-MacBook-Pro:shell anthonycatel$ ./js -b --no-ion ray.js
runtime = 149.954 ms
Anthonys-MacBook-Pro:shell anthonycatel$ ./js -b ray.js
runtime = 72.108 ms

So a better perf with a bigger array. (from 829ms to 72ms)
No longer blocks: IonSpeed
Summary: IonMonkey: huge perf regression → IonMonkey huge perf regression
Blocks: IonSpeed
Summary: IonMonkey huge perf regression → IonMonkey: huge perf regression
I can reproduce the original issue on Linux x64, Intel(R) Core(TM) i7-2820QM CPU @ 2.30GHz.  I obtain the following results:

ray.js being the attached version, ray-2.js being the modified version as suggested in comment 3.

./js -b --no-jm _build/ray-2.js
runtime = 115.722 ms
./js -b --no-ion _build/ray-2.js
runtime = 119.365 ms
./js -b _build/ray-2.js
runtime = 54.862 ms

./js -b --no-jm _build/ray.js
runtime = 875.211 ms
./js -b --no-ion _build/ray.js
runtime = 272.428 ms
./js -b _build/ray.js
runtime = 604.619 ms
Ever confirmed: true
Attached file Reduced testcase
Here is a reduced testcase
I often run the benchmark to have an overview of Nightly optimizations.

The results I had with the build of today morning were extremly different of usually.

Those last 4 months my score have grown from 4900 to ~5750 with almost the same results : really good NovierStrokes, stable regexp at 1000pts, stable crypto at 12000.

When I benched today the first global score was really huge (almost 6800pts) but not stable : the more I relaunch the bench, the more the global and the regexp scores go down (each time lower and after a dozen of consecutivs benchs, the regexp score is at 36pts!).

The crypto bench lost almost 2500 pts and seems stable at 9500 (12000 yesterday).

The good point is that EarlyBoyer's score increased significantly (If I have good memories).

My reflex was to profile the bench and I've got a weird result when I analyzed it : Ffx crashed

Here is the report : bp-3d3e9ddd-bb9f-49f0-a178-d34f52120912 and I repeated it once to check if I could reproduce it bp-1f19349b-6ef8-4234-a974-4a3602120912

This line pushed to post here :
7 	EnterIon 	js/src/ion/Ion.cpp:1325

I didn't remember enough scores to tell you more but I prefer to communicate.

Hope it will help.
Running ./js -b _build/ray.js under valdring reveals that we are using a VM call which relies on GetPCScript.  This function does not appear in other profiles (or not big enough to be a concern).
Summary: IonMonkey: huge perf regression → IonMonkey: GetElementMonitored where it is not needed.
(In reply to Jean Claveau from comment #6)
> …

This seems unrelated for now, so it has been moved to a dedicated bug: Bug 790663.
Thanks for reporting.
With Bug 794679 landed, perf is as follows:

ion: 324ms
--no-ion: 419ms
Closed: 9 years ago
Resolution: --- → FIXED
Awesome. Thanks.
You need to log in before you can comment on or make changes to this bug.