IonMonkey: GetElementMonitored where it is not needed.

RESOLVED FIXED

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: paraboul, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ion:p2])

Attachments

(2 attachments)

2.17 KB, application/octet-stream
Details
533 bytes, application/octet-stream
Details
(Reporter)

Description

6 years ago
Created attachment 660430 [details]
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
(Reporter)

Comment 1

6 years ago
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: 705294
Summary: IonMonkey huge perf regression → IonMonkey: huge perf regression
(Reporter)

Comment 3

6 years ago
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: 705294
Summary: IonMonkey: huge perf regression → IonMonkey huge perf regression
(Reporter)

Updated

6 years ago
Blocks: 705294
(Reporter)

Updated

6 years ago
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
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 5

6 years ago
Created attachment 660451 [details]
Reduced testcase

Here is a reduced testcase

Comment 6

6 years ago
I often run the http://v8.googlecode.com/svn/data/benchmarks/v7/run.html 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 	libxul.so 	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.
Whiteboard: [ion:p2]
With Bug 794679 landed, perf is as follows:

ion: 324ms
--no-ion: 419ms
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Reporter)

Comment 10

6 years ago
Awesome. Thanks.
You need to log in before you can comment on or make changes to this bug.