From 936b0b0fcdf8758248fcf12d3280f1ed5524cf10 Mon Sep 17 00:00:00 2001 From: Brendan Eich Date: Mon, 30 Aug 2010 14:54:08 -0700 Subject: [PATCH] Bug 592001 - Fix v8-regexp regression in wake of patch for bug 558451 (r=gal; CLOSED TREE). --- js/src/jspropertytree.cpp | 2 +- js/src/jsscope.cpp | 58 +++++++++++++++++---------------------- js/src/jsscope.h | 34 ++++++++++++++++++----- 3 files changed, 53 insertions(+), 41 deletions(-) diff --git a/js/src/jspropertytree.cpp b/js/src/jspropertytree.cpp index 16476da9aa9..5d1e74619fd 100644 --- a/js/src/jspropertytree.cpp +++ b/js/src/jspropertytree.cpp @@ -118,7 +118,7 @@ KidsChunk::create(JSContext *cx) { KidsChunk *chunk; - chunk = (KidsChunk *) js_calloc(sizeof *chunk); + chunk = (KidsChunk *) js_calloc(sizeof(KidsChunk)); if (!chunk) { JS_UNLOCK_GC(cx->runtime); JS_ReportOutOfMemory(cx); diff --git a/js/src/jsscope.cpp b/js/src/jsscope.cpp index 6c45a9e4509..80d725431e1 100644 --- a/js/src/jsscope.cpp +++ b/js/src/jsscope.cpp @@ -135,7 +135,7 @@ JS_FRIEND_DATA(JSScopeStats) js_scope_stats = {0}; #endif bool -PropertyTable::init(JSContext *cx, Shape *lastProp) +PropertyTable::init(Shape *lastProp) { int sizeLog2; @@ -158,7 +158,6 @@ PropertyTable::init(JSContext *cx, Shape *lastProp) METER(tableAllocFails); return false; } - cx->updateMallocCounter(JS_BIT(sizeLog2) * sizeof(Shape *)); hashShift = JS_DHASH_BITS - sizeLog2; for (Shape::Range r = lastProp->all(); !r.empty(); r.popFront()) { @@ -172,15 +171,19 @@ PropertyTable::init(JSContext *cx, Shape *lastProp) } bool -Shape::maybeHash(JSContext *cx) +Shape::reallyHash(int count) { JS_ASSERT(!table); - uint32 nentries = entryCount(); - if (nentries >= PropertyTable::HASH_THRESHOLD) { - table = cx->create(nentries); - return table && table->init(cx, this); + JS_ASSERT(count >= PropertyTable::HASH_THRESHOLD); + + /* Inline version of JSContext::create to avoid cx dependency. */ + void *mem = js_malloc(sizeof(PropertyTable)); + if (mem) { + table = new (mem) PropertyTable(uint32(count)); + if (table) + return table->init(this); } - return true; + return false; } #ifdef DEBUG @@ -375,7 +378,7 @@ PropertyTable::search(jsid id, bool adding) } bool -PropertyTable::change(JSContext *cx, int change) +PropertyTable::change(int change) { int oldlog2, newlog2; uint32 oldsize, newsize, nbytes; @@ -389,7 +392,7 @@ PropertyTable::change(JSContext *cx, int change) oldsize = JS_BIT(oldlog2); newsize = JS_BIT(newlog2); nbytes = PROPERTY_TABLE_NBYTES(newsize); - newTable = (Shape **) cx->calloc(nbytes); + newTable = (Shape **) js_calloc(nbytes); if (!newTable) { METER(tableAllocFails); return false; @@ -401,9 +404,6 @@ PropertyTable::change(JSContext *cx, int change) oldTable = entries; entries = newTable; - /* Treat the above calloc as a JS_malloc, to match CreateScopeTable. */ - cx->updateMallocCounter(nbytes); - /* Copy only live entries, leaving removed and free ones behind. */ for (oldspp = oldTable; oldsize != 0; oldspp++) { shape = SHAPE_FETCH(oldspp); @@ -418,7 +418,7 @@ PropertyTable::change(JSContext *cx, int change) } /* Finally, free the old entries storage. */ - cx->free(oldTable); + js_free(oldTable); return true; } @@ -545,6 +545,7 @@ Shape::newDictionaryList(JSContext *cx, Shape **listp) Shape **childp = listp; *childp = NULL; + int count = -1; while (shape) { JS_ASSERT(!shape->inDictionary()); @@ -558,11 +559,15 @@ Shape::newDictionaryList(JSContext *cx, Shape **listp) JS_ASSERT(!dprop->table); childp = &dprop->parent; shape = shape->parent; + ++count; } list = *listp; JS_ASSERT(list->inDictionary()); - list->maybeHash(cx); + + /* Update memory pressure, since this table is not a shared resource. */ + if (list->maybeHash(count)) + list->table->updateMemoryPressure(cx); return list; } @@ -672,8 +677,9 @@ JSObject::addPropertyCommon(JSContext *cx, jsid id, METER(compresses); else METER(grows); - if (!table->change(cx, change) && table->entryCount + table->removedCount == size - 1) + if (!table->change(change) && table->entryCount + table->removedCount == size - 1) return NULL; + table->updateMemoryPressure(cx); METER(searches); METER(changeSearches); spp = table->search(id, true); @@ -706,17 +712,6 @@ JSObject::addPropertyCommon(JSContext *cx, jsid id, 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); - METER(adds); return shape; } @@ -818,11 +813,6 @@ JSObject::putProperty(JSContext *cx, jsid id, shape->setTable(table); } - if (!lastProp->table) { - /* See comment in JSObject::addPropertyCommon about ignoring OOM here. */ - lastProp->maybeHash(cx); - } - METER(puts); return shape; } @@ -998,7 +988,9 @@ JSObject::removeProperty(JSContext *cx, jsid id) uint32 size = table->capacity(); if (size > PropertyTable::MIN_SIZE && table->entryCount <= size >> 2) { METER(shrinks); - (void) table->change(cx, -1); + (void) table->change(-1); + if (inDictionaryMode()) + table->updateMemoryPressure(cx); } } diff --git a/js/src/jsscope.h b/js/src/jsscope.h index cf296b261aa..fd5c136c786 100644 --- a/js/src/jsscope.h +++ b/js/src/jsscope.h @@ -226,9 +226,9 @@ struct PropertyTable { uint32 removedCount; /* removed entry sentinels in table */ js::Shape **entries; /* table of ptrs to shared tree nodes */ - PropertyTable(uint32 nentries) + PropertyTable(uint32 count) : hashShift(JS_DHASH_BITS - MIN_SIZE_LOG2), - entryCount(nentries), + entryCount(count), removedCount(0) { /* NB: entries is set by init, which must be called. */ @@ -241,12 +241,16 @@ struct PropertyTable { /* By definition, hashShift = JS_DHASH_BITS - log2(capacity). */ uint32 capacity() const { return JS_BIT(JS_DHASH_BITS - hashShift); } + void updateMemoryPressure(JSContext *cx) const { + cx->updateMallocCounter(capacity() * sizeof(Shape *)); + } + /* * NB: init and change are fallible but do not report OOM, so callers can * cope or ignore. They do update the malloc counter on success. */ - bool init(JSContext *cx, js::Shape *lastProp); - bool change(JSContext *cx, int change); + bool init(js::Shape *lastProp); + bool change(int change); js::Shape **search(jsid id, bool adding); }; @@ -352,7 +356,18 @@ struct Shape : public JSObjectMap js::Shape *getChild(JSContext *cx, const js::Shape &child, js::Shape **listp); - bool maybeHash(JSContext *cx); + /* + * NB: return true only if we need a PropertyTable and we successfully + * allocated one, so callers who care can update memory pressure. + */ + bool maybeHash(int count) { + JS_ASSERT(!table); + if (count >= PropertyTable::HASH_THRESHOLD) + return reallyHash(count); + return false; + } + + bool reallyHash(int count); void setTable(js::PropertyTable *t) const { table = t; } @@ -778,15 +793,20 @@ Shape::search(js::Shape **startp, jsid id, bool adding) * 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; + js::Shape **spp = startp; + int count = -1; - for (spp = startp; js::Shape *shape = *spp; spp = &shape->parent) { + while (js::Shape *shape = *spp) { if (shape->id == id) { METER(hits); return spp; } + spp = &shape->parent; + ++count; } + METER(misses); + (*startp)->maybeHash(count); return spp; } return (*startp)->table->search(id, adding);