From d9c8e2ea860d4b8aec1ecc335e8e855fc75325cf Mon Sep 17 00:00:00 2001 From: "igor@mir2.org" Date: Wed, 23 Jan 2008 01:38:32 -0800 Subject: [PATCH] Bug 411722: faster implementation of js_GetLocalNameArray. r,a=brendan --- js/src/jsfun.c | 326 ++++++++++++++++++++++++---------------------- js/src/jsfun.h | 32 +++-- js/src/jsopcode.c | 20 ++- 3 files changed, 202 insertions(+), 176 deletions(-) diff --git a/js/src/jsfun.c b/js/src/jsfun.c index b654780155d3..3df5a7807a40 100644 --- a/js/src/jsfun.c +++ b/js/src/jsfun.c @@ -70,6 +70,10 @@ # include "jsiter.h" #endif +#if JS_HAS_XDR +# include "jsxdrapi.h" +#endif + /* Generic function/call/arguments tinyids -- also reflected bit numbers. */ enum { CALL_ARGUMENTS = -1, /* predefined arguments local variable */ @@ -749,7 +753,9 @@ call_enumerate(JSContext *cx, JSObject *obj) JSFunction *fun; uintN n, i, slot; void *mark; - JSAtom **names, *name; + jsuword *names; + JSBool ok; + JSAtom *name; JSObject *pobj; JSProperty *prop; jsval v; @@ -765,17 +771,21 @@ call_enumerate(JSContext *cx, JSObject *obj) * local functions. */ fun = fp->fun; - n = fun->nargs + fun->u.i.nvars; + n = JS_GET_LOCAL_NAME_COUNT(fun); if (n == 0) return JS_TRUE; mark = JS_ARENA_MARK(&cx->tempPool); - names = js_GetLocalNames(cx, fun, &cx->tempPool, NULL); - if (!names) + + /* From this point the control must flow through the label out. */ + names = js_GetLocalNameArray(cx, fun, &cx->tempPool); + if (!names) { + ok = JS_FALSE; goto out; + } for (i = 0; i != n; ++i) { - name = names[i]; + name = JS_LOCAL_NAME_TO_ATOM(names[i]); if (!name) continue; @@ -783,10 +793,9 @@ call_enumerate(JSContext *cx, JSObject *obj) * Trigger reflection by looking up the name of the argument or * variable. */ - if (!js_LookupProperty(cx, obj, ATOM_TO_JSID(name), &pobj, &prop)) { - names = NULL; + ok = js_LookupProperty(cx, obj, ATOM_TO_JSID(name), &pobj, &prop); + if (!ok) goto out; - } /* * At this point the call object always has a property corresponding @@ -800,10 +809,11 @@ call_enumerate(JSContext *cx, JSObject *obj) v = (i < fun->nargs) ? fp->argv[i] : fp->vars[i - fun->nargs]; LOCKED_OBJ_SET_SLOT(obj, slot, v); } + ok = JS_TRUE; out: JS_ARENA_RELEASE(&cx->tempPool, mark); - return names != NULL; + return ok; } static JSBool @@ -1159,6 +1169,9 @@ fun_convert(JSContext *cx, JSObject *obj, JSType type, jsval *vp) #include "jsxdrapi.h" +static JSBool +XDRLocalNames(JSXDRState *xdr, JSFunction *fun); + /* XXX store parent and proto, if defined */ static JSBool fun_xdrObject(JSXDRState *xdr, JSObject **objp) @@ -1166,7 +1179,7 @@ fun_xdrObject(JSXDRState *xdr, JSObject **objp) JSContext *cx; JSFunction *fun; uint32 nullAtom; /* flag to indicate if fun->atom is NULL */ - uintN nargs, nvars, n; + uintN nargs, nvars; uint32 localsword; /* word to xdr argument and variable counts */ uint32 flagsword; /* originally only flags was JS_XDRUint8'd */ JSTempValueRooter tvr; @@ -1218,81 +1231,8 @@ fun_xdrObject(JSXDRState *xdr, JSObject **objp) } /* do arguments and local vars */ - if (fun->object && (n = nargs + nvars) != 0) { - void *mark; - uintN i; - uintN bitmapLength; - uint32 *bitmap; - JSAtom **names, *name; - JSLocalKind localKind; - - mark = JS_ARENA_MARK(&xdr->cx->tempPool); - - /* From this point the control must flow through label release_mark. */ - bitmapLength = JS_HOWMANY(n, JS_BITS_PER_UINT32); - if (xdr->mode == JSXDR_ENCODE) { - names = js_GetLocalNames(xdr->cx, fun, &xdr->cx->tempPool, &bitmap); - if (!names) { - ok = JS_FALSE; - goto release_mark; - } - } else { -#ifdef __GNUC__ - names = NULL; /* quell GCC uninitialized warning */ -#endif - JS_ARENA_ALLOCATE_CAST(bitmap, uint32 *, &xdr->cx->tempPool, - bitmapLength * sizeof *bitmap); - if (!bitmap) { - js_ReportOutOfScriptQuota(xdr->cx); - ok = JS_FALSE; - goto release_mark; - } - } - for (i = 0; i != bitmapLength; ++i) { - ok = JS_XDRUint32(xdr, &bitmap[i]); - if (!ok) - goto release_mark; - } - for (i = 0; i != n; ++i) { - if (i < nargs && - !(bitmap[i / JS_BITS_PER_UINT32] & - JS_BIT(i & (JS_BITS_PER_UINT32 - 1)))) { - if (xdr->mode == JSXDR_DECODE) { - ok = js_AddLocal(xdr->cx, fun, NULL, JSLOCAL_ARG); - if (!ok) - goto release_mark; - } else { - JS_ASSERT(!names[i]); - } - continue; - } - if (xdr->mode == JSXDR_ENCODE) - name = names[i]; - ok = js_XDRStringAtom(xdr, &name); - if (!ok) - goto release_mark; - if (xdr->mode == JSXDR_DECODE) { - localKind = (i < nargs) - ? JSLOCAL_ARG - : bitmap[i / JS_BITS_PER_UINT32] & - JS_BIT(i & (JS_BITS_PER_UINT32 - 1)) - ? JSLOCAL_CONST - : JSLOCAL_VAR; - ok = js_AddLocal(xdr->cx, fun, name, localKind); - if (!ok) - goto release_mark; - } - } - ok = JS_TRUE; - - release_mark: - JS_ARENA_RELEASE(&xdr->cx->tempPool, mark); - if (!ok) - goto out; - - if (xdr->mode == JSXDR_DECODE) - js_FreezeLocalNames(cx, fun); - } + if (!XDRLocalNames(xdr, fun)) + goto bad; if (!js_XDRScript(xdr, &fun->u.i.script, NULL)) goto bad; @@ -2306,7 +2246,7 @@ js_AddLocal(JSContext *cx, JSFunction *fun, JSAtom *atom, JSLocalKind kind) else JS_ASSERT(kind == JSLOCAL_VAR); } - n = fun->nargs + fun->u.i.nvars; + n = JS_GET_LOCAL_NAME_COUNT(fun); if (n == 0) { JS_ASSERT(fun->u.i.names.taggedAtom == 0); fun->u.i.names.taggedAtom = taggedAtom; @@ -2399,7 +2339,7 @@ js_LookupLocal(JSContext *cx, JSFunction *fun, JSAtom *atom, uintN *indexp) JSLocalNameHashEntry *entry; JS_ASSERT(FUN_INTERPRETED(fun)); - n = fun->nargs + fun->u.i.nvars; + n = JS_GET_LOCAL_NAME_COUNT(fun); if (n == 0) return JSLOCAL_NONE; if (n <= MAX_ARRAY_LOCALS) { @@ -2409,7 +2349,7 @@ js_LookupLocal(JSContext *cx, JSFunction *fun, JSAtom *atom, uintN *indexp) i = n; do { --i; - if (atom == (JSAtom *) (array[i] & ~1)) { + if (atom == JS_LOCAL_NAME_TO_ATOM(array[i])) { if (i < fun->nargs) { if (indexp) *indexp = i; @@ -2417,7 +2357,9 @@ js_LookupLocal(JSContext *cx, JSFunction *fun, JSAtom *atom, uintN *indexp) } if (indexp) *indexp = i - fun->nargs; - return (array[i] & 1) ? JSLOCAL_CONST : JSLOCAL_VAR; + return JS_LOCAL_NAME_IS_CONST(array[i]) + ? JSLOCAL_CONST + : JSLOCAL_VAR; } } while (i != 0); } else { @@ -2434,68 +2376,65 @@ js_LookupLocal(JSContext *cx, JSFunction *fun, JSAtom *atom, uintN *indexp) return JSLOCAL_NONE; } -typedef struct JSGetLocalNamesArgs { +typedef struct JSLocalNameEnumeratorArgs { JSFunction *fun; - JSAtom **names; - uint32 *bitmap; + jsuword *names; #ifdef DEBUG uintN nCopiedArgs; uintN nCopiedVars; #endif -} JSGetLocalNamesArgs; - -#define SET_BIT32(bitmap, bit) \ - ((bitmap)[(bit) >> JS_BITS_PER_UINT32_LOG2] |= \ - JS_BIT((bit) & (JS_BITS_PER_UINT32 - 1))) +} JSLocalNameEnumeratorArgs; JS_STATIC_DLL_CALLBACK(JSDHashOperator) get_local_names_enumerator(JSDHashTable *table, JSDHashEntryHdr *hdr, uint32 number, void *arg) { JSLocalNameHashEntry *entry; - JSGetLocalNamesArgs *args; + JSLocalNameEnumeratorArgs *args; uint i; + jsuword constFlag; entry = (JSLocalNameHashEntry *) hdr; - args = (JSGetLocalNamesArgs *) arg; + args = (JSLocalNameEnumeratorArgs *) arg; JS_ASSERT(entry->name); if (entry->localKind == JSLOCAL_ARG) { JS_ASSERT(entry->index < args->fun->nargs); JS_ASSERT(args->nCopiedArgs++ < args->fun->nargs); i = entry->index; + constFlag = 0; } else { JS_ASSERT(entry->localKind == JSLOCAL_VAR || entry->localKind == JSLOCAL_CONST); JS_ASSERT(entry->index < args->fun->u.i.nvars); JS_ASSERT(args->nCopiedVars++ < args->fun->u.i.nvars); i = args->fun->nargs + entry->index; + constFlag = (entry->localKind == JSLOCAL_CONST) ? 1 : 0; } - args->names[i] = entry->name; - if (args->bitmap && entry->localKind != JSLOCAL_VAR) - SET_BIT32(args->bitmap, i); + args->names[i] = (jsuword) entry->name | constFlag; return JS_DHASH_NEXT; } -JSAtom ** -js_GetLocalNames(JSContext *cx, JSFunction *fun, JSArenaPool *pool, - uint32 **bitmap) +jsuword * +js_GetLocalNameArray(JSContext *cx, JSFunction *fun, JSArenaPool *pool) { - uintN n, i; - size_t allocsize; - JSAtom **names; - jsuword *array; + uintN n; + jsuword *names; JSLocalNameMap *map; - JSGetLocalNamesArgs args; + JSLocalNameEnumeratorArgs args; JSNameIndexPair *dup; JS_ASSERT(FUN_INTERPRETED(fun)); - JS_ASSERT(OBJ_IS_NATIVE(fun->object)); - n = fun->nargs + fun->u.i.nvars; + n = JS_GET_LOCAL_NAME_COUNT(fun); JS_ASSERT(n != 0); - allocsize = n * sizeof *names; - if (bitmap) - allocsize += JS_HOWMANY(n, JS_BITS_PER_UINT32) * sizeof(uint32); - JS_ARENA_ALLOCATE_CAST(names, JSAtom **, pool, allocsize); + + if (n <= MAX_ARRAY_LOCALS) + return (n == 1) ? &fun->u.i.names.taggedAtom : fun->u.i.names.array; + + /* + * No need to check for overflow of the allocation size as we are making a + * copy of already allocated data. As such it must fit size_t. + */ + JS_ARENA_ALLOCATE_CAST(names, jsuword *, pool, (size_t) n * sizeof *names); if (!names) { js_ReportOutOfScriptQuota(cx); return NULL; @@ -2505,45 +2444,23 @@ js_GetLocalNames(JSContext *cx, JSFunction *fun, JSArenaPool *pool, /* Some parameter names can be NULL due to destructuring patterns. */ memset(names, 0, fun->nargs * sizeof *names); #endif - if (bitmap) { - *bitmap = (uint32 *) (names + n); - memset(*bitmap, 0, JS_HOWMANY(n, JS_BITS_PER_UINT32) * sizeof(uint32)); - } - - if (n <= MAX_ARRAY_LOCALS) { - array = (n == 1) ? &fun->u.i.names.taggedAtom : fun->u.i.names.array; - - i = n; - do { - --i; - names[i] = (JSAtom *) (array[i] & ~1); - if (bitmap && - (i < fun->nargs ? array[i] != 0 : array[i] & 1)) { - SET_BIT32(*bitmap, i); - } - } while (i != 0); - } else { - map = fun->u.i.names.map; - args.fun = fun; - args.names = names; - args.bitmap = bitmap ? *bitmap : NULL; + map = fun->u.i.names.map; + args.fun = fun; + args.names = names; #ifdef DEBUG - args.nCopiedArgs = 0; - args.nCopiedVars = 0; + args.nCopiedArgs = 0; + args.nCopiedVars = 0; #endif - JS_DHashTableEnumerate(&map->names, get_local_names_enumerator, &args); - for (dup = map->lastdup; dup; dup = dup->link) { - JS_ASSERT(dup->index < fun->nargs); - JS_ASSERT(args.nCopiedArgs++ < fun->nargs); - names[dup->index] = dup->name; - if (bitmap) - SET_BIT32(*bitmap, dup->index); - } -#if !JS_HAS_DESTRUCTURING - JS_ASSERT(args.nCopiedArgs == fun->nargs); -#endif - JS_ASSERT(args.nCopiedVars == fun->u.i.nvars); + JS_DHashTableEnumerate(&map->names, get_local_names_enumerator, &args); + for (dup = map->lastdup; dup; dup = dup->link) { + JS_ASSERT(dup->index < fun->nargs); + JS_ASSERT(args.nCopiedArgs++ < fun->nargs); + names[dup->index] = (jsuword) dup->name; } +#if !JS_HAS_DESTRUCTURING + JS_ASSERT(args.nCopiedArgs == fun->nargs); +#endif + JS_ASSERT(args.nCopiedVars == fun->u.i.nvars); return names; } @@ -2573,7 +2490,7 @@ TraceLocalNames(JSTracer *trc, JSFunction *fun) jsuword *array; JS_ASSERT(FUN_INTERPRETED(fun)); - n = fun->nargs + fun->u.i.nvars; + n = JS_GET_LOCAL_NAME_COUNT(fun); if (n == 0) return; if (n <= MAX_ARRAY_LOCALS) { @@ -2631,3 +2548,106 @@ js_FreezeLocalNames(JSContext *cx, JSFunction *fun) fun->u.i.names.array = array; } } + +#if JS_HAS_XDR + +static JSBool +XDRLocalNames(JSXDRState *xdr, JSFunction *fun) +{ + uintN n, bitmapLength, i; + void *mark; + uint32 *bitmap; + jsuword *names; + JSAtom *name; + JSBool ok; + JSLocalKind localKind; + + JS_ASSERT(FUN_INTERPRETED(fun)); + n = JS_GET_LOCAL_NAME_COUNT(fun); + if (n == 0) + return JS_TRUE; + + mark = JS_ARENA_MARK(&xdr->cx->tempPool); + + /* + * From this point the control must flow through the label out. + * + * To xdr the names we prefix the names with a bitmap descriptor and then + * xdr the names as strings. For argument names (indexes below fun->nargs) + * the corresponding bit in the bitmap is unset when the name is null. + * Such null names are not encoded or decoded. For variable names (indexes + * starting from fun->nargs) bitmap's bit is set when the name is declared + * as const, not as ordinary var. + */ + bitmapLength = JS_HOWMANY(n, JS_BITS_PER_UINT32); + JS_ARENA_ALLOCATE_CAST(bitmap, uint32 *, &xdr->cx->tempPool, + bitmapLength * sizeof *bitmap); + + if (xdr->mode == JSXDR_ENCODE) { + names = js_GetLocalNameArray(xdr->cx, fun, &xdr->cx->tempPool); + if (!names) + return JS_FALSE; + memset(bitmap, 0, bitmapLength * sizeof *bitmap); + for (i = 0; i != n; ++i) { + if (i < fun->nargs + ? JS_LOCAL_NAME_TO_ATOM(names[i]) != NULL + : JS_LOCAL_NAME_IS_CONST(names[i])) { + bitmap[i >> JS_BITS_PER_UINT32_LOG2] |= + JS_BIT(i & (JS_BITS_PER_UINT32 - 1)); + } + } + } +#ifdef __GNUC__ + else { + names = NULL; /* quell GCC uninitialized warning */ + } +#endif + + for (i = 0; i != bitmapLength; ++i) { + ok = JS_XDRUint32(xdr, &bitmap[i]); + if (!ok) + goto out; + } + + for (i = 0; i != n; ++i) { + if (i < fun->nargs && + !(bitmap[i >> JS_BITS_PER_UINT32_LOG2] & + JS_BIT(i & (JS_BITS_PER_UINT32 - 1)))) { + if (xdr->mode == JSXDR_DECODE) { + ok = js_AddLocal(xdr->cx, fun, NULL, JSLOCAL_ARG); + if (!ok) + goto out; + } else { + JS_ASSERT(JS_LOCAL_NAME_TO_ATOM(names[i])); + } + continue; + } + if (xdr->mode == JSXDR_ENCODE) + name = JS_LOCAL_NAME_TO_ATOM(names[i]); + ok = js_XDRStringAtom(xdr, &name); + if (!ok) + goto out; + if (xdr->mode == JSXDR_DECODE) { + localKind = i < fun->nargs + ? JSLOCAL_ARG + : (bitmap[i >> JS_BITS_PER_UINT32_LOG2] & + JS_BIT(i & (JS_BITS_PER_UINT32 - 1))) + ? JSLOCAL_CONST + : JSLOCAL_VAR; + ok = js_AddLocal(xdr->cx, fun, name, localKind); + if (!ok) + goto out; + } + } + if (xdr->mode == JSXDR_DECODE) + js_FreezeLocalNames(xdr->cx, fun); + ok = JS_TRUE; + + out: + JS_ARENA_RELEASE(&xdr->cx->tempPool, mark); + + return ok; +} + +#endif + diff --git a/js/src/jsfun.h b/js/src/jsfun.h index 3b331874f1b0..1873d79f2a64 100644 --- a/js/src/jsfun.h +++ b/js/src/jsfun.h @@ -206,6 +206,8 @@ typedef enum JSLocalKind { JSLOCAL_CONST } JSLocalKind; +#define JS_GET_LOCAL_NAME_COUNT(fun) ((fun)->nargs + (fun)->u.i.nvars) + extern JSBool js_AddLocal(JSContext *cx, JSFunction *fun, JSAtom *atom, JSLocalKind kind); @@ -219,21 +221,27 @@ extern JSLocalKind js_LookupLocal(JSContext *cx, JSFunction *fun, JSAtom *atom, uintN *indexp); /* - * Get names of arguments and variables for the interpreted function. + * Functions to work with local names as an array of words. * - * The result is an array allocated from the given pool with - * fun->nargs + fun->u.i.nvars - * elements with the names of the arguments coming first. The argument - * name is null when it corresponds to a destructive pattern. + * js_GetLocalNameArray returns the array or null when it cannot be allocated + * The function must be called only when JS_GET_LOCAL_NAME_COUNT(fun) is not + * zero. The function use the supplied pool to allocate the array. * - * When bitmap is not null, on successful return it will contain a bit array - * where for each index below fun->nargs the bit is set when the corresponding - * argument name is not null. For indexes greater or equal fun->nargs the bit - * is set when the corresponding var is really a const. + * The elements of the array with index below fun->nargs correspond to the + * names of function arguments and of function variables otherwise. Use + * JS_LOCAL_NAME_TO_ATOM to convert array's element into an atom. It can be + * null when the element is an argument corresponding to a destructuring + * pattern. For a variable use JS_LOCAL_NAME_IS_CONST to check if it + * corresponds to the const declaration. */ -extern JSAtom ** -js_GetLocalNames(JSContext *cx, JSFunction *fun, JSArenaPool *pool, - uint32 **bitmap); +extern jsuword * +js_GetLocalNameArray(JSContext *cx, JSFunction *fun, JSArenaPool *pool); + +#define JS_LOCAL_NAME_TO_ATOM(nameWord) \ + ((JSAtom *) ((nameWord) & ~(jsuword) 1)) + +#define JS_LOCAL_NAME_IS_CONST(nameWord) \ + ((((nameWord) & (jsuword) 1)) != 0) extern void js_FreezeLocalNames(JSContext *cx, JSFunction *fun); diff --git a/js/src/jsopcode.c b/js/src/jsopcode.c index 3f908583af8a..a5f0b5e8bb90 100644 --- a/js/src/jsopcode.c +++ b/js/src/jsopcode.c @@ -616,7 +616,7 @@ struct JSPrinter { JSScript *script; /* script being printed */ jsbytecode *dvgfence; /* js_DecompileValueGenerator fencepost */ JSFunction *fun; /* interpreted function */ - JSAtom **localNames; /* argument and variable names */ + jsuword *localNames; /* argument and variable names */ }; /* @@ -644,10 +644,9 @@ JS_NEW_PRINTER(JSContext *cx, const char *name, JSFunction *fun, jp->script = NULL; jp->dvgfence = NULL; jp->fun = fun; - if (!fun || !FUN_INTERPRETED(fun) || fun->nargs + fun->u.i.nvars == 0) { - jp->localNames = NULL; - } else { - jp->localNames = js_GetLocalNames(cx, fun, &jp->pool, NULL); + jp->localNames = NULL; + if (fun && FUN_INTERPRETED(fun) && JS_GET_LOCAL_NAME_COUNT(fun)) { + jp->localNames = js_GetLocalNameArray(cx, fun, &jp->pool); if (!jp->localNames) { js_DestroyPrinter(jp); return NULL; @@ -1103,15 +1102,14 @@ GetSlotAtom(JSPrinter *jp, JSBool argument, uintN slot) LOCAL_ASSERT_RV(jp->localNames, NULL); if (argument) { LOCAL_ASSERT_RV(slot < fun->nargs, NULL); - name = jp->localNames[slot]; -#if !JS_HAS_DESTRUCTURING - LOCAL_ASSERT_RV(name, NULL); -#endif } else { LOCAL_ASSERT_RV(slot < fun->u.i.nvars, NULL); - name = jp->localNames[fun->nargs + slot]; - LOCAL_ASSERT_RV(name, NULL); + slot += fun->nargs; } + name = JS_LOCAL_NAME_TO_ATOM(jp->localNames[slot]); +#if !JS_HAS_DESTRUCTURING + LOCAL_ASSERT_RV(name, NULL); +#endif return name; }