Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Top-level use of "this" prevents ion compilation

RESOLVED FIXED in Firefox 45

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: Jesse Ruderman, Assigned: jandem)

Tracking

({perf, regression, testcase})

Trunk
mozilla45
perf, regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Removing the second line of this shell program makes it 9x faster:

var start = dateNow();
this.Array;
for (var i = 0; i < 10000000; ++i) {}
var end = dateNow();
print(Math.floor(end - start) + " ms");

Pretty much anything more complicated than "this;" causes the slowdown, e.g.
  * this.Array;
  * this, {};
  * (function(){})(this);  // often needed for asm modules or strict-mode code

I noticed this slowdown while converting Mersenne Twister to asm style.
(Reporter)

Comment 1

4 years ago
Created attachment 812348 [details]
regression-range.txt
(Reporter)

Comment 2

4 years ago
Regression range depends on the exact second line.  The attachment above is for "this, {};".  Its large regression range seems to be due to hgrange('d2cce982a7c8', '4a6b8dd4dfe3'), # broken virtualenv
(Reporter)

Comment 3

4 years ago
> (function(){})(this)

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/e0bc47d3f814
user:        Jan de Mooij
date:        Mon Dec 17 11:04:08 2012 +0100
summary:     Bug 821682 - Compile DEFVAR, DEFCONST and PICK. r=djvj

> this.Array;

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/c13b9e58eedb
user:        Jan de Mooij
date:        Wed Dec 19 09:43:22 2012 +0100
summary:     Bug 822744 - Compile OBJECT, REGEXP, LAMBDA. r=djvj

> this, {}; 

Large regression range in comment 1.

Updated

2 years ago
Component: JavaScript Engine → JavaScript Engine: JIT
OS: Mac OS X → All
Hardware: x86_64 → All
Not sure if this is still valid, but putting a needinfo on jandem to be sure.
Flags: needinfo?(jdemooij)

Comment 5

2 years ago
With 'this.Array;' I get 260ms on Nightly and 45ms on Chrome 43
Without 'this.Array;' I get 23ms on Nightly and 45ms on Chrome 43.
(Assignee)

Comment 6

2 years ago
(In reply to Christian Holler (:decoder) from comment #4)
> Not sure if this is still valid, but putting a needinfo on jandem to be sure.

It is, thanks for the reminder. Patch in a bit.
(Assignee)

Comment 7

2 years ago
(In reply to Jan de Mooij [:jandem] from comment #6)
> (In reply to Christian Holler (:decoder) from comment #4)
> > Not sure if this is still valid, but putting a needinfo on jandem to be sure.
> 
> It is, thanks for the reminder. Patch in a bit.

Actually, this bug is a fair amount of work to fix: we either (1) should add an extra MBasicBlock/snapshot slot for `this` in global scripts, or (2) fix bug 1125423, so that we have the global `this` for the compartment readily available.

(2) is a bit annoying because it requires changes outside SpiderMonkey, but I think it's where we want to be long-term so I'd prefer it over putting a lot of effort on (1). Unfortunately neither of these is trivial.
Depends on: 1125423
Flags: needinfo?(jdemooij)
(Assignee)

Updated

2 years ago
Blocks: 1212247

Updated

2 years ago
Duplicate of this bug: 1212247
(Assignee)

Comment 9

2 years ago
Created attachment 8691334 [details] [diff] [review]
Patch

After all the other changes, fixing this is almost trivial.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8691334 - Flags: review?(shu)
(Assignee)

Updated

2 years ago
Depends on: 1132183
Comment on attachment 8691334 [details] [diff] [review]
Patch

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

A triumph.
Attachment #8691334 - Flags: review?(shu) → review+

Comment 11

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/33d2af1ba94e

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/33d2af1ba94e
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.