1869 lambda_out: 1870 if (freeMoreParens) 1871 cx->free(cx->regExpStatics.moreParens); 26.6% 1872 cx->regExpStatics = save; 1873 return ok; 1874 } It seems to be a combination of memcpy overhead and L2 cache misses.

string-unpack-code saves and restores regExpStatics 81474 times (so 81474 * 2 memcpies).

The attached patch makes JSRegExpStatics a pointer in JSContext. Its allocated when its written to, and deallocated when the context dies. When calling into a regexp lambda function, the current state is pushed onto a stack and the state is set to NULL. This is a change from our previous behavior where we copied the state, and then restored it, overwriting any changes that happened. This is all completely unspecified behavior (before and after). If anyone relies on this, they are out of luck. This optimization is a 2ms speedup on its own (unpack-code), and it takes care of a major source of L2 cache misses that another patch experienced (558754, butterfly effect).

Part of the reason that 558754 made such a big impact is that it subtly moved regExpStatics around relative to the start of JSRuntime and JSThreadData, making it no longer 32-byte aligned (which it was before accidentally). That made L2 cache misses more expensive since an extra cache line had to be fetched. We should patrol our code for memcpy and memset use and profile those areas a bit. Note that this memcpy was implicit (*a = *b), not explicit.

Alright so this doesn't really work as intended. We can't just clear the regexp statics and start with a fresh one. This bug tracks back to here: https://bugzilla.mozilla.org/show_bug.cgi?id=288688 To quote Brendan, "Quick and dirty patch next. There's a better patch that should be done, but the patch I'll attach next fixes this, AFAICT." I guess a good starting point is to back out the change, and then fix it differently using the test case as guidance what to fix.

This patch implements a saner memory handling for regExpStatics using luke's js::Vector, so we no longer have to save and restore the statics across lambda replace invocations. This mildly changes behavior for static fields of RegExp.prototype, but I think the previous behavior is just as insane as the new one, just differently insane.

(In reply to comment #6) > Created an attachment (id=440155) [details] > patch > > This patch implements a saner memory handling for regExpStatics using luke's > js::Vector, so we no longer have to save and restore the statics across lambda > replace invocations. This mildly changes behavior for static fields of > RegExp.prototype, RegExp, not RegExp.prototype. > but I think the previous behavior is just as insane as the > new one, just differently insane. It doesn't matter what you think. Does this break the web? http://www.google.com/codesearch?as_q=RegExp\.\%24&btnG=Search+Code&hl=en&as_lang=javascript&as_license_restrict=i&as_license=&as_package=&as_filename=&as_case= I have no idea, but we should not just change semantics and make a wish. At least refine this code search and do some testing. What do other browsers do? /be

Comment on attachment 440155 [details] [diff] [review] patch >+ return false; >+ >+ uintN i = 0; >+ uintN n = cx->regExpStatics.parens.length(); >+ for (; i < n; i++) { Could you scope uintN n to the for loop, make use of the init part of the for-head? Looks like n is not used after the loop. Whew, great to have jsvector.h and C++ after all these years. /be

Test case: javascript:function f() {"xxx".search(/(x)/);return "z";}"yyy".replace(/(y)/g, f);alert(RegExp.$1); us without the patch: y us with the patch: x chrome: x webkit/safari: x opera: x ie8: y

Thanks -- will r+ in a minute. We should release-note the JS change based on comment 9 results, with us moving to the not-IE camp ;-). /be

Already plus'ed! /be

This needs beta coverage, so setting it to P1.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a5pre) Gecko/20100425 Minefield/3.7a5pre I believe this broke Facebook's Inbox. Log in to Facebook. Click Messages. See no messages. This changed between the build I'm using now and the nightly before. The changes include a merge with 59 TM changesets so I had to use TM hourlies from http://hourly-archive.localgho.st/hourly-archive2/tracemonkey-linux/ to find the regression window. http://hg.mozilla.org/tracemonkey/pushloghtml?fromchange=63d72c1ee19f&tochange=35c25547a135 Error Console says Error: a is undefined Source File: http://static.ak.fbcdn.net/rsrc.php/z5N5C/hash/dhdy6xq3.js Line: 24 That long is ridiculously long, and probably doesn't tell you much as it looks like |a| comes from somewhere else. DataStore=window.DataStore||{_storage:{},_elements:{},_tokenCounter:1,_NOT_IN_DOM_CONST:1,_getStorage:function(a){var b;if(typeof a=='string'){b='str_'+a;}else{b='elem_'+(a.__FB_TOKEN||(a.__FB_TOKEN=[DataStore._tokenCounter++]))[0];DataStore._elements[b]=a;}return DataStore._storage[b]||(DataStore._storage[b]={});},_shouldDeleteData:function(a){if(!a.nodeName)return false;try{if(null!=a.offsetParent)return false;}catch(ex){}if(document.documentElement.contains){return !document.documentElement.contains(a);}else return (document.documentElement.compareDocumentPosition(a)&DataStore._NOT_IN_DOM_CONST);},set:function(c,b,d){var a=DataStore._getStorage(c);a[b]=d;return c;},get:function(e,d,c){var b=DataStore._getStorage(e);var f=b[d];if((undefined===f)&&(typeof e.getAttribute=='function')){var a=e.getAttribute('data-'+d);f=(null===a)?undefined:a;}if((c!==undefined)&&(f===undefined))f=b[d]=c;return f;},remove:function(c,b){var a=DataStore._getStorage(c);delete a[b];return c;},cleanup:function(){var b,a;for(b in DataStore._elements){a=DataStore._elements[b];if(DataStore._shouldDeleteData(a)){delete DataStore._storage[b];delete DataStore._elements[b];}}}};

