Closed Bug 420426 Opened 14 years ago Closed 14 years ago

10% sunspider regression as a result of bug 418069

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9beta4

People

(Reporter: mrbkap, Assigned: brendan)

References

Details

(Keywords: perf, regression)

Attachments

(2 files, 2 obsolete files)

Assignee: nobody → general
Component: XPConnect → JavaScript Engine
Flags: blocking1.9+
OS: Linux → All
Priority: -- → P1
QA Contact: xpconnect → general
Hardware: PC → All
Target Milestone: --- → mozilla1.9beta4
Assignee: general → brendan
Status: NEW → ASSIGNED
Attachment #306672 - Flags: review?(mrbkap)
I left ComputeThis static in jsinterp.c and called only from js_ComputeThis. It's a tail call, gcc on Mac seems to inline ComputeThis on this account. I can inline by hand if you insist, but I'd rather not at this stage.

/be
Comment on attachment 306672 [details] [diff] [review]
attempt to get ComputeThis out of all critical paths

It'd be good to run this on the JS testsuite, but it looks good to me.
Attachment #306672 - Flags: review?(mrbkap) → review+
Attachment #306672 - Flags: approval1.9b4+
Fixed:

js/src/jsinterp.c 3.454

/be
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Depends on: 420436
nye (MozillaTest) hit the assertion:

Assertion failure: !obj->map->ops->thisObject, at /home/andrew/tbox/SeaMonkey-mochi/Linux_2.6.22.14-72.fc6_Depend/mozilla/js/src/jsinterp.c:4208

running dom/tests/mochitest/ajax/prototype/test_Prototype.html
failures in shell and windows:

<http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTest/1204357140.1204358350.24510.gz&fulltext=1>

<http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTest/1204357140.1204358318.24418.gz&fulltext=1>

e4x/Regress/regress-370016.js EXIT STATUS: CRASHED 0 signal 6, reason: Assertion failure: OBJECT_IS_XML(cx, obj), at jsinterp.c:6324 BUGNUMBER: 370016 STATUS: with (nonxmlobj) function::

ecma/ExecutionContexts/10.1.4-10.js expected: Expected exit 0 actual: Actual exit 3, signal 0 reason: ./ecma/ExecutionContexts/10.1.4-10.js:91: TypeError: Number.prototype.toString called on incompatible With 10.1.4-10 Scope Chain and Identifier Resolution 

ecma/ExecutionContexts/10.2.2-2.js eval(new MyObject('hello').getClass()) expected: [object Object] actual: [object With] reason: wrong value

ecma/ExecutionContexts/10.2.2-2.js new MyObject('hello').getClass() expected: [object Object] actual: [object With] reason: wrong value

ecma/GlobalObject/15.1.2.1-2.js expected: Expected exit 0 actual: Actual exit 3, signal 0 reason: ./ecma/GlobalObject/15.1.2.1-2.js:64: TypeError: Date.prototype.getUTCMonth called on incompatible With 15.1.2.1-2 eval(x)

ecma/Statements/12.10.js expected: Expected exit 0 actual: Actual exit 3, signal 0 reason: ./ecma/Statements/12.10.js:59: TypeError: Number.prototype.valueOf called on incompatible With 12.10-1 The with statement

ecma_2/Statements/try-006.js EXIT STATUS: CRASHED 0 signal 6, reason: Assertion failure: !obj->map->ops->thisObject, at jsinterp.c:4208 try-006 The try statement

js1_5/Scope/scope-003.js EXIT STATUS: CRASHED 0 signal 6, reason: Assertion failure: OBJ_GET_CLASS(cx, obj) != &js_CallClass, at jsinterp.c:4207

this needs to come back out.
backed out. shoot me when you wake up.

cvs update -j3.454 -j3.453 jsinterp.c
RCS file: /cvsroot/mozilla/js/src/jsinterp.c,v
retrieving revision 3.454
retrieving revision 3.453
Merging differences between 3.454 and 3.453 into jsinterp.c

Checking in jsinterp.c;
/cvsroot/mozilla/js/src/jsinterp.c,v  <--  jsinterp.c
new revision: 3.455; previous revision: 3.454

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
bc, would appreciate your testing this to pieces.

/be
Attachment #306672 - Attachment is obsolete: true
Attachment #306754 - Flags: review?(mrbkap)
Attachment #306754 - Attachment is obsolete: true
Attachment #306755 - Flags: review?(mrbkap)
Attachment #306754 - Flags: review?(mrbkap)
On linux, I tested the regressions 

e4x/Regress/regress-354998.js
e4x/Regress/regress-370016.js
ecma_2/Statements/try-006.js
ecma/ExecutionContexts/10.1.4-10.js
ecma/ExecutionContexts/10.2.2-2.js
ecma/GlobalObject/15.1.2.1-2.js
ecma/ObjectObjects/15.2.4.2.js
ecma/Statements/12.10.js
js1_5/Scope/scope-003.js

with the original patch here (this.patch) and the patch in bug 420426 (more-this.patch) and the shell passed, the browser failed 

ecma/ObjectObjects/15.2.4.2.js

myvar = this;  myvar.toString = Object.prototype.toString; myvar.toString() expected: [object Window] actual: [object XPCCrossOriginWrapper] reason: wrong value

var MYVAR = new Object( this ); MYVAR.toString() expected: [object Window] actual: [object XPCCrossOriginWrapper] reason: wrong value

I think that is unrelated to this bug since I see them (and other regression) in the opt browser tests from after the backout.

I haven't done a full test run though. 
Attachment #306755 - Flags: review?(mrbkap) → review+
Comment on attachment 306755 [details] [diff] [review]
oops, right patch here

bc confirmed over irc that this fixes the js testsuite regressions. Need to unregress Tsspider stat!

/be
Attachment #306755 - Flags: approval1.9b4?
Comment on attachment 306755 [details] [diff] [review]
oops, right patch here

a=mconnor on behalf of drivers
Attachment #306755 - Flags: approval1.9b4? → approval1.9b4+
Fixed:

js/src/jsapi.h 3.186
js/src/jsinterp.c 3.456
js/src/jsinterp.h 3.78
js/src/jsnum.c 3.109

/be
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
Depends on: 420585
tests covered by sunspider and comment 11
Flags: in-testsuite+
Flags: in-litmus-
v 1.9.0
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.