Bug 723346 - Make sharpObjectMap a modern HashMap; r=Waldo

This is the last user of the old-style C hashtables in the JS engine.
This commit is contained in:
Terrence Cole
2012-02-15 17:55:25 -08:00
parent e4ac08e990
commit 989ba317e5
5 changed files with 79 additions and 103 deletions

View File

@@ -1434,23 +1434,22 @@ JSObject::makeDenseArraySlow(JSContext *cx)
class ArraySharpDetector class ArraySharpDetector
{ {
JSContext *cx; JSContext *cx;
JSHashEntry *he; bool success;
bool alreadySeen; bool alreadySeen;
bool sharp; bool sharp;
public: public:
ArraySharpDetector(JSContext *cx) ArraySharpDetector(JSContext *cx)
: cx(cx), : cx(cx),
he(NULL), success(false),
alreadySeen(false), alreadySeen(false),
sharp(false) sharp(false)
{} {}
bool init(JSObject *obj) { bool init(JSObject *obj) {
he = js_EnterSharpObject(cx, obj, NULL, &alreadySeen); success = js_EnterSharpObject(cx, obj, NULL, &alreadySeen, &sharp);
if (!he) if (!success)
return false; return false;
sharp = IS_SHARP(he);
return true; return true;
} }
@@ -1460,7 +1459,7 @@ class ArraySharpDetector
} }
~ArraySharpDetector() { ~ArraySharpDetector() {
if (he && !sharp) if (success && !sharp)
js_LeaveSharpObject(cx, NULL); js_LeaveSharpObject(cx, NULL);
} }
}; };

View File

@@ -970,6 +970,7 @@ JSContext::JSContext(JSRuntime *rt)
stack(thisDuringConstruction()), /* depends on cx->thread_ */ stack(thisDuringConstruction()), /* depends on cx->thread_ */
parseMapPool_(NULL), parseMapPool_(NULL),
globalObject(NULL), globalObject(NULL),
sharpObjectMap(this),
argumentFormatMap(NULL), argumentFormatMap(NULL),
lastMessage(NULL), lastMessage(NULL),
errorReporter(NULL), errorReporter(NULL),

View File

