diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index 77713cde38d..d1d52c5f12e 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -3200,7 +3200,7 @@ JSFunction::lookupLocal(JSContext *cx, JSAtom *atom, uintN *indexp) { JS_ASSERT(FUN_INTERPRETED(this)); - Shape *shape = SHAPE_FETCH(Shape::search(cx->runtime, &u.i.names, ATOM_TO_JSID(atom))); + Shape *shape = SHAPE_FETCH(Shape::search(&u.i.names, ATOM_TO_JSID(atom))); if (shape) { JSLocalKind localKind; diff --git a/js/src/jsscope.cpp b/js/src/jsscope.cpp index e0147f76224..278498c1833 100644 --- a/js/src/jsscope.cpp +++ b/js/src/jsscope.cpp @@ -139,24 +139,29 @@ JS_FRIEND_DATA(JSScopeStats) js_scope_stats = {0}; #endif bool -PropertyTable::init(JSRuntime *rt, Shape *lastProp) +PropertyTable::init(Shape *lastProp, JSContext *cx) { - /* - * Either we're creating a table for a large scope that was populated - * via property cache hit logic under JSOP_INITPROP, JSOP_SETNAME, or - * JSOP_SETPROP; or else calloc failed at least once already. In any - * event, let's try to grow, overallocating to hold at least twice the - * current population. - */ - uintN sizeLog2 = JS_CeilingLog2(2 * entryCount); - if (sizeLog2 < MIN_SIZE_LOG2) + int sizeLog2; + + if (entryCount > HASH_THRESHOLD) { + /* + * Either we're creating a table for a large scope that was populated + * via property cache hit logic under JSOP_INITPROP, JSOP_SETNAME, or + * JSOP_SETPROP; or else calloc failed at least once already. In any + * event, let's try to grow, overallocating to hold at least twice the + * current population. + */ + sizeLog2 = JS_CeilingLog2(2 * entryCount); + } else { + JS_ASSERT(hashShift == JS_DHASH_BITS - MIN_SIZE_LOG2); sizeLog2 = MIN_SIZE_LOG2; + } /* - * Use rt->calloc for memory accounting and overpressure handling + * Use cx->runtime->calloc for memory accounting and overpressure handling * without OOM reporting. See PropertyTable::change. */ - entries = (Shape **) rt->calloc(JS_BIT(sizeLog2) * sizeof(Shape *)); + entries = (Shape **) cx->runtime->calloc(JS_BIT(sizeLog2) * sizeof(Shape *)); if (!entries) { METER(tableAllocFails); return false; @@ -180,14 +185,15 @@ PropertyTable::init(JSRuntime *rt, Shape *lastProp) } bool -Shape::hashify(JSRuntime *rt) +Shape::maybeHash(JSContext *cx) { JS_ASSERT(!table); - void* mem = rt->malloc(sizeof(PropertyTable)); - if (!mem) - return false; - table = new(mem) PropertyTable(entryCount()); - return table->init(rt, this); + uint32 nentries = entryCount(); + if (nentries >= PropertyTable::HASH_THRESHOLD) { + table = cx->create(nentries); + return table && table->init(this, cx); + } + return true; } #ifdef DEBUG @@ -499,7 +505,7 @@ Shape::getChild(JSContext *cx, const js::Shape &child, Shape **listp) newShape->setTable(table); } else { if (!newShape->table) - newShape->hashify(cx->runtime); + newShape->maybeHash(cx); } return newShape; } @@ -518,6 +524,8 @@ Shape::getChild(JSContext *cx, const js::Shape &child, Shape **listp) if (shape) { JS_ASSERT(shape->parent == this); JS_ASSERT(this == *listp); + if (!shape->table) + shape->maybeHash(cx); *listp = shape; } return shape; @@ -626,7 +634,7 @@ Shape::newDictionaryList(JSContext *cx, Shape **listp) list = *listp; JS_ASSERT(list->inDictionary()); - list->hashify(cx->runtime); + list->maybeHash(cx); return list; } @@ -838,6 +846,18 @@ JSObject::addPropertyInternal(JSContext *cx, jsid id, LIVE_SCOPE_METER(cx, ++cx->runtime->liveObjectProps); JS_RUNTIME_METER(cx->runtime, totalObjectProps); #endif + + /* + * If we reach the hashing threshold, try to allocate lastProp->table. + * If we can't (a rare event, preceded by swapping to death on most + * modern OSes), stick with linear search rather than whining about + * this little set-back. Therefore we must test !lastProp->table and + * entry count >= PropertyTable::HASH_THRESHOLD, not merely whether the + * entry count just reached the threshold. + */ + if (!lastProp->table) + lastProp->maybeHash(cx); + CHECK_SHAPE_CONSISTENCY(this); METER(adds); return shape; @@ -999,6 +1019,11 @@ JSObject::putProperty(JSContext *cx, jsid id, } shape = newShape; + + if (!shape->table) { + /* See JSObject::addPropertyInternal comment about ignoring OOM. */ + shape->maybeHash(cx); + } } /* @@ -1235,7 +1260,7 @@ JSObject::removeProperty(JSContext *cx, jsid id) /* * Non-dictionary-mode property tables are shared immutables, so all we * need do is retract lastProp and we'll either get or else lazily make - * via a later hashify the exact table for the new property lineage. + * via a later maybeHash the exact table for the new property lineage. */ JS_ASSERT(shape == lastProp); removeLastProperty(); diff --git a/js/src/jsscope.h b/js/src/jsscope.h index 1bd9b716983..354cc940b6e 100644 --- a/js/src/jsscope.h +++ b/js/src/jsscope.h @@ -50,7 +50,6 @@ #include "jstypes.h" #include "jscntxt.h" -#include "jscompartment.h" #include "jshashtable.h" #include "jsobj.h" #include "jsprvtd.h" @@ -214,14 +213,15 @@ namespace js { /* * Shapes use multiplicative hashing, _a la_ jsdhash.[ch], but specialized to - * minimize footprint. But if a Shape lineage has been searched fewer than - * HASH_MIN_SEARCHES times, we use linear search and avoid allocating - * scope->table. + * minimize footprint. But if a Shape lineage has fewer than HASH_THRESHOLD + * entries, we use linear search and avoid allocating scope->table. */ struct PropertyTable { - static const uint32 HASH_MIN_SEARCHES = 7; - static const uint32 MIN_SIZE_LOG2 = 4; - static const uint32 MIN_SIZE = JS_BIT(MIN_SIZE_LOG2); + enum { + HASH_THRESHOLD = 6, + MIN_SIZE_LOG2 = 4, + MIN_SIZE = JS_BIT(MIN_SIZE_LOG2) + }; int hashShift; /* multiplicative hash shift */ @@ -266,7 +266,7 @@ struct PropertyTable { * cope or ignore. They do however use JSRuntime's calloc method in order * to update the malloc counter on success. */ - bool init(JSRuntime *rt, js::Shape *lastProp); + bool init(js::Shape *lastProp, JSContext *cx); bool change(int log2Delta, JSContext *cx); js::Shape **search(jsid id, bool adding); }; @@ -293,7 +293,6 @@ struct Shape : public JSObjectMap friend bool HasUnreachableGCThings(TreeFragment *f); protected: - mutable uint32 numSearches; /* Only updated until it reaches HASH_MIN_SEARCHES. */ mutable js::PropertyTable *table; public: @@ -350,8 +349,7 @@ struct Shape : public JSObjectMap else to obj->lastProp */ }; - static inline js::Shape **search(JSRuntime *rt, js::Shape **startp, jsid id, - bool adding = false); + static inline js::Shape **search(js::Shape **startp, jsid id, bool adding = false); static js::Shape *newDictionaryShape(JSContext *cx, const js::Shape &child, js::Shape **listp); static js::Shape *newDictionaryList(JSContext *cx, js::Shape **listp); @@ -360,7 +358,7 @@ struct Shape : public JSObjectMap js::Shape *getChild(JSContext *cx, const js::Shape &child, js::Shape **listp); - bool hashify(JSRuntime *rt); + bool maybeHash(JSContext *cx); void setTable(js::PropertyTable *t) const { JS_ASSERT_IF(t && t->freelist != SHAPE_INVALID_SLOT, t->freelist < slotSpan); @@ -652,7 +650,7 @@ struct EmptyShape : public js::Shape inline js::Shape ** JSObject::nativeSearch(jsid id, bool adding) { - return js::Shape::search(compartment()->rt, &lastProp, id, adding); + return js::Shape::search(&lastProp, id, adding); } inline const js::Shape * @@ -832,36 +830,29 @@ extern JS_FRIEND_DATA(JSScopeStats) js_scope_stats; namespace js { JS_ALWAYS_INLINE js::Shape ** -Shape::search(JSRuntime *rt, js::Shape **startp, jsid id, bool adding) +Shape::search(js::Shape **startp, jsid id, bool adding) { - js::Shape *start = *startp; METER(searches); - if (start->table || - (start->numSearches >= PropertyTable::HASH_MIN_SEARCHES && start->hashify(rt))) - { - return start->table->search(id, adding); - } + if (!(*startp)->table) { + /* + * Not enough properties to justify hashing: search from *startp. + * + * We don't use a Range here, or stop at null parent (the empty shape + * at the end), to avoid an extra load per iteration just to save a + * load and id test at the end (when missing). + */ + js::Shape **spp; - /* - * Not enough searches done so far to justify hashing: search linearly - * from *startp. - * - * We don't use a Range here, or stop at null parent (the empty shape - * at the end), to avoid an extra load per iteration just to save a - * load and id test at the end (when missing). - */ - JS_ASSERT(!start->table); - start->numSearches++; - - js::Shape **spp; - for (spp = startp; js::Shape *shape = *spp; spp = &shape->parent) { - if (shape->id == id) { - METER(hits); - return spp; + for (spp = startp; js::Shape *shape = *spp; spp = &shape->parent) { + if (shape->id == id) { + METER(hits); + return spp; + } } + METER(misses); + return spp; } - METER(misses); - return spp; + return (*startp)->table->search(id, adding); } #undef METER diff --git a/js/src/jsscopeinlines.h b/js/src/jsscopeinlines.h index dd61cbd5a5d..b36ec5ed6d7 100644 --- a/js/src/jsscopeinlines.h +++ b/js/src/jsscopeinlines.h @@ -170,8 +170,8 @@ inline Shape::Shape(jsid id, js::PropertyOp getter, js::PropertyOp setter, uint32 slot, uintN attrs, uintN flags, intN shortid, uint32 shape, uint32 slotSpan) : JSObjectMap(shape, slotSpan), - numSearches(0), table(NULL), id(id), rawGetter(getter), rawSetter(setter), slot(slot), - attrs(uint8(attrs)), flags(uint8(flags)), shortid(int16(shortid)), parent(NULL) + table(NULL), id(id), rawGetter(getter), rawSetter(setter), slot(slot), attrs(uint8(attrs)), + flags(uint8(flags)), shortid(int16(shortid)), parent(NULL) { JS_ASSERT_IF(slotSpan != SHAPE_INVALID_SLOT, slotSpan < JSObject::NSLOTS_LIMIT); JS_ASSERT_IF(getter && (attrs & JSPROP_GETTER), getterObj->isCallable()); @@ -181,7 +181,7 @@ Shape::Shape(jsid id, js::PropertyOp getter, js::PropertyOp setter, uint32 slot, inline Shape::Shape(JSContext *cx, Class *aclasp) - : JSObjectMap(js_GenerateShape(cx, false), JSSLOT_FREE(aclasp)), numSearches(0), table(NULL), + : JSObjectMap(js_GenerateShape(cx, false), JSSLOT_FREE(aclasp)), table(NULL), id(JSID_EMPTY), clasp(aclasp), rawSetter(NULL), slot(SHAPE_INVALID_SLOT), attrs(0), flags(SHARED_EMPTY), shortid(0), parent(NULL) {