Last Comment Bug 922406 - Top-level use of "this" prevents ion compilation
: Top-level use of "this" prevents ion compilation
Status: RESOLVED FIXED
: perf, regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine: JIT (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla45
Assigned To: Jan de Mooij [:jandem]
:
: Hannes Verschore [:h4writer]
Mentors:
: 1212247 (view as bug list)
Depends on: 1125423 1132183
Blocks: 1212247
  Show dependency treegraph
 
Reported: 2013-09-30 16:31 PDT by Jesse Ruderman
Modified: 2015-12-01 15:46 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
regression-range.txt (14.15 KB, text/plain)
2013-09-30 16:32 PDT, Jesse Ruderman
no flags Details
Patch (2.38 KB, patch)
2015-11-24 03:44 PST, Jan de Mooij [:jandem]
shu: review+
Details | Diff | Splinter Review

Description User image Jesse Ruderman 2013-09-30 16:31:11 PDT
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.
Comment 1 User image Jesse Ruderman 2013-09-30 16:32:58 PDT
Created attachment 812348 [details]
regression-range.txt
Comment 2 User image Jesse Ruderman 2013-09-30 17:11:33 PDT
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
Comment 3 User image Jesse Ruderman 2013-09-30 17:36:18 PDT
> (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.
Comment 4 User image Christian Holler (:decoder) 2015-07-11 04:10:23 PDT
Not sure if this is still valid, but putting a needinfo on jandem to be sure.
Comment 5 User image Guilherme Lima 2015-07-11 08:54:13 PDT
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.
Comment 6 User image Jan de Mooij [:jandem] 2015-07-13 03:29:53 PDT
(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.
Comment 7 User image Jan de Mooij [:jandem] 2015-07-17 05:06:03 PDT
(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.
Comment 8 User image Lars T Hansen [:lth] 2015-11-17 08:13:14 PST
*** Bug 1212247 has been marked as a duplicate of this bug. ***
Comment 9 User image Jan de Mooij [:jandem] 2015-11-24 03:44:37 PST
Created attachment 8691334 [details] [diff] [review]
Patch

After all the other changes, fixing this is almost trivial.
Comment 10 User image Shu-yu Guo [:shu] 2015-11-25 15:51:12 PST
Comment on attachment 8691334 [details] [diff] [review]
Patch

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

A triumph.
Comment 12 User image Wes Kocher (:KWierso) 2015-12-01 15:46:16 PST
https://hg.mozilla.org/mozilla-central/rev/33d2af1ba94e

Note You need to log in before you can comment on or make changes to this bug.