Javascript performance 30x lower than in other browsers

RESOLVED FIXED in Firefox 37

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: ivan.kuckir, Assigned: jandem)

Tracking

({perf, reproducible, testcase})

35 Branch
mozilla38
perf, reproducible, testcase
Points:
---

Firefox Tracking Flags

(firefox37 fixed, firefox38 fixed)

Details

Attachments

(3 attachments)

(Reporter)

Description

4 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.99 Safari/537.36

Steps to reproduce:

I am creating a webapp for editing images. I "compile" my JS code before publishing with Closure Compiler (http://closure-compiler.appspot.com/home). "Compiled" code works as fast as original code in Chrome and IE 11, but it runs 30x slower in FF.

Here is a demo http://jsperf.com/pp-blending . Why is "f.blend.mainCnC" so slow in FF? Do you think you can make FF as fast as IE, or do you expect programmers to write special versions of code just for FF?

Comment 1

4 years ago
Created attachment 8552401 [details]
Compiled vs. uncompiled JS

Updated

4 years ago
Component: Untriaged → JavaScript Engine
Keywords: perf, reproducible, testcase
OS: Windows 7 → All
Product: Firefox → Core
Hardware: x86_64 → All
(Assignee)

Comment 2

4 years ago
Created attachment 8552422 [details]
Shell testcase

Thanks for the report!

I can reproduce this in the shell with the attached testcase:

Original 326
Compiled: 8574

d8:

Original 243
Compiled: 256

I'll look into it.
(Assignee)

Updated

4 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 3

4 years ago
Created attachment 8552496 [details] [diff] [review]
Patch

The problem is that we have bad type information and think one of the operands of a MMul could be an object and we deoptimize. This in turns leads to bad type information for other Add/Mul/Sub/Div operations (their slow paths all show up in the profile).

This patch removes the unnecessary might-be-object and might-be-symbol checks, so that we will fall back to information from Baseline caches. This is fine because MToInt32 and MToDouble now properly set the Guard flag if the input could be a symbol or object.

With that I get:

Original: 332
Compiled: 210

So this fixes the problem and Compiled is now faster than Original even (and faster than d8). Still have to find out why Original is slower now.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8552496 - Flags: review?(hv1989)
(Assignee)

Comment 4

4 years ago
(In reply to Jan de Mooij [:jandem] from comment #3)
> Original: 332
> Compiled: 210
> 
> So this fixes the problem and Compiled is now faster than Original even (and
> faster than d8). Still have to find out why Original is slower now.

It seems to be a regalloc issue. With LSRA we have some really big move groups. If I use Backtracking I get:

Original: 221
Compiled: 218

So with BT, Compiled becomes a little slower but Original is way faster and both are faster than d8/Chrome. Hopefully BT will be enabled for all JIT code soon.
Depends on: 826741
Attachment #8552496 - Flags: review?(hv1989) → review+
(Assignee)

Updated

4 years ago
Component: JavaScript Engine → JavaScript Engine: JIT
https://hg.mozilla.org/mozilla-central/rev/65aea1c559fd
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
(Assignee)

Comment 7

4 years ago
Comment on attachment 8552496 [details] [diff] [review]
Patch

As we're still pretty early in the cycle, we should probably uplift this to 37 as the patch is just removing code and can make some operations a lot faster.

Approval Request Comment
[Feature/regressing bug #]: Older Ion issue.
[User impact if declined]: Websites/games a lot slower in some cases.
[Describe test coverage new/current, TreeHerder]: Tested on treeherder.
[Risks and why]: Low risk, the patch just removes some now-unnecessary checks.
[String/UUID change made/needed]: None.
Attachment #8552496 - Flags: approval-mozilla-aurora?
status-firefox37: --- → affected
status-firefox38: --- → fixed
Attachment #8552496 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Depends on: 1140890
(Assignee)

Updated

4 years ago
No longer depends on: 1140890
You need to log in before you can comment on or make changes to this bug.