Bug 524743 - Shape regeneration still does not touch most empty scopes. r=brendan.
authorJason Orendorff <jorendorff@mozilla.com>
Tue, 27 Oct 2009 16:00:26 -0500
changeset 34360 d54c92b7f76e0462231b846144c1f88f3f3650a6
parent 34359 bb0a5cb4bbc3b93eac9aa3bb1aa0df396a44c9dd
child 34361 5c4b01fe33caca1b8ff47e73597186b7a2cef6c5
push idunknown
push userunknown
push dateunknown
reviewersbrendan
bugs524743
milestone1.9.3a1pre
Bug 524743 - Shape regeneration still does not touch most empty scopes. r=brendan.
js/src/jscntxt.h
js/src/jsgc.cpp
js/src/jsscope.cpp
js/src/jsscopeinlines.h
js/src/tests/js1_8_1/regress/jstests.list
js/src/tests/js1_8_1/regress/regress-524743.js
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -482,17 +482,17 @@ struct JSRuntime {
      * During gc, if rt->gcRegenShapes &&
      *   (scope->flags & JSScope::SHAPE_REGEN) == rt->gcRegenShapesScopeFlag,
      * then the scope's shape has already been regenerated during this GC.
      * To avoid having to sweep JSScopes, the bit's meaning toggles with each
      * shape-regenerating GC.
      *
      * FIXME Once scopes are GC'd (bug 505004), this will be obsolete.
      */
-    uint8              gcRegenShapesScopeFlag;
+    uint8               gcRegenShapesScopeFlag;
 
 #ifdef JS_GC_ZEAL
     jsrefcount          gcZeal;
 #endif
 
     JSGCCallback        gcCallback;
 
     /*
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -3119,17 +3119,21 @@ js_GC(JSContext *cx, JSGCInvocationKind 
     JS_ASSERT(!rt->gcUntracedArenaStackTop);
     JS_ASSERT(rt->gcTraceLaterCount == 0);
 
     /*
      * Reset the property cache's type id generator so we can compress ids.
      * Same for the protoHazardShape proxy-shape standing in for all object
      * prototypes having readonly or setter properties.
      */
-    if (rt->shapeGen & SHAPE_OVERFLOW_BIT) {
+    if (rt->shapeGen & SHAPE_OVERFLOW_BIT
+#ifdef JS_GC_ZEAL
+        || rt->gcZeal >= 1
+#endif
+        ) {
         rt->gcRegenShapes = true;
         rt->gcRegenShapesScopeFlag ^= JSScope::SHAPE_REGEN;
         rt->shapeGen = 0;
         rt->protoHazardShape = 0;
     }
 
     js_PurgeThreads(cx);
 #ifdef JS_TRACER
--- a/js/src/jsscope.cpp
+++ b/js/src/jsscope.cpp
@@ -1059,20 +1059,21 @@ JSScope::add(JSContext *cx, jsid id,
     JSScopeProperty **spp, *sprop, *overwriting, **spvec, **spp2, child;
     uint32 size, splen, i;
     int change;
     JSTempValueRooter tvr;
 
     JS_ASSERT(JS_IS_SCOPE_LOCKED(cx, this));
     CHECK_ANCESTOR_LINE(this, true);
 
+    JS_ASSERT(!JSVAL_IS_NULL(id));
     JS_ASSERT_IF(attrs & JSPROP_GETTER, getter);
     JS_ASSERT_IF(attrs & JSPROP_SETTER, setter);
-
-    JS_ASSERT(!JSVAL_IS_NULL(id));
+    JS_ASSERT_IF(!cx->runtime->gcRegenShapes,
+                 hasRegenFlag(cx->runtime->gcRegenShapesScopeFlag));
 
     /*
      * You can't add properties to a sealed scope.  But note well that you can
      * change property attributes in a sealed scope, even though that replaces
      * a JSScopeProperty * in the scope's hash table -- but no id is added, so
      * the scope remains sealed.
      */
     if (sealed()) {
--- a/js/src/jsscopeinlines.h
+++ b/js/src/jsscopeinlines.h
@@ -112,17 +112,17 @@ JSScope::methodWriteBarrier(JSContext *c
 }
 
 inline void
 JSScope::trace(JSTracer *trc)
 {
     JSContext *cx = trc->context;
     JSScopeProperty *sprop = lastProp;
     uint8 regenFlag = cx->runtime->gcRegenShapesScopeFlag;
-    if (IS_GC_MARKING_TRACER(trc) && cx->runtime->gcRegenShapes && hasRegenFlag(regenFlag)) {
+    if (IS_GC_MARKING_TRACER(trc) && cx->runtime->gcRegenShapes && !hasRegenFlag(regenFlag)) {
         /*
          * Either this scope has its own shape, which must be regenerated, or
          * it must have the same shape as lastProp.
          */
         uint32 newShape;
 
         if (sprop) {
             if (!(sprop->flags & SPROP_FLAG_SHAPE_REGEN)) {
@@ -135,17 +135,17 @@ JSScope::trace(JSTracer *trc)
             newShape = js_RegenerateShapeForGC(cx);
             JS_ASSERT_IF(sprop, newShape != sprop->shape);
         }
         shape = newShape;
         flags ^= JSScope::SHAPE_REGEN;
 
         /* Also regenerate the shapes of empty scopes, in case they are not shared. */
         for (JSScope *empty = emptyScope;
-             empty && empty->hasRegenFlag(regenFlag);
+             empty && !empty->hasRegenFlag(regenFlag);
              empty = empty->emptyScope) {
             empty->shape = js_RegenerateShapeForGC(cx);
             empty->flags ^= JSScope::SHAPE_REGEN;
         }
     }
     if (sprop) {
         JS_ASSERT(has(sprop));
 
--- a/js/src/tests/js1_8_1/regress/jstests.list
+++ b/js/src/tests/js1_8_1/regress/jstests.list
@@ -75,8 +75,9 @@ script regress-479430-04.js
 script regress-479430-05.js
 script regress-495773.js
 script regress-495907.js
 script regress-496922.js
 script regress-507053.js
 script regress-507295.js
 script regress-507424.js
 script regress-515885.js
+script regress-524743.js
new file mode 100644
--- /dev/null
+++ b/js/src/tests/js1_8_1/regress/regress-524743.js
@@ -0,0 +1,14 @@
+// Any copyright is dedicated to the Public Domain.
+// http://creativecommons.org/licenses/publicdomain/
+if (typeof gczeal != 'undefined')
+    gczeal(2);
+if (typeof gc != 'undefined') {
+    for (var i = 0; i < 10; i++) {
+        var obj = {};
+        for (var j = 0; j < 5; j++) {
+            obj[Math.floor(Math.random() * 5)] = 0;
+            gc();
+        }
+    }
+}
+reportCompare("no assertion failure", "no assertion failure", "bug 524743");