From 989ba317e567ca7d8ef8fece12dcadf03cb15409 Mon Sep 17 00:00:00 2001 From: Terrence Cole Date: Wed, 15 Feb 2012 17:55:25 -0800 Subject: [PATCH] Bug 723346 - Make sharpObjectMap a modern HashMap; r=Waldo This is the last user of the old-style C hashtables in the JS engine. --- js/src/jsarray.cpp | 11 ++-- js/src/jscntxt.cpp | 1 + js/src/jscntxt.h | 21 +++++-- js/src/jsobj.cpp | 140 ++++++++++++++++++--------------------------- js/src/jsobj.h | 9 +-- 5 files changed, 79 insertions(+), 103 deletions(-) diff --git a/js/src/jsarray.cpp b/js/src/jsarray.cpp index dc1e0bbbc033..80be1d59e576 100644 --- a/js/src/jsarray.cpp +++ b/js/src/jsarray.cpp @@ -1434,23 +1434,22 @@ JSObject::makeDenseArraySlow(JSContext *cx) class ArraySharpDetector { JSContext *cx; - JSHashEntry *he; + bool success; bool alreadySeen; bool sharp; public: ArraySharpDetector(JSContext *cx) : cx(cx), - he(NULL), + success(false), alreadySeen(false), sharp(false) {} bool init(JSObject *obj) { - he = js_EnterSharpObject(cx, obj, NULL, &alreadySeen); - if (!he) + success = js_EnterSharpObject(cx, obj, NULL, &alreadySeen, &sharp); + if (!success) return false; - sharp = IS_SHARP(he); return true; } @@ -1460,7 +1459,7 @@ class ArraySharpDetector } ~ArraySharpDetector() { - if (he && !sharp) + if (success && !sharp) js_LeaveSharpObject(cx, NULL); } }; diff --git a/js/src/jscntxt.cpp b/js/src/jscntxt.cpp index 663b604a8d00..66ffc9db423b 100644 --- a/js/src/jscntxt.cpp +++ b/js/src/jscntxt.cpp @@ -970,6 +970,7 @@ JSContext::JSContext(JSRuntime *rt) stack(thisDuringConstruction()), /* depends on cx->thread_ */ parseMapPool_(NULL), globalObject(NULL), + sharpObjectMap(this), argumentFormatMap(NULL), lastMessage(NULL), errorReporter(NULL), diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index c6f54d95b2d3..c17fd19d8b55 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -76,12 +76,23 @@ JS_BEGIN_EXTERN_C struct DtoaState; JS_END_EXTERN_C -struct JSSharpObjectMap { - jsrefcount depth; - uint32_t sharpgen; - JSHashTable *table; +struct JSSharpInfo { + bool hasGen; + bool isSharp; - JSSharpObjectMap() : depth(0), sharpgen(0), table(NULL) {} + JSSharpInfo() : hasGen(false), isSharp(false) {} +}; + +typedef js::HashMap JSSharpTable; + +struct JSSharpObjectMap { + jsrefcount depth; + uint32_t sharpgen; + JSSharpTable table; + + JSSharpObjectMap(JSContext *cx) : depth(0), sharpgen(0), table(js::TempAllocPolicy(cx)) { + table.init(); + } }; namespace js { diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index 8f921b8f9ad0..b29d88059232 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -203,14 +203,8 @@ obj_setProto(JSContext *cx, JSObject *obj, jsid id, JSBool strict, Value *vp) #endif /* !JS_HAS_OBJ_PROTO_PROP */ -static JSHashNumber -js_hash_object(const void *key) -{ - return JSHashNumber(uintptr_t(key) >> JS_GCTHING_ALIGN); -} - -static JSHashEntry * -MarkSharpObjects(JSContext *cx, JSObject *obj, JSIdArray **idap) +static bool +MarkSharpObjects(JSContext *cx, JSObject *obj, JSIdArray **idap, JSSharpInfo *value) { JS_CHECK_RECURSION(cx, return NULL); @@ -218,21 +212,15 @@ MarkSharpObjects(JSContext *cx, JSObject *obj, JSIdArray **idap) JSSharpObjectMap *map = &cx->sharpObjectMap; JS_ASSERT(map->depth >= 1); - JSHashTable *table = map->table; - JSHashNumber hash = js_hash_object(obj); - JSHashEntry **hep = JS_HashTableRawLookup(table, hash, obj); - JSHashEntry *he = *hep; - if (!he) { - jsatomid sharpid = 0; - he = JS_HashTableRawAdd(table, hep, hash, obj, (void *) sharpid); - if (!he) { - JS_ReportOutOfMemory(cx); - return NULL; - } + JSSharpInfo sharpid; + JSSharpTable::Ptr p = map->table.lookup(obj); + if (!p) { + if (!map->table.put(obj, sharpid)) + return false; ida = JS_Enumerate(cx, obj); if (!ida) - return NULL; + return false; bool ok = true; for (jsint i = 0, length = ida->length; i < length; i++) { @@ -261,7 +249,7 @@ MarkSharpObjects(JSContext *cx, JSObject *obj, JSIdArray **idap) if (hasSetter) { /* Mark the getter, then set val to setter. */ if (hasGetter && v.value().isObject()) { - ok = !!MarkSharpObjects(cx, &v.value().toObject(), NULL); + ok = MarkSharpObjects(cx, &v.value().toObject(), NULL, NULL); if (!ok) break; } @@ -271,7 +259,7 @@ MarkSharpObjects(JSContext *cx, JSObject *obj, JSIdArray **idap) if (!ok) break; } - if (v.value().isObject() && !MarkSharpObjects(cx, &v.value().toObject(), NULL)) { + if (v.value().isObject() && !MarkSharpObjects(cx, &v.value().toObject(), NULL, NULL)) { ok = false; break; } @@ -279,47 +267,42 @@ MarkSharpObjects(JSContext *cx, JSObject *obj, JSIdArray **idap) if (!ok || !idap) JS_DestroyIdArray(cx, ida); if (!ok) - return NULL; + return false; } else { - jsatomid sharpid = uintptr_t(he->value); - if (sharpid == 0) { - sharpid = ++map->sharpgen << SHARP_ID_SHIFT; - he->value = (void *) sharpid; + if (!p->value.hasGen && !p->value.isSharp) { + p->value.hasGen = true; } + sharpid = p->value; ida = NULL; } if (idap) *idap = ida; - return he; + if (value) + *value = sharpid; + return true; } -JSHashEntry * -js_EnterSharpObject(JSContext *cx, JSObject *obj, JSIdArray **idap, bool *alreadySeen) +bool +js_EnterSharpObject(JSContext *cx, JSObject *obj, JSIdArray **idap, bool *alreadySeen, bool *isSharp) { if (!JS_CHECK_OPERATION_LIMIT(cx)) - return NULL; + return false; *alreadySeen = false; JSSharpObjectMap *map = &cx->sharpObjectMap; - JSHashTable *table = map->table; - if (!table) { - table = JS_NewHashTable(8, js_hash_object, JS_CompareValues, - JS_CompareValues, NULL, NULL); - if (!table) { - JS_ReportOutOfMemory(cx); - return NULL; - } - map->table = table; - JS_KEEP_ATOMS(cx->runtime); - } - JSHashEntry *he; - jsatomid sharpid; + JS_ASSERT_IF(map->depth == 0, map->table.count() == 0); + JS_ASSERT_IF(map->table.count() == 0, map->depth == 0); + + JSSharpTable::Ptr p; + JSSharpInfo sharpid; JSIdArray *ida = NULL; /* From this point the control must flow either through out: or bad:. */ if (map->depth == 0) { + JS_KEEP_ATOMS(cx->runtime); + /* * Although MarkSharpObjects tries to avoid invoking getters, * it ends up doing so anyway under some circumstances; for @@ -332,21 +315,18 @@ js_EnterSharpObject(JSContext *cx, JSObject *obj, JSIdArray **idap, bool *alread * ensure that such a call doesn't free the hash table we're * still using. */ - ++map->depth; - he = MarkSharpObjects(cx, obj, &ida); - --map->depth; - if (!he) + map->depth = 1; + bool success = MarkSharpObjects(cx, obj, &ida, &sharpid); + JS_ASSERT(map->depth == 1); + map->depth = 0; + if (!success) goto bad; - JS_ASSERT((uintptr_t(he->value) & SHARP_BIT) == 0); + JS_ASSERT(!sharpid.isSharp); if (!idap) { JS_DestroyIdArray(cx, ida); ida = NULL; } } else { - JSHashNumber hash = js_hash_object(obj); - JSHashEntry **hep = JS_HashTableRawLookup(table, hash, obj); - he = *hep; - /* * It's possible that the value of a property has changed from the * first time the object's properties are traversed (when the property @@ -354,24 +334,20 @@ js_EnterSharpObject(JSContext *cx, JSObject *obj, JSIdArray **idap, bool *alread * converted to strings), i.e., the JSObject::getProperty() call is not * idempotent. */ - if (!he) { - he = JS_HashTableRawAdd(table, hep, hash, obj, NULL); - if (!he) { - JS_ReportOutOfMemory(cx); + p = map->table.lookup(obj); + if (!p) { + if (!map->table.put(obj, sharpid)) goto bad; - } - sharpid = 0; goto out; } + sharpid = p->value; } - sharpid = uintptr_t(he->value); - if (sharpid != 0) + if (sharpid.isSharp || sharpid.hasGen) *alreadySeen = true; out: - JS_ASSERT(he); - if ((sharpid & SHARP_BIT) == 0) { + if (!sharpid.isSharp) { if (idap && !ida) { ida = JS_Enumerate(cx, obj); if (!ida) @@ -382,17 +358,17 @@ out: if (idap) *idap = ida; - return he; + *isSharp = sharpid.isSharp; + return true; bad: /* Clean up the sharpObjectMap table on outermost error. */ if (map->depth == 0) { JS_UNKEEP_ATOMS(cx->runtime); map->sharpgen = 0; - JS_HashTableDestroy(map->table); - map->table = NULL; + map->table.clear(); } - return NULL; + return false; } void @@ -403,8 +379,7 @@ js_LeaveSharpObject(JSContext *cx, JSIdArray **idap) if (--map->depth == 0) { JS_UNKEEP_ATOMS(cx->runtime); map->sharpgen = 0; - JS_HashTableDestroy(map->table); - map->table = NULL; + map->table.clear(); } if (idap) { if (JSIdArray *ida = *idap) { @@ -414,18 +389,10 @@ js_LeaveSharpObject(JSContext *cx, JSIdArray **idap) } } -static intN -gc_sharp_table_entry_marker(JSHashEntry *he, intN i, void *arg) -{ - MarkObjectRoot((JSTracer *)arg, (JSObject *)he->key, "sharp table entry"); - return JS_DHASH_NEXT; -} - void js_TraceSharpMap(JSTracer *trc, JSSharpObjectMap *map) { JS_ASSERT(map->depth > 0); - JS_ASSERT(map->table); /* * During recursive calls to MarkSharpObjects a non-native object or @@ -447,7 +414,8 @@ js_TraceSharpMap(JSTracer *trc, JSSharpObjectMap *map) * with otherwise unreachable objects. But this is way too complex * to justify spending efforts. */ - JS_HashTableEnumerateEntries(map->table, gc_sharp_table_entry_marker, trc); + for (JSSharpTable::Range r = map->table.all(); !r.empty(); r.popFront()) + MarkObjectRoot(trc, r.front().key, "sharp table entry"); } #if JS_HAS_TOSOURCE @@ -475,8 +443,8 @@ obj_toSource(JSContext *cx, uintN argc, Value *vp) JSIdArray *ida; bool alreadySeen = false; - JSHashEntry *he = js_EnterSharpObject(cx, obj, &ida, &alreadySeen); - if (!he) + bool isSharp = false; + if (!js_EnterSharpObject(cx, obj, &ida, &alreadySeen, &isSharp)) return false; if (!ida) { @@ -491,10 +459,14 @@ obj_toSource(JSContext *cx, uintN argc, Value *vp) vp->setString(str); return true; } - JS_ASSERT(!IS_SHARP(he)); - if (alreadySeen) - MAKE_SHARP(he); + JS_ASSERT(!isSharp); + if (alreadySeen) { + JSSharpTable::Ptr p = cx->sharpObjectMap.table.lookup(obj); + JS_ASSERT(p); + JS_ASSERT(!p->value.isSharp); + p->value.isSharp = true; + } /* Automatically call js_LeaveSharpObject when we leave this frame. */ class AutoLeaveSharpObject { @@ -537,7 +509,6 @@ obj_toSource(JSContext *cx, uintN argc, Value *vp) JSString *s = ToString(cx, IdToValue(id)); if (!s || !(idstr = s->ensureLinear(cx))) return false; - vp->setString(idstr); /* local root */ int valcnt = 0; if (prop) { @@ -576,7 +547,6 @@ obj_toSource(JSContext *cx, uintN argc, Value *vp) s = js_QuoteString(cx, idstr, jschar('\'')); if (!s || !(idstr = s->ensureLinear(cx))) return false; - vp->setString(idstr); /* local root */ } for (int j = 0; j < valcnt; j++) { diff --git a/js/src/jsobj.h b/js/src/jsobj.h index 3f9a322e5812..fe0bb874e493 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -1554,13 +1554,8 @@ class ValueArray { }; /* For manipulating JSContext::sharpObjectMap. */ -#define SHARP_BIT ((jsatomid) 1) -#define SHARP_ID_SHIFT 2 -#define IS_SHARP(he) (uintptr_t((he)->value) & SHARP_BIT) -#define MAKE_SHARP(he) ((he)->value = (void *) (uintptr_t((he)->value)|SHARP_BIT)) - -extern JSHashEntry * -js_EnterSharpObject(JSContext *cx, JSObject *obj, JSIdArray **idap, bool *alreadySeen); +extern bool +js_EnterSharpObject(JSContext *cx, JSObject *obj, JSIdArray **idap, bool *alreadySeen, bool *isSharp); extern void js_LeaveSharpObject(JSContext *cx, JSIdArray **idap);