Thanks for the report. The source file you linked contains regexp manipulation, so I am inclined to believe this patch is implicated. Though, I can't see any manipulation of the static regexp state, which this patch touches. I will try to run this in a debug build and see what I can find.

(In reply to comment #16) > Thanks for the report. The source file you linked contains regexp manipulation, > so I am inclined to believe this patch is implicated. Though, I can't see any > manipulation of the static regexp state, which this patch touches. I will try > to run this in a debug build and see what I can find. I did builds using hg bisect, and it identified the following checkin as the regressor, which would seem to exonerate this bug as the cause: The first bad revision is: changeset: 41264:6bda799d061e parent: 41263:72122e42712b parent: 41205:498ea1457799 user: Robert Sayre <sayrer@gmail.com> date: Sat Apr 24 12:56:26 2010 -0400 summary: Merge mozilla-central to tracemonkey.

Nope, that's the changeset that merged this change onto m-c; you've added to the evidence blaming it. :-)

Yes, I should have realized that hg bisect kind of treated the entire thing as one change. I will try backing this change out instead and report back.

I believe this change also broke the server status table on http://wonderproxy.com which is populated by appending table rows via JS. Current builds only display the flag of the proxy's location, ones without this change display the location and status icon. I don't see any regexp usage in the JS though: http://wonderproxy.com/js/addServerStatus.js

OK. I have verified that backing out the patch for this bug fixes both the Facebook messages issue and the http://wonderproxy.com issue.

