From be78d4f6516e0688a7203d22cc04cc9928b1bad9 Mon Sep 17 00:00:00 2001 From: Igor Bukanov Date: Wed, 29 Apr 2009 07:07:21 -0700 Subject: [PATCH] bug 487846 - optimizing shape prediction for set opcodes. r=brendan --- js/src/jsinterp.cpp | 84 ++++++++++++++++++++++++++++++++------------- js/src/jsinterp.h | 9 +++-- js/src/jsobj.cpp | 29 +++++++--------- js/src/jstracer.cpp | 5 ++- 4 files changed, 81 insertions(+), 46 deletions(-) diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index 3c76ce6aa51f..50bfc53968d4 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -108,18 +108,18 @@ js_GenerateShape(JSContext *cx, JSBool gcLocked) } JS_REQUIRES_STACK JSPropCacheEntry * -js_FillPropertyCache(JSContext *cx, JSObject *obj, jsuword kshape, - uintN scopeIndex, uintN protoIndex, - JSObject *pobj, JSScopeProperty *sprop) +js_FillPropertyCache(JSContext *cx, JSObject *obj, + uintN scopeIndex, uintN protoIndex, JSObject *pobj, + JSScopeProperty *sprop, JSBool addedSprop) { JSPropertyCache *cache; jsbytecode *pc; JSScope *scope; + jsuword kshape, khash; JSOp op; const JSCodeSpec *cs; jsuword vword; ptrdiff_t pcoff; - jsuword khash; JSAtom *atom; JSPropCacheEntry *entry; @@ -192,6 +192,7 @@ js_FillPropertyCache(JSContext *cx, JSObject *obj, jsuword kshape, pc = cx->fp->regs->pc; op = js_GetOpcode(cx, cx->fp->script, pc); cs = &js_CodeSpec[op]; + kshape = 0; do { /* @@ -223,12 +224,12 @@ js_FillPropertyCache(JSContext *cx, JSObject *obj, jsuword kshape, PCMETER(cache->brandfills++); #ifdef DEBUG_notme fprintf(stderr, - "branding %p (%s) for funobj %p (%s), kshape %lu\n", + "branding %p (%s) for funobj %p (%s), shape %lu\n", pobj, LOCKED_OBJ_GET_CLASS(pobj)->name, JSVAL_TO_OBJECT(v), JS_GetFunctionName(GET_FUNCTION_PRIVATE(cx, JSVAL_TO_OBJECT(v))), - kshape); + OBJ_SHAPE(obj)); #endif js_MakeScopeShapeUnique(cx, scope); if (js_IsPropertyCacheDisabled(cx)) { @@ -239,8 +240,6 @@ js_FillPropertyCache(JSContext *cx, JSObject *obj, jsuword kshape, return JS_NO_PROP_CACHE_FILL; } SCOPE_SET_BRANDED(scope); - if (OBJ_SCOPE(obj) == scope) - kshape = scope->shape; } vword = JSVAL_OBJECT_TO_PCVAL(v); break; @@ -257,28 +256,55 @@ js_FillPropertyCache(JSContext *cx, JSObject *obj, jsuword kshape, } else { /* Best we can do is to cache sprop (still a nice speedup). */ vword = SPROP_TO_PCVAL(sprop); + if (addedSprop && + sprop == scope->lastProp && + scope->shape == sprop->shape) { + /* + * Our caller added a new property. We also know that a setter + * that js_NativeSet could have run has not mutated the scope + * so the added property is still the last one added and the + * scope is not branded. + * + * We want to cache under scope's shape before the property + * addition to bias for the case when the mutator opcode + * always adds the same property. It allows to optimize + * periodic execution of object initializers or explicit + * initialization sequences like + * + * obj = {}; obj.x = 1; obj.y = 2; + * + * We assume that on average the win from this optimization is + * bigger that the cost of an extra mismatch per loop due to + * the bias for the following case: + * + * obj = {}; ... for (...) { ... obj.x = ... } + * + * On the first iteration JSOP_SETPROP fills the cache with + * the shape of newly created object, not the shape after + * obj.x is assigned. That mismatches obj's shape on the + * second iteration. Note that on third and the following + * iterations the cache will be hit since the shape no longer + * mutates. + */ + JS_ASSERT(scope->object == obj); + if (sprop->parent) { + kshape = sprop->parent->shape; + } else { + JSObject *proto = STOBJ_GET_PROTO(obj); + if (proto && OBJ_IS_NATIVE(proto)) + kshape = OBJ_SHAPE(proto); + } + } } } while (0); - /* - * Our caller preserved the scope shape prior to the js_GetPropertyHelper - * or similar call out of the interpreter. We want to cache under that - * shape if op is overtly mutating, to bias for the case where the mutator - * udpates shape predictably. - * - * Note that an apparently non-mutating op such as JSOP_NAME may still - * mutate the base object via, e.g., lazy standard class initialization, - * but that is a one-time event and we'll have to miss the old shape and - * re-fill under the new one. - */ - if (!(cs->format & (JOF_SET | JOF_INCDEC)) && obj == pobj) - kshape = scope->shape; - + if (kshape == 0) + kshape = OBJ_SHAPE(obj); khash = PROPERTY_CACHE_HASH_PC(pc, kshape); if (obj == pobj) { - JS_ASSERT(kshape != 0 || scope->shape != 0); JS_ASSERT(scopeIndex == 0 && protoIndex == 0); JS_ASSERT(OBJ_SCOPE(obj)->object == obj); + JS_ASSERT(kshape != 0); } else { if (op == JSOP_LENGTH) { atom = cx->runtime->atomState.lengthAtom; @@ -310,7 +336,7 @@ js_FillPropertyCache(JSContext *cx, JSObject *obj, jsuword kshape, } entry = &cache->table[khash]; - PCMETER(if (!PCVAL_IS_NULL(entry->vword)) cache->recycles++); + PCMETER(PCVAL_IS_NULL(entry->vword) || cache->recycles++); entry->kpc = pc; entry->kshape = kshape; entry->vcap = PCVCAP_MAKE(scope->shape, scopeIndex, protoIndex); @@ -318,6 +344,13 @@ js_FillPropertyCache(JSContext *cx, JSObject *obj, jsuword kshape, cache->empty = JS_FALSE; PCMETER(cache->fills++); + + /* + * The modfills counter is not exact. It increases if a getter or setter + * recurse into the interpreter. + */ + PCMETER(entry == cache->pctestentry || cache->modfills++); + PCMETER(cache->pctestentry = NULL); return entry; } @@ -471,6 +504,7 @@ js_PurgePropertyCache(JSContext *cx, JSPropertyCache *cache) P(rofills); P(disfills); P(oddfills); + P(modfills); P(brandfills); P(noprotos); P(longchains); @@ -4574,6 +4608,7 @@ js_Interpret(JSContext *cx) * in native object o. */ entry = &cache->table[PROPERTY_CACHE_HASH_PC(regs.pc, kshape)]; + PCMETER(cache->pctestentry = entry); PCMETER(cache->tests++); PCMETER(cache->settests++); if (entry->kpc == regs.pc && entry->kshape == kshape) { @@ -6280,6 +6315,7 @@ js_Interpret(JSContext *cx) kshape = scope->shape; cache = &JS_PROPERTY_CACHE(cx); entry = &cache->table[PROPERTY_CACHE_HASH_PC(regs.pc, kshape)]; + PCMETER(cache->pctestentry = entry); PCMETER(cache->tests++); PCMETER(cache->initests++); diff --git a/js/src/jsinterp.h b/js/src/jsinterp.h index e1497d714f85..60b64267166c 100644 --- a/js/src/jsinterp.h +++ b/js/src/jsinterp.h @@ -271,11 +271,13 @@ typedef struct JSPropertyCache { JSPropCacheEntry table[PROPERTY_CACHE_SIZE]; JSBool empty; #ifdef JS_PROPERTY_CACHE_METERING + JSPropCacheEntry *pctestentry; /* entry of the last PC-based test */ uint32 fills; /* number of cache entry fills */ uint32 nofills; /* couldn't fill (e.g. default get) */ uint32 rofills; /* set on read-only prop can't fill */ uint32 disfills; /* fill attempts on disabled cache */ uint32 oddfills; /* fill attempt after setter deleted */ + uint32 modfills; /* fill that rehashed to a new entry */ uint32 brandfills; /* scope brandings to type structural method fills */ uint32 noprotos; /* resolve-returned non-proto pobj */ @@ -348,9 +350,9 @@ typedef struct JSPropertyCache { * possible. */ extern JS_REQUIRES_STACK JSPropCacheEntry * -js_FillPropertyCache(JSContext *cx, JSObject *obj, jsuword kshape, - uintN scopeIndex, uintN protoIndex, - JSObject *pobj, JSScopeProperty *sprop); +js_FillPropertyCache(JSContext *cx, JSObject *obj, + uintN scopeIndex, uintN protoIndex, JSObject *pobj, + JSScopeProperty *sprop, JSBool addedSprop); /* * Property cache lookup macros. PROPERTY_CACHE_TEST is designed to inline the @@ -373,6 +375,7 @@ js_FillPropertyCache(JSContext *cx, JSObject *obj, jsuword kshape, JSPropertyCache *cache_ = &JS_PROPERTY_CACHE(cx); \ uint32 kshape_ = (JS_ASSERT(OBJ_IS_NATIVE(obj)), OBJ_SHAPE(obj)); \ entry = &cache_->table[PROPERTY_CACHE_HASH_PC(pc, kshape_)]; \ + PCMETER(cache_->pctestentry = entry); \ PCMETER(cache_->tests++); \ JS_ASSERT(&obj != &pobj); \ if (entry->kpc == pc && entry->kshape == kshape_) { \ diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index 1871ecd2edd1..cb7459a9920d 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -3714,6 +3714,7 @@ js_DefineNativeProperty(JSContext *cx, JSObject *obj, jsid id, jsval value, JSClass *clasp; JSScope *scope; JSScopeProperty *sprop; + bool added; JSPropCacheEntry *entry; js_LeaveTraceIfGlobalObject(cx, obj); @@ -3721,8 +3722,6 @@ js_DefineNativeProperty(JSContext *cx, JSObject *obj, jsid id, jsval value, /* Convert string indices to integers if appropriate. */ CHECK_FOR_STRING_INDEX(id); - uint32 shape = OBJ_SHAPE(obj); - #if JS_HAS_GETTER_SETTER /* * If defining a getter or setter, we must check for its counterpart and @@ -3790,6 +3789,7 @@ js_DefineNativeProperty(JSContext *cx, JSObject *obj, jsid id, jsval value, if (!scope) goto bad; + added = false; if (!sprop) { /* Add a new property, or replace an existing one of the same id. */ if (clasp->flags & JSCLASS_SHARE_ALL_PROPERTIES) @@ -3799,6 +3799,7 @@ js_DefineNativeProperty(JSContext *cx, JSObject *obj, jsid id, jsval value, shortid); if (!sprop) goto bad; + added = true; } /* Store value before calling addProperty, in case the latter GC's. */ @@ -3814,7 +3815,7 @@ js_DefineNativeProperty(JSContext *cx, JSObject *obj, jsid id, jsval value, if (cacheResult) { JS_ASSERT_NOT_ON_TRACE(cx); if (!(attrs & JSPROP_SHARED)) - entry = js_FillPropertyCache(cx, obj, shape, 0, 0, obj, sprop); + entry = js_FillPropertyCache(cx, obj, 0, 0, obj, sprop, added); else PCMETER(JS_PROPERTY_CACHE(cx).nofills++); } @@ -4034,14 +4035,12 @@ js_FindPropertyHelper(JSContext *cx, jsid id, JSBool cacheResult, JSObject **objp, JSObject **pobjp, JSProperty **propp) { JSObject *scopeChain, *obj, *parent, *pobj; - uint32 shape; JSPropCacheEntry *entry; int scopeIndex, protoIndex; JSProperty *prop; JS_ASSERT_IF(cacheResult, !JS_ON_TRACE(cx)); scopeChain = js_GetTopStackFrame(cx)->scopeChain; - shape = OBJ_SHAPE(scopeChain); /* Scan entries on the scope chain that we can cache across. */ entry = JS_NO_PROP_CACHE_FILL; @@ -4080,9 +4079,9 @@ js_FindPropertyHelper(JSContext *cx, jsid id, JSBool cacheResult, } #endif if (cacheResult) { - entry = js_FillPropertyCache(cx, scopeChain, shape, + entry = js_FillPropertyCache(cx, scopeChain, scopeIndex, protoIndex, pobj, - (JSScopeProperty *) prop); + (JSScopeProperty *) prop, false); } SCOPE_DEPTH_ACCUM(&rt->scopeSearchDepthStats, scopeIndex); goto out; @@ -4163,9 +4162,9 @@ js_FindIdentifierBase(JSContext *cx, JSObject *scopeChain, jsid id) #ifdef DEBUG JSPropCacheEntry *entry = #endif - js_FillPropertyCache(cx, scopeChain, OBJ_SHAPE(scopeChain), + js_FillPropertyCache(cx, scopeChain, scopeIndex, protoIndex, pobj, - (JSScopeProperty *) prop); + (JSScopeProperty *) prop, false); JS_ASSERT(entry); JS_UNLOCK_OBJ(cx, pobj); return obj; @@ -4310,7 +4309,6 @@ js_GetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, JSBool cacheResult, jsval *vp) { JSObject *aobj, *obj2; - uint32 shape; int protoIndex; JSProperty *prop; JSScopeProperty *sprop; @@ -4320,7 +4318,6 @@ js_GetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, JSBool cacheResult, CHECK_FOR_STRING_INDEX(id); aobj = js_GetProtoIfDenseArray(cx, obj); - shape = OBJ_SHAPE(aobj); protoIndex = js_LookupPropertyWithFlags(cx, aobj, id, cx->resolveFlags, &obj2, &prop); if (protoIndex < 0) @@ -4396,7 +4393,7 @@ js_GetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, JSBool cacheResult, if (cacheResult) { JS_ASSERT_NOT_ON_TRACE(cx); - js_FillPropertyCache(cx, aobj, shape, 0, protoIndex, obj2, sprop); + js_FillPropertyCache(cx, aobj, 0, protoIndex, obj2, sprop, false); } JS_UNLOCK_OBJ(cx, obj2); return JS_TRUE; @@ -4449,7 +4446,6 @@ JSPropCacheEntry * js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, JSBool cacheResult, jsval *vp) { - uint32 shape; int protoIndex; JSObject *pobj; JSProperty *prop; @@ -4459,6 +4455,7 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, JSBool cacheResult, intN shortid; JSClass *clasp; JSPropertyOp getter, setter; + bool added; JSPropCacheEntry *entry; /* Convert string indices to integers if appropriate. */ @@ -4473,7 +4470,6 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, JSBool cacheResult, goto read_only_error; } - shape = OBJ_SHAPE(obj); protoIndex = js_LookupPropertyWithFlags(cx, obj, id, cx->resolveFlags, &pobj, &prop); if (protoIndex < 0) @@ -4601,6 +4597,7 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, JSBool cacheResult, #endif } + added = false; if (!sprop) { /* * Purge the property cache of now-shadowed id in obj's scope chain. @@ -4637,6 +4634,7 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, JSBool cacheResult, js_RemoveScopeProperty(cx, scope, id); JS_UNLOCK_SCOPE(cx, scope); return NULL); + added = true; } if (!js_NativeSet(cx, obj, sprop, vp)) @@ -4646,10 +4644,9 @@ js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, JSBool cacheResult, if (cacheResult) { JS_ASSERT_NOT_ON_TRACE(cx); if (!(attrs & JSPROP_SHARED)) - entry = js_FillPropertyCache(cx, obj, shape, 0, 0, obj, sprop); + entry = js_FillPropertyCache(cx, obj, 0, 0, obj, sprop, added); else PCMETER(JS_PROPERTY_CACHE(cx).nofills++); - } JS_UNLOCK_SCOPE(cx, scope); return entry; diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index 85e5002ee7f4..9f98166e734d 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -6084,9 +6084,8 @@ TraceRecorder::test_property_cache(JSObject* obj, LIns* obj_ins, JSObject*& obj2 OBJ_DROP_PROPERTY(cx, obj2, prop); ABORT_TRACE("property found on non-native object"); } - entry = js_FillPropertyCache(cx, aobj, OBJ_SHAPE(aobj), 0, - protoIndex, obj2, - (JSScopeProperty*) prop); + entry = js_FillPropertyCache(cx, aobj, 0, protoIndex, obj2, + (JSScopeProperty*) prop, false); JS_ASSERT(entry); if (entry == JS_NO_PROP_CACHE_FILL) entry = NULL;