@@ -76,12 +76,23 @@ JS_BEGIN_EXTERN_C
struct DtoaState; struct DtoaState;
JS_END_EXTERN_C JS_END_EXTERN_C
struct JSSharpInfo {
bool hasGen;
bool isSharp;
JSSharpInfo() : hasGen(false), isSharp(false) {}
};
typedef js::HashMap<JSObject *, JSSharpInfo> JSSharpTable;
struct JSSharpObjectMap { struct JSSharpObjectMap {
jsrefcount depth; jsrefcount depth;
uint32_t sharpgen; uint32_t sharpgen;
JSHashTable *table; JSSharpTable table;
JSSharpObjectMap() : depth(0), sharpgen(0), table(NULL) {} JSSharpObjectMap(JSContext *cx) : depth(0), sharpgen(0), table(js::TempAllocPolicy(cx)) {
table.init();
}
}; };
namespace js { namespace js {

View File

@@ -203,14 +203,8 @@ obj_setProto(JSContext *cx, JSObject *obj, jsid id, JSBool strict, Value *vp)
#endif /* !JS_HAS_OBJ_PROTO_PROP */ #endif /* !JS_HAS_OBJ_PROTO_PROP */
static JSHashNumber static bool
js_hash_object(const void *key) MarkSharpObjects(JSContext *cx, JSObject *obj, JSIdArray **idap, JSSharpInfo *value)
{
return JSHashNumber(uintptr_t(key) >> JS_GCTHING_ALIGN);
}
static JSHashEntry *
MarkSharpObjects(JSContext *cx, JSObject *obj, JSIdArray **idap)
{ {
JS_CHECK_RECURSION(cx, return NULL); JS_CHECK_RECURSION(cx, return NULL);
@@ -218,21 +212,15 @@ MarkSharpObjects(JSContext *cx, JSObject *obj, JSIdArray **idap)
JSSharpObjectMap *map = &cx->sharpObjectMap; JSSharpObjectMap *map = &cx->sharpObjectMap;
JS_ASSERT(map->depth >= 1); JS_ASSERT(map->depth >= 1);
JSHashTable *table = map->table; JSSharpInfo sharpid;
JSHashNumber hash = js_hash_object(obj); JSSharpTable::Ptr p = map->table.lookup(obj);
JSHashEntry **hep = JS_HashTableRawLookup(table, hash, obj); if (!p) {
JSHashEntry *he = *hep; if (!map->table.put(obj, sharpid))
if (!he) { return false;
jsatomid sharpid = 0;
he = JS_HashTableRawAdd(table, hep, hash, obj, (void *) sharpid);
if (!he) {
JS_ReportOutOfMemory(cx);
return NULL;
}
ida = JS_Enumerate(cx, obj); ida = JS_Enumerate(cx, obj);
if (!ida) if (!ida)
return NULL; return false;
bool ok = true; bool ok = true;
for (jsint i = 0, length = ida->length; i < length; i++) { for (jsint i = 0, length = ida->length; i < length; i++) {
@@ -261,7 +249,7 @@ MarkSharpObjects(JSContext *cx, JSObject *obj, JSIdArray **idap)
if (hasSetter) { if (hasSetter) {
/* Mark the getter, then set val to setter. */ /* Mark the getter, then set val to setter. */
if (hasGetter && v.value().isObject()) { if (hasGetter && v.value().isObject()) {
ok = !!MarkSharpObjects(cx, &v.value().toObject(), NULL); ok = MarkSharpObjects(cx, &v.value().toObject(), NULL, NULL);
if (!ok) if (!ok)
break; break;
} }
@@ -271,7 +259,7 @@ MarkSharpObjects(JSContext *cx, JSObject *obj, JSIdArray **idap)
if (!ok) if (!ok)
break; break;
} }
if (v.value().isObject() && !MarkSharpObjects(cx, &v.value().toObject(), NULL)) { if (v.value().isObject() && !MarkSharpObjects(cx, &v.value().toObject(), NULL, NULL)) {
ok = false; ok = false;
break; break;
} }
@@ -279,47 +267,42 @@ MarkSharpObjects(JSContext *cx, JSObject *obj, JSIdArray **idap)
if (!ok || !idap) if (!ok || !idap)
JS_DestroyIdArray(cx, ida); JS_DestroyIdArray(cx, ida);
if (!ok) if (!ok)
return NULL; return false;
} else { } else {
jsatomid sharpid = uintptr_t(he->value); if (!p->value.hasGen && !p->value.isSharp) {
if (sharpid == 0) { p->value.hasGen = true;
sharpid = ++map->sharpgen << SHARP_ID_SHIFT;
he->value = (void *) sharpid;
} }
sharpid = p->value;
ida = NULL; ida = NULL;
} }
if (idap) if (idap)
*idap = ida; *idap = ida;
return he; if (value)
*value = sharpid;
return true;
} }
JSHashEntry * bool
js_EnterSharpObject(JSContext *cx, JSObject *obj, JSIdArray **idap, bool *alreadySeen) js_EnterSharpObject(JSContext *cx, JSObject *obj, JSIdArray **idap, bool *alreadySeen, bool *isSharp)
{ {
if (!JS_CHECK_OPERATION_LIMIT(cx)) if (!JS_CHECK_OPERATION_LIMIT(cx))
return NULL; return false;
*alreadySeen = false; *alreadySeen = false;
JSSharpObjectMap *map = &cx->sharpObjectMap; 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; JS_ASSERT_IF(map->depth == 0, map->table.count() == 0);
jsatomid sharpid; JS_ASSERT_IF(map->table.count() == 0, map->depth == 0);
JSSharpTable::Ptr p;
JSSharpInfo sharpid;
JSIdArray *ida = NULL; JSIdArray *ida = NULL;
/* From this point the control must flow either through out: or bad:. */ /* From this point the control must flow either through out: or bad:. */
if (map->depth == 0) { if (map->depth == 0) {
JS_KEEP_ATOMS(cx->runtime);
/* /*
* Although MarkSharpObjects tries to avoid invoking getters, * Although MarkSharpObjects tries to avoid invoking getters,
* it ends up doing so anyway under some circumstances; for * 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 * ensure that such a call doesn't free the hash table we're
* still using. * still using.
*/ */
++map->depth; map->depth = 1;
he = MarkSharpObjects(cx, obj, &ida); bool success = MarkSharpObjects(cx, obj, &ida, &sharpid);
--map->depth; JS_ASSERT(map->depth == 1);
if (!he) map->depth = 0;
if (!success)
goto bad; goto bad;
JS_ASSERT((uintptr_t(he->value) & SHARP_BIT) == 0); JS_ASSERT(!sharpid.isSharp);
if (!idap) { if (!idap) {
JS_DestroyIdArray(cx, ida); JS_DestroyIdArray(cx, ida);
ida = NULL; ida = NULL;
} }
} else { } 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 * It's possible that the value of a property has changed from the
* first time the object's properties are traversed (when the property * 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 * converted to strings), i.e., the JSObject::getProperty() call is not
* idempotent. * idempotent.
*/ */
if (!he) { p = map->table.lookup(obj);
he = JS_HashTableRawAdd(table, hep, hash, obj, NULL); if (!p) {
if (!he) { if (!map->table.put(obj, sharpid))
JS_ReportOutOfMemory(cx);
goto bad; goto bad;
}
sharpid = 0;
goto out; goto out;
} }
sharpid = p->value;
} }
sharpid = uintptr_t(he->value); if (sharpid.isSharp || sharpid.hasGen)
if (sharpid != 0)
*alreadySeen = true; *alreadySeen = true;
out: out:
JS_ASSERT(he); if (!sharpid.isSharp) {
if ((sharpid & SHARP_BIT) == 0) {
if (idap && !ida) { if (idap && !ida) {
ida = JS_Enumerate(cx, obj); ida = JS_Enumerate(cx, obj);
if (!ida) if (!ida)
@@ -382,17 +358,17 @@ out:
if (idap) if (idap)
*idap = ida; *idap = ida;
return he; *isSharp = sharpid.isSharp;
return true;
bad: bad:
/* Clean up the sharpObjectMap table on outermost error. */ /* Clean up the sharpObjectMap table on outermost error. */
if (map->depth == 0) { if (map->depth == 0) {
JS_UNKEEP_ATOMS(cx->runtime); JS_UNKEEP_ATOMS(cx->runtime);
map->sharpgen = 0; map->sharpgen = 0;
JS_HashTableDestroy(map->table); map->table.clear();
map->table = NULL;
} }
return NULL; return false;
} }
void void
@@ -403,8 +379,7 @@ js_LeaveSharpObject(JSContext *cx, JSIdArray **idap)
if (--map->depth == 0) { if (--map->depth == 0) {
JS_UNKEEP_ATOMS(cx->runtime); JS_UNKEEP_ATOMS(cx->runtime);
map->sharpgen = 0; map->sharpgen = 0;
JS_HashTableDestroy(map->table); map->table.clear();
map->table = NULL;
} }
if (idap) { if (idap) {
if (JSIdArray *ida = *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 void
js_TraceSharpMap(JSTracer *trc, JSSharpObjectMap *map) js_TraceSharpMap(JSTracer *trc, JSSharpObjectMap *map)
{ {
JS_ASSERT(map->depth > 0); JS_ASSERT(map->depth > 0);
JS_ASSERT(map->table);
/* /*
* During recursive calls to MarkSharpObjects a non-native object or * 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 * with otherwise unreachable objects. But this is way too complex
* to justify spending efforts. * 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 #if JS_HAS_TOSOURCE
@@ -475,8 +443,8 @@ obj_toSource(JSContext *cx, uintN argc, Value *vp)
JSIdArray *ida; JSIdArray *ida;
bool alreadySeen = false; bool alreadySeen = false;
JSHashEntry *he = js_EnterSharpObject(cx, obj, &ida, &alreadySeen); bool isSharp = false;
if (!he) if (!js_EnterSharpObject(cx, obj, &ida, &alreadySeen, &isSharp))
return false; return false;
if (!ida) { if (!ida) {
@@ -491,10 +459,14 @@ obj_toSource(JSContext *cx, uintN argc, Value *vp)
vp->setString(str); vp->setString(str);
return true; return true;
} }
JS_ASSERT(!IS_SHARP(he));
if (alreadySeen) JS_ASSERT(!isSharp);
MAKE_SHARP(he); 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. */ /* Automatically call js_LeaveSharpObject when we leave this frame. */
class AutoLeaveSharpObject { class AutoLeaveSharpObject {
@@ -537,7 +509,6 @@ obj_toSource(JSContext *cx, uintN argc, Value *vp)
JSString *s = ToString(cx, IdToValue(id)); JSString *s = ToString(cx, IdToValue(id));
if (!s || !(idstr = s->ensureLinear(cx))) if (!s || !(idstr = s->ensureLinear(cx)))
return false; return false;
vp->setString(idstr); /* local root */
int valcnt = 0; int valcnt = 0;
if (prop) { if (prop) {
@@ -576,7 +547,6 @@ obj_toSource(JSContext *cx, uintN argc, Value *vp)
s = js_QuoteString(cx, idstr, jschar('\'')); s = js_QuoteString(cx, idstr, jschar('\''));
if (!s || !(idstr = s->ensureLinear(cx))) if (!s || !(idstr = s->ensureLinear(cx)))
return false; return false;
vp->setString(idstr); /* local root */
} }
for (int j = 0; j < valcnt; j++) { for (int j = 0; j < valcnt; j++) {

View File

@@ -1554,13 +1554,8 @@ class ValueArray {
}; };
/* For manipulating JSContext::sharpObjectMap. */ /* For manipulating JSContext::sharpObjectMap. */
#define SHARP_BIT ((jsatomid) 1) extern bool
#define SHARP_ID_SHIFT 2 js_EnterSharpObject(JSContext *cx, JSObject *obj, JSIdArray **idap, bool *alreadySeen, bool *isSharp);
#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 void extern void
js_LeaveSharpObject(JSContext *cx, JSIdArray **idap); js_LeaveSharpObject(JSContext *cx, JSIdArray **idap);