See bug 492918 comment 6.

Dep of blocker means blocker. I sure hope a lot of sites on the web use closures this way, given how many hoops we're jumping through for Dromaeo.

Not just Dromaeo, JQuery too. Also the Go game, IIRC. Probably others. Be good to have a short list. /be

The patch gives about a 3.75x speedup on the microbenchmark. It is currently cycling on try.

JSOP_BINDNAME Results for the Web (GMail, GDocs, Facebook, CNN, MSNBC, few dromaeo tests) + Browser Startup. I tested it with "if(obj != JS_GetGlobalForObject(cx, cx->fp->scopeChain))" right before the object is pushed on the stack in the interpreter. Overall: bindname obj != global Object: 135735 bindname obj == global Object: 3698

Gregor: Can you print JS_GET_CLASS(cx, obj)->name too? /be

Refreshed to apply on current tip (after native getter/setter patch).

Comment on attachment 390912 [details] [diff] [review] Patch >+/* Trailing whitespace waaaahmbulance has been called -- I gave 'em your credit card #. >+ * js_SetCallArg and js_SetCallVar are extern fastcall copies of the ..12345678901234567890123456789012345678901234567890123456789012345678901234567890 >+ * setter functions. These versions are required in order to set call vars >+ * from traces. Wrap before col. 80 but not so early or ragged right as above. >+ */ >+extern JSBool JS_FASTCALL >+js_SetCallArg(JSContext *cx, JSObject *obj, jsid id, jsval vp); >+ >+extern JSBool JS_FASTCALL >+js_SetCallVar(JSContext *cx, JSObject *obj, jsid id, jsval vp); Should use v as final parameter's name, not vp. >+ if (sprop->setter == SetCallArg) { >+ ci = &js_SetCallArg_ci; Underindented by one. Whoops, you put up a new patch that fixes this. Closing out this review, hope it's mostly useful still. >+ } else if (sprop->setter == SetCallVar) { >+ ci = &js_SetCallVar_ci; >+ } else { >+ ABORT_TRACE("can't trace special CallClass setter"); >+ } No need to brace all of this if-else tree. >+ Trailing space, scour the patch for it. >+ LIns* v_ins = get(&r); >+ box_jsval(r, v_ins); >+ LIns* args[] = { >+ v_ins, >+ INS_CONST(SPROP_USERID(sprop)), >+ obj_ins, >+ cx_ins >+ }; >+ LIns* call_ins = lir->insCall(ci, args); >+ guard(true, >+ addName(lir->ins2(LIR_eq, call_ins, INS_CONST(JS_TRUE)), Use false for expected result and lir->ins_eq0 here, eh? >@@ -10920,8 +10946,15 @@ > } > } > >- if (obj != globalObj) >- ABORT_TRACE("JSOP_BINDNAME must return global object on trace"); >+ /* >+ * If obj is a js_CallClass object, then we are tracing a reference >+ * to an upvar in a heavyweight function. We cannot reach this point >+ * of the trace with a different call object because of the guard >+ * on the function call, so we can assume the result of the bindname >+ * is constant on this trace. ..12345678901234567890123456789012345678901234567890123456789012345678901234567890 Hyper-nit: Wrapping could be closer to col. 80 but at least not ragged right. Vim'll rewrap it for you for free. /be

Comment on attachment 390963 [details] [diff] [review] Patch 2 r=me with nits picked, thanks. /be

(In reply to comment #6) > Gregor: Can you print JS_GET_CLASS(cx, obj)->name too? > > /be Sure... 1323946 Call 4531 With 2 XULElement

Pushed with nits fixed as 60a9ef4e1a3d.

Mass change: adding fixed1.9.2 keyword (This bug was identified as a mozilla1.9.2 blocker which was fixed before the mozilla-1.9.2 repository was branched (August 13th, 2009) as per this query: http://is.gd/2ydcb - if this bug is not actually fixed on mozilla1.9.2, please remove the keyword. Apologies for the bugspam)