I can reproduce the bug in facebook. I have confirmed that neither regexp_static_getProperty nor regexp_static_setProperty are hit while the bug happens, so whatever is going on here is not the change to RegExp we intended (that would be worse, it means we couldn't do the optimization we tried to do here). It must be an unintentional bug in the patch. Thanks for the confirmation Bill. This is very useful info.

I backed this out on mozilla-central --- see bug 561700. There were test failures that went unreported. http://hg.mozilla.org/mozilla-central/rev/2968d19b0165

The backout is causing ctypes usage in Chrome Workers to fail. E.g. http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1272268408.1272269169.24747.gz We seem to be crashing in dtoa.c. I'm at a loss to know what to do here. Those tests were enabled before this Andreas patch landed. I don't see anything that landed since this patch that looks like it could have interfered. I could try backing out stuff randomly but that's probably not the best approach. At this point I think I'll just close the tree and wait for someone to come along and debug it. It's late here anyway.

There is a dependency problem in ctypes that manifests that way; can't find the bug number right now, but a clobber should resolve it.

Bug 559056? I didn't know we knew it was a dependency problem.

The ctypes dependency bug was bug 561619. (Clearly shaver and I still read too much bugmail ;-). /be

Well, that sucked. Fortunately I managed to go to bed instead of exploding in bitterness over losing my evening to this.

Dependency problem is pushed to TM; should merge to m-c pretty soon. Apologies that you spent time on this; I also sunk a day into it, without nailing down that it was a build problem. Glad it's fixed now.

No worries, thanks for fixing it.

I decided to chase down where the a is undefined message comes from: #0 js_ReportIsNullOrUndefined (cx=0x105470c00, spindex=1, v=22, fallback=0x0) at ../../../js/src/jscntxt.cpp:1754 #1 0x00000001040f8e68 in js_ValueToNonNullObject (cx=0x105470c00, v=22) at ../../../js/src/jsobj.cpp:6123 #2 0x00000001040c9ffc in js_Interpret (cx=0x105470c00) at jsops.cpp:1450 #3 0x00000001040e395d in js_Invoke (cx=0x105470c00, argc=1, vp=0x105c13ed8, flags=0) at jsinterp.cpp:842 #4 0x00000001040a68ee in js_fun_apply (cx=0x105470c00, argc=1, vp=0x105c13e60) at ../../../js/src/jsfun.cpp:2069 #5 0x00000001040ce4f7 in js_Interpret (cx=0x105470c00) at jsops.cpp:2210 #6 0x00000001040e395d in js_Invoke (cx=0x105470c00, argc=1, vp=0x105c13e38, flags=0) at jsinterp.cpp:842 #7 0x00000001040e3fec in js_InternalInvoke (cx=0x105470c00, obj=0x11e257280, fval=4796594240, flags=0, argc=1, argv=0x11c79ec40, rval=0x7fff5fbfcab8) at jsinterp.cpp:899 #8 0x0000000104046f61 in JS_CallFunctionValue (cx=0x105470c00, obj=0x11e257280, fval=4796594240, argc=1, argv=0x11c79ec40, rval=0x7fff5fbfcab8) at ../../../js/src/jsapi.cpp:4945 #9 0x0000000100990d39 in nsJSContext::CallEventHandler (this=0x1200c81b0, aTarget=0x11b42b400, aScope=0x11e257280, aHandler=0x11de63840, aargv=0x11c79ec08, arv=0x7fff5fbfcc70) at ../../../dom/base/nsJSEnvironment.cpp:2163 #10 0x00000001009c611f in nsGlobalWindow::RunTimeout (this=0x11b42b400, aTimeout=0x11c77eee0) at ../../../dom/base/nsGlobalWindow.cpp:8499 #11 0x00000001009c6672 in nsGlobalWindow::TimerCallback (aTimer=0x11c77ef40, aClosure=0x11c77eee0) at ../../../dom/base/nsGlobalWindow.cpp:8843 #12 0x000000010136aade in nsTimerImpl::Fire (this=0x11c77ef40) at ../../../xpcom/threads/nsTimerImpl.cpp:427 #13 0x000000010136acfc in nsTimerEvent::Run (this=0x11b6f0a70) at ../../../xpcom/threads/nsTimerImpl.cpp:519 #14 0x0000000101363f0e in nsThread::ProcessNextEvent (this=0x104b03980, mayWait=0, result=0x7fff5fbfcf14) at ../../../xpcom/threads/nsThread.cpp:527 #15 0x00000001012f2c1d in NS_ProcessPendingEvents_P (thread=0x104b03980, timeout=20) at nsThreadUtils.cpp:200 #16 0x00000001011a8010 in nsBaseAppShell::NativeEventCallback (this=0x116cc2df0) at ../../../../widget/src/xpwidgets/nsBaseAppShell.cpp:125 #17 0x000000010115ced4 in nsAppShell::ProcessGeckoEvents (aInfo=0x116cc2df0) at ../../../../widget/src/cocoa/nsAppShell.mm:394 #18 0x00007fff880bdf21 in __CFRunLoopDoSources0 () #19 0x00007fff880bc119 in __CFRunLoopRun () #20 0x00007fff880bb8df in CFRunLoopRunSpecific () #21 0x00007fff87a1aada in RunCurrentEventLoopInMode () #22 0x00007fff87a1a83d in ReceiveNextEventCommon () #23 0x00007fff87a1a798 in BlockUntilNextEventMatchingListInMode () #24 0x00007fff88229a2a in _DPSNextEvent () #25 0x00007fff88229379 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] () #26 0x00007fff881ef05b in -[NSApplication run] () #27 0x000000010115c773 in nsAppShell::Run (this=0x116cc2df0) at ../../../../widget/src/cocoa/nsAppShell.mm:747 #28 0x0000000100ed49f2 in nsAppStartup::Run (this=0x116ded0f0) at ../../../../../toolkit/components/startup/src/nsAppStartup.cpp:182 #29 0x0000000100027198 in XRE_main (argc=1, argv=0x7fff5fbfea70, aAppData=0x104b00340) at ../../../toolkit/xre/nsAppRunner.cpp:3536 #30 0x00000001000011f9 in main (argc=1, argv=0x7fff5fbfea70) at ../../../browser/app/nsBrowserApp.cpp:158 (gdb) call js_DumpStackFrame(cx->fp) JSStackFrame at 0x105c146d0 <unnamed function at 0x11dde0620 (JSFunction at 0x11dde0620)> file http://static.ak.fbcdn.net/rsrc.php/z5N5C/hash/dhdy6xq3.js line 24 pc = 0x11b633649 current op: getargprop slots: 0x105c14780 sp: 0x105c14798 = slots + 3 0x105c14780: undefined 0x105c14788: "elem_" 0x105c14790: undefined argv: 0x105c146a8 (argc: 1) this: <Object at 0x11e22e440> rval: undefined flags: none scopeChain: (JSObject *) 0x11e257280 displaySave: (JSStackFrame *) 0x105c145d0 JSStackFrame at 0x105c145d0 <unnamed function at 0x11dde0770 (JSFunction at 0x11dde0770)> file http://static.ak.fbcdn.net/rsrc.php/z5N5C/hash/dhdy6xq3.js line 24 pc = 0x11b633942 current op: call slots: 0x105c14680 sp: 0x105c146b0 = slots + 6 0x105c14680: undefined 0x105c14688: undefined 0x105c14690: undefined 0x105c14698: <unnamed function at 0x11dde0620 (JSFunction at 0x11dde0620)> 0x105c146a0: <Object at 0x11e22e440> 0x105c146a8: undefined argv: 0x105c14578 (argc: 3) this: <Object at 0x11e22e440> rval: undefined flags: none scopeChain: (JSObject *) 0x11e257280 displaySave: (JSStackFrame *) 0x105c14360 JSStackFrame at 0x105c14480 <unnamed function at 0x11e22e8c0 (JSFunction at 0x11dde0cb0)> file http://static.ak.fbcdn.net/rsrc.php/z5N5C/hash/dhdy6xq3.js line 25 pc = 0x11b634e42 current op: call slots: 0x105c14530 sp: 0x105c14590 = slots + 12 0x105c14530: undefined 0x105c14538: undefined 0x105c14540: undefined 0x105c14548: undefined 0x105c14550: undefined 0x105c14558: undefined 0x105c14560: undefined 0x105c14568: <unnamed function at 0x11dde0770 (JSFunction at 0x11dde0770)> 0x105c14570: <Object at 0x11e22e440> 0x105c14578: undefined 0x105c14580: "Event.listeners" 0x105c14588: <Object at 0x11de3cb80> argv: 0x105c14438 (argc: 3) this: <DOMPrototype object at 0x11e22e600> rval: undefined flags: none scopeChain: (JSObject *) 0x11e22e680 displaySave: (JSStackFrame *) 0x7fff5fbfc750 JSStackFrame at 0x105c14360 <function ThreadRow at 0x11eb33bd0 (JSFunction at 0x11eb33bd0)> file http://static.ak.fbcdn.net/rsrc.php/zBW19/hash/be7kdin2.js line 31 pc = 0x11c851775 current op: call slots: 0x105c14410 sp: 0x105c14450 = slots + 8 0x105c14410: <Array object at 0x11de3f9c0> 0x105c14418: 3 0x105c14420: undefined 0x105c14428: <unnamed function at 0x11e22e8c0 (JSFunction at 0x11dde0cb0)> 0x105c14430: <DOMPrototype object at 0x11e22e600> 0x105c14438: undefined 0x105c14440: "mouseover" 0x105c14448: <unnamed function at 0x11de3cac0 (JSFunction at 0x11dda6f50)> argv: 0x105c14338 (argc: 2) this: <Object at 0x11de45f00> rval: undefined flags: constructing computed_this scopeChain: (JSObject *) 0x11e257280 displaySave: (JSStackFrame *) 0x105c14270 JSStackFrame at 0x105c14270 <unnamed function at 0x11df001c0 (JSFunction at 0x11eb37070)> file http://static.ak.fbcdn.net/rsrc.php/zBW19/hash/be7kdin2.js line 35 pc = 0x11c85e57c current op: new slots: 0x105c14320 sp: 0x105c14348 = slots + 5 0x105c14320: undefined 0x105c14328: <function ThreadRow at 0x11eb33bd0 (JSFunction at 0x11eb33bd0)> 0x105c14330: <Object at 0x11de45f00> 0x105c14338: <Object at 0x11deedb00> 0x105c14340: 1.47248e+12 argv: 0x105c14248 (argc: 1) this: <Object at 0x11deedb00> rval: undefined flags: computed_this scopeChain: (JSObject *) 0x11e257280 displaySave: (JSStackFrame *) 0x105c14180 JSStackFrame at 0x105c14180 <unnamed function at 0x11df06d00 (JSFunction at 0x11eb32000)> file http://static.ak.fbcdn.net/rsrc.php/zBW19/hash/be7kdin2.js line 26 pc = 0x11c84c82e current op: call slots: 0x105c14230 sp: 0x105c14250 = slots + 4 0x105c14230: undefined 0x105c14238: <unnamed function at 0x11df001c0 (JSFunction at 0x11eb37070)> 0x105c14240: <Object at 0x11deedb00> 0x105c14248: 1.47248e+12 argv: 0x105c14148 (argc: 2) this: <Object at 0x11de58700> rval: undefined flags: computed_this scopeChain: (JSObject *) 0x11e257280 displaySave: (JSStackFrame *) 0x105c14060 JSStackFrame at 0x105c14060 <unnamed function at 0x11df06cc0 (JSFunction at 0x11eb31f50)> file http://static.ak.fbcdn.net/rsrc.php/zBW19/hash/be7kdin2.js line 26 pc = 0x11c84c6b8 current op: call slots: 0x105c14110 sp: 0x105c14158 = slots + 9 0x105c14110: <Object at 0x11deea880> 0x105c14118: 1.27232e+12 0x105c14120: <Array object at 0x11de45ec0> 0x105c14128: 0 0x105c14130: 1.47248e+12 0x105c14138: <unnamed function at 0x11df06d00 (JSFunction at 0x11eb32000)> 0x105c14140: <Object at 0x11de58700> 0x105c14148: 0 0x105c14150: 1.47248e+12 argv: 0x105c14028 (argc: 0) this: <Object at 0x11de58700> rval: undefined flags: computed_this scopeChain: (JSObject *) 0x11e257280 displaySave: (JSStackFrame *) 0x105c13f60 JSStackFrame at 0x105c13f60 <unnamed function at 0x11df06b40 (JSFunction at 0x11eb31cb0)> file http://static.ak.fbcdn.net/rsrc.php/zBW19/hash/be7kdin2.js line 26 pc = 0x11c84bea2 current op: call slots: 0x105c14010 sp: 0x105c14028 = slots + 3 0x105c14010: 1.27232e+12 0x105c14018: <unnamed function at 0x11df06cc0 (JSFunction at 0x11eb31f50)> 0x105c14020: <Object at 0x11de58700> argv: 0x105c13f20 (argc: 0) this: <Object at 0x11de58700> rval: undefined flags: computed_this scopeChain: (JSObject *) 0x11e257280 displaySave: (JSStackFrame *) 0x7fff5fbfbd20 JSStackFrame at 0x7fff5fbfbd20 <unnamed function at 0x11df00040 (JSFunction at 0x11eb36d90)> file http://static.ak.fbcdn.net/rsrc.php/zBW19/hash/be7kdin2.js line 35 pc = 0x10595fe70 current op: call slots: 0x105c13ef0 sp: 0x105c13f20 = slots + 6 0x105c13ef0: null 0x105c13ef8: undefined 0x105c13f00: undefined 0x105c13f08: undefined 0x105c13f10: <unnamed function at 0x11df06b40 (JSFunction at 0x11eb31cb0)> 0x105c13f18: <Object at 0x11de58700> argv: 0x105c13ee8 (argc: 1) this: <Object at 0x11deedb00> rval: undefined flags: computed_this rooted_argv scopeChain: (JSObject *) 0x11e257280 JSStackFrame at 0x7fff5fbfc750 <unnamed function at 0x11de63840 (JSFunction at 0x11dda6f50)> file http://static.ak.fbcdn.net/rsrc.php/z49PH/hash/9p47jvzp.js line 11 pc = 0x11b446afe current op: apply slots: 0x105c13e50 sp: 0x105c13e80 = slots + 6 0x105c13e50: <Object at 0x11deedb00> 0x105c13e58: <Array object at 0x11de58540> 0x105c13e60: <function apply at 0x11dd9d4d0 (JSFunction at 0x11dd9d4d0)> 0x105c13e68: <unnamed function at 0x11df00040 (JSFunction at 0x11eb36d90)> 0x105c13e70: <Object at 0x11deedb00> 0x105c13e78: <Array object at 0x11de58540> argv: 0x105c13e48 (argc: 1) argsobj: <Object object at 0x11de58440> this: <Window object at 0x11e257280> rval: undefined flags: rooted_argv scopeChain: (JSObject *) 0x11de63680 function on top of the stack: _getStorage: function (a) { var b; if (typeof a == 'string') { b = 'str_' + a; } else { b = 'elem_' + (a.__FB_TOKEN || (a.__FB_TOKEN = [DataStore._tokenCounter++]))[0]; DataStore._elements[b] = a; } return DataStore._storage[b] || (DataStore._storage[b] = {}); }, so a is undefined here.

caller: get: function (e, d, c) { var b = DataStore._getStorage(e); var f = b[d]; if ((undefined === f) && (typeof e.getAttribute == 'function')) { var a = e.getAttribute('data-' + d); f = (null === a) ? undefined : a; } if ((c !== undefined) && (f === undefined)) f = b[d] = c; return f; }, e is undefined ehre.

caller: listen: function (h, p, j, m) { if (typeof h == 'string') h = $(h, true); if (typeof m == 'undefined') m = Event.Priority.NORMAL; if (typeof p == 'object') { var i = {}; for (var o in p) i[o] = Event.listen(h, o, p[o], m); return i; } if (p.match(/^on/i)) throw new TypeError("Bad event name `" + event + "': use `click', not `onclick'."); p = p.toLowerCase(); var k = DataStore.get(h, b, {}); if (f[p]) { var g = f[p]; p = g.base; j = g.wrap(j); } a(h, p); var q = k[p]; if (!(m in q)) q[m] = []; var l = q[m].length, n = new EventHandlerRef(j, q[m], l); q[m].push(n); return n; },

argv: 0x125ee9638 (argc: 3) (gdb) x/3x 0x125ee9638 0x125ee9638: 0x00000016 0x00000000 0x1e3aad84 (gdb) p *(JSString*)0x1e3aad80 Cannot access memory at address 0x1e3aad80 So that's not a string alright.

Note that there was a call to match() immediately before this point. Maybe that clobbered some stuff?

Note, its all valid. I am just leotarded. 64-bit build, so I have to do x/3gx: 0x1054af638: 0x0000000000000016 0x000000011d69b3e4 0x1054af648: 0x000000011d69dd40 (gdb) x/3gx 0x1054af638 0x1054af638: 0x0000000000000016 0x000000011d69b3e4 0x1054af648: 0x000000011d69dd40 (gdb) call js_DumpValue(0x000000011d69b3e4) jsval 0x11d69b3e4 = "mouseover" (gdb) call js_DumpValue(0x000000011d69dd40) jsval 0x11d69dd40 = <unnamed function at 0x11d69dd40 (JSFunction at 0x11dda94d0)> (gdb)

function ThreadRow(a, e) { copy_properties(this, { threadId: e, _controller: a, _dirty: true, root: null, readLinks: [], authors: null, date: null, subject: null, preview: null, selector: null, selectorLabel: null }); this._controller.threadCache.subscribe('GigaboxxThreadCache/dirty/' + e, this.dirty.bind(this), Arbiter.SUBSCRIBE_NEW); this.parent.construct(this, URI('/gigaboxx/templates/ThreadRow.tmpl'), { id: e }); Event.listen(this.root, 'mouseover', CSS.addClass.curry(this.root, 'GBThreadRow_Hover')); we are here ^ Event.listen(this.root, 'mouseout', CSS.removeClass.curry(this.root, 'GBThreadRow_Hover')); var c = [this.selectorLabel, this.icon, this.badge, this.deleteButton]; for (var d = 0; d < c.length; d++) { var b = c[d]; Event.listen(b, 'mouseover', CSS.addClass.curry(this.root, 'GBThreadRow_HoverBlocker')); Event.listen(b, 'mouseout', CSS.removeClass.curry(this.root, 'GBThreadRow_HoverBlocker')); } }

this is the following object, and it does not have a root property: (gdb) call js_DumpObject(0x11ebf7e40) object 0x11ebf7e40 class 0x1042ba440 Object properties: enumerate "_errorView": slot 47 enumerate "_messageHistoryView": slot 46 enumerate "_threadListView": slot 45 enumerate "_errorMsg": slot 44 enumerate "_errorCode": slot 43 enumerate "activeThreadMessagesRoot": slot 42 enumerate "_activeReplyForm": slot 41 enumerate "_hasShownMuffin": slot 40 enumerate "_activeThreadData": slot 39 enumerate "bottomMessagePager": slot 38 enumerate "topMessagePager": slot 37 enumerate "bottomPager": slot 36 enumerate "topPager": slot 35 enumerate "_messagePagerCollection": slot 34 enumerate "_pagerCollection": slot 33 enumerate "_threadList": slot 32 enumerate "_resetPending": slot 31 enumerate "_threadRows": slot 30 enumerate "threadCache": slot 29 enumerate "_viewStateData": slot 28 enumerate "_savedSearches": slot 27 enumerate "_syncCounts": slot 26 enumerate "_pageSize": slot 25 enumerate "_view": slot 24 enumerate "_viewState": slot 23 enumerate "_activeThread": slot 22 enumerate "_counterSubscription": slot 21 enumerate "_postPaintHooks": slot 20 enumerate "_prePaintHooks": slot 19 enumerate "_asyncPendingRequestQueue": slot 18 enumerate "_asyncRequestQueue": slot 17 enumerate "_blockLoadData": slot 16 enumerate "_isDirty": slot 15 enumerate "_state": slot 14 enumerate "_undo_manager": slot 13 enumerate "_contentArea": slot 12 enumerate "toolbarManager": slot 11 enumerate "_pageHeader": slot 10 enumerate "_folderNav": slot 9 enumerate "_config": slot 8 enumerate "_originalPageTitle": slot 7 enumerate "isDarkUser": slot 6 enumerate "userId": slot 5 enumerate "_arbiter": slot 4 enumerate "STATE_MAP": slot 3 enumerate "Debug": slot 2 proto <Object at 0x11ebcdac0> parent <Window object at 0x11e0bb340> slots: 2 = <Object at 0x11ebf7e80> 3 = <Object at 0x11ebf7ec0> 4 = <Object at 0x11d600a80> 5 = "6029215" 6 = 0 7 = "Facebook" 8 = <Object at 0x11ebf7d40> 9 = null 10 = <HTMLDivElement object at 0x11ebf7d80> 11 = <Object at 0x11d6006c0> 12 = <HTMLDivElement object at 0x11ebf7c80> 13 = <Object at 0x11d600fc0> 14 = "PAINT" 15 = false 16 = false 17 = <Array object at 0x11d601100> 18 = <Array object at 0x11d601140> 19 = <Array object at 0x11d601180> 20 = <Array object at 0x11d6011c0> 21 = <Object at 0x11d619bc0> 22 = null 23 = <Object at 0x11d601200> 24 = "thread_list" 25 = 20 26 = false 27 = <Object at 0x11d6015c0> 28 = <Object at 0x11d601600> 29 = <Object at 0x11d601640> 30 = <Object at 0x11d601700> 31 = false 32 = <Object at 0x11d601a80> 33 = <Object at 0x11d612dc0> 34 = <Object at 0x11d612e80> 35 = <Object at 0x11d604bc0> 36 = <Object at 0x11d608740> 37 = <Object at 0x11d60a940> 38 = <Object at 0x11d60cb80> 39 = null 40 = false 41 = null 42 = null 43 = null 44 = null 45 = <Object at 0x11d67e380> 46 = <Object at 0x11d6912c0> 47 = <Object at 0x11d691340>

my two favorite property names in this debugging session so far enumerate "_hasShownMuffin": slot 40 enumerate "isDarkUser": slot 6

Looks like this gets fixed if I properly preserve the state of the regexp statics. Disgusting.

preserve regexp statics

Comment on attachment 441651 [details] [diff] [review] patch >diff --git a/js/src/jsstr.cpp b/js/src/jsstr.cpp >--- a/js/src/jsstr.cpp >+++ b/js/src/jsstr.cpp >@@ -1764,16 +1764,29 @@ PushRegExpSubstr(JSContext *cx, const JS > size_t off = sub.chars - whole->chars(); > JSString *str = js_NewDependentString(cx, whole, off, sub.length); > if (!str) > return false; > *sp++ = STRING_TO_JSVAL(str); > return true; > } > >+class PreserveRegExpStatics { >+ JSContext *cx; >+ JSRegExpStatics save; Blank line here. >+ public: >+ PreserveRegExpStatics(JSContext *cx) : cx(cx), save(cx) { >+ save.copy(cx->regExpStatics); >+ } [snip] >@@ -1795,16 +1808,18 @@ FindReplaceLength(JSContext *cx, Replace > > if (!rdata.invokevp) { > rdata.invokevp = js_AllocStack(cx, 2 + argc, &rdata.invokevpMark); > if (!rdata.invokevp) > return false; > } > jsval* invokevp = rdata.invokevp; > >+ PreserveRegExpStatics save(cx); >+ Toldyaso... but wait, didn't *you* tell *me* this would break our Mozilla/IE backward compat by emulating WebKit? /be

I don't intend to land this patch with the fix. I will rewrite the regexp code to use a write barrier in the static code instead. test() just remembers the regexp+lastIndex and the string, not the match results. This is the only state that needs to be preserved. When the static state is actually used, I will reify it using the saved information (by re-executing the regexp). This will speed up the sane case (and probably SS) and penalize using the static state.

backed out of TM as well http://hg.mozilla.org/tracemonkey/rev/eb4e203caf33

Reexecuting the regexp has observable effects if //g, namely lastIndex. In any event re-executing may take a long time. What is the performance effect of the patch you just attached? /be

Yeah, of course, lastIndex has to be cached as well. Its a quad (input, reobj, lastIndex, match). match is lazily NULL initially. Re-executing in case someone uses RegExp.* seems ok as a de-optimized path. We should evangelize against the use of that. Its totally non-standed. I mocked up a patch. The biggest problem is leftContext and rightContext. Those can't be extracted from the match object, so I would have to save them somewhere else. That's uberlame. I didn't measure but I think the additional patch would still keep things performance neutral. The new proposal should be a win. Not clear by how much though. Maybe not worth it?

Smells like your approach will save about as many bytes, and cost in code size and complexity. Doesn't seem worth it, as a gut-check. /be

Brendan is right. Don't optimize without profiles. I will land what we have, and we can do more later.

Waldo backed out. Re-landing same patch. I caused a recompile that exposed a missing virtual destructor in some gc patch (I think). http://hg.mozilla.org/tracemonkey/rev/ac8dcb4886a9

Follow-up: http://hg.mozilla.org/tracemonkey/rev/5ffbb4cf9947

The saga continues. Brendan, could I get a retroactive r+ for this? we have to poke the gc when losing the value of input.

Put the opening { on its own line! Why did you remove the gcPoke? /be

which one?

Comment on attachment 442282 [details] [diff] [review] patch I was asking why we lost a net poke, requiring the followup fix -- gal 'splained on irc. The methods moved to out-of-line, even though clearRoots is small. I missed the reason for that. No biggie, just curious. r=me with out-of-line method definition brace style fixed. /be

#56: JSContext wasn't defined at that place (it comes later), so cx->runtime didn't work. I could have moved them below JSContext. Maybe worth it. I will profile this a bit and see.