Closed Bug 445893 Opened 11 years ago Closed 11 years ago

Use the property cache for JSOP_NAMEINC, etc.

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9.1a1

People

(Reporter: sayrer, Assigned: brendan)

Details

(Keywords: perf)

Attachments

(1 file)

Good for a performance win.

See TM changeset 16321:ead9e74ecc9c.
Flags: wanted1.9.1+
Keywords: perf
http://hg.mozilla.org/tracemonkey/index.cgi/rev/ead9e74ecc9c

/be
Status: NEW → ASSIGNED
OS: Mac OS X → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.9.1a1
Attached patch fixSplinter Review
Attachment #330181 - Flags: review?(shaver)
(By which I mean, "r=shaver".)
Fixed:

changeset:   16048:5c009a853d70
tag:         tip
user:        Brendan Eich <brendan@mozilla.org>
date:        Fri Jul 18 19:16:15 2008 -0700
summary:     Property-cache JSOP_NAMEINC etc. direct slot case (445893, r=shaver).

/be
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Backed out:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/337a43c87296

It hit a hang on the Windows tinderboxes (hang could mean crash dialog) and a fatal JS Assert (crash dialog) on my Windows debug build:

It was the (3) == len assert inside the DO_NEXT_OP(JSOP_INCNAME_LENGTH) line that you added -- that assert is only present in the !JS_THREADED_INTERP case, so it wouldn't show up on gcc-based platforms.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The hang in question was running xpcshell unit tests -- in particular, I think while running xpcom/unit/test_bug374754.js.
The stack of the assert was:

_DbgBreakPoint@0()	
JS_Assert(const char * s, const char * file, int ln) Line 59	C++
js_Interpret(JSContext * cx) Line 4013	C++
js_Execute(JSContext * cx, JSObject * chain, JSScript * script, JSStackFrame * down, unsigned int flags, long * result) Line 1550	C++
JS_ExecuteScript(JSContext * cx, JSObject * obj, JSScript * script, long * rval) Line 4932	C++
ProcessFile(JSContext * cx, JSObject * obj, const char * filename, _iobuf * file, int forceTTY) Line 673	C++
Process(JSContext * cx, JSObject * obj, const char * filename, int forceTTY) Line 752	C++
ProcessArgs(JSContext * cx, JSObject * obj, char * * argv, int argc) Line 871	C++
main(int argc, char * * argv, char * * envp) Line 1459	C++


In ProcessFile:

filename	0x00ee8f8d "c:/builds/mozilla-central/mozilla/tools/test-harness/xpcshell-simple/execute_test.js"	const char *


In JS_Assert:

+		s	0x101426fc "(3) == len"	const char *
+		file	0x101426c4 "c:/builds/mozilla-central/mozilla/js/src/jsinterp.cpp"	const char *



I still have it in the debugger if you need more.
Fixed again:

changeset:   16051:a7338e058704
tag:         tip
user:        Brendan Eich <brendan@mozilla.org>
date:        Fri Jul 18 22:53:22 2008 -0700
summary:     Property-cache JSOP_NAMEINC etc. direct slot case (445893, r=shaver).

The windows version of DO_NEXT_OP has a false degree of freedom -- its actual param must == len, if not be len. Grrr. Eagerly awaiting cross-platform inline + call threading!

/be
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.