In almost all places js_DecompileValueGenerator in SpiderMonkey is followed by a call to JS_ReportErrorNumber together with JS_GetStringBytes on the result. Thus I suggest to add a function that combine these operations. It should reduce a code size and removes number of calls to JS_GetStringBytes minimizing future patches for bug 366725.

The patch adds js_ReportValueErrorFlags function and js_ReportValueError macros to replace the calls to js_DecompileValueGenerator, JS_GetStringBytes and JS_ReportErrorNumber. In addition, js_DecompileValueGenerator is changed to return malloced bytes representing a copy of the string to avoid allocation new JSString and keeping its deflated bytes in the cache. The patch reduces the size of the optimized build of jsshell by 500 bytes.

The previous attachment was a wrong patch.

Pinging for a review

Comment on attachment 251275 [details] [diff] [review] Implementation v1 for real >+#define js_ReportValueError(cx, errorNumber, spindex, v, fallback) \ >+ ((void)js_ReportValueErrorFlags(cx, JSREPORT_ERROR, errorNumber, \ >+ spindex, v, fallback, NULL, NULL)) >+ >+#define js_ReportValueError2(cx, errorNumber, spindex, v, fallback, arg) \ >+ ((void)js_ReportValueErrorFlags(cx, JSREPORT_ERROR, errorNumber, \ >+ spindex, v, fallback, arg, NULL)) >+ >+#define js_ReportValueError3(cx, errorNumber, spindex, v, fallback, arg, arg2)\ >+ ((void)js_ReportValueErrorFlags(cx, JSREPORT_ERROR, errorNumber, \ >+ spindex, v, fallback, arg, arg2)) Ultra-nit: might eliminate spaces after commas in macro formal parameter lists, avoid running up against \ in last definition (and match style elsewhere -- I think I started squeezing macro param lists because I was looking at enough .i files that the ( actual) pattern hurt my eyes). >@@ -2417,19 +2409,21 @@ interrupt: > js_Disassemble1(cx, script, pc, > PTRDIFF(pc, script->code, jsbytecode), JS_FALSE, > tracefp); > nuses = js_CodeSpec[op].nuses; > if (nuses) { > SAVE_SP_AND_PC(fp); > for (n = -nuses; n < 0; n++) { >- str = js_DecompileValueGenerator(cx, n, sp[n], NULL); >- if (str) { >+ char *bytes = js_DecompileValueGenerator(cx, n, sp[n], >+ NULL); >+ if (bytes) { > fprintf(tracefp, "%s %s", > (n == -nuses) ? " inputs:" : ",", >- JS_GetStringBytes(str)); >+ bytes); >+ JS_free(bytes); Missing cx param to JS_free, here and in the bottom tracing code -- test a build with JS_THREADED_INTERP forced to be undefined? /be

Fixing the problems from the previous comment.

The previous attachment was generated through quilt, not cvs.

I committed the patch from comment 6 to the trunk: Checking in jscntxt.c; /cvsroot/mozilla/js/src/jscntxt.c,v <-- jscntxt.c new revision: 3.103; previous revision: 3.102 done Checking in jscntxt.h; /cvsroot/mozilla/js/src/jscntxt.h,v <-- jscntxt.h new revision: 3.141; previous revision: 3.140 done Checking in jsfun.c; /cvsroot/mozilla/js/src/jsfun.c,v <-- jsfun.c new revision: 3.176; previous revision: 3.175 done Checking in jsinterp.c; /cvsroot/mozilla/js/src/jsinterp.c,v <-- jsinterp.c new revision: 3.324; previous revision: 3.323 done Checking in jsiter.c; /cvsroot/mozilla/js/src/jsiter.c,v <-- jsiter.c new revision: 3.58; previous revision: 3.57 done Checking in jsnum.c; /cvsroot/mozilla/js/src/jsnum.c,v <-- jsnum.c new revision: 3.77; previous revision: 3.76 done Checking in jsobj.c; /cvsroot/mozilla/js/src/jsobj.c,v <-- jsobj.c new revision: 3.324; previous revision: 3.323 done Checking in jsopcode.c; /cvsroot/mozilla/js/src/jsopcode.c,v <-- jsopcode.c new revision: 3.203; previous revision: 3.202 done Checking in jsopcode.h; /cvsroot/mozilla/js/src/jsopcode.h,v <-- jsopcode.h new revision: 3.41; previous revision: 3.40 done Checking in jsxml.c; /cvsroot/mozilla/js/src/jsxml.c,v <-- jsxml.c new revision: 3.139; previous revision: 3.138 done