Bug 841617 - replace ObjectElements::convertDoubleElements with a 'flags' field (r=bhackett)
authorLuke Wagner <luke@mozilla.com>
Wed, 13 Feb 2013 18:11:52 -0800
changeset 123837 39abe4f679552ba98389700b2eff9773280424df
parent 123836 c95439870e05b30e6d5ccc0ce72b424aa816f157
child 123838 ec8fb77b41af19effc00f1a16777621aad73a781
push id1401
push userpastithas@mozilla.com
push dateThu, 07 Mar 2013 07:26:45 +0000
treeherderfx-team@ee4879719f78 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs841617
milestone22.0a1
Bug 841617 - replace ObjectElements::convertDoubleElements with a 'flags' field (r=bhackett)
js/src/ion/CodeGenerator.cpp
js/src/ion/IonMacroAssembler.cpp
js/src/jsobjinlines.h
js/src/jstypedarrayinlines.h
js/src/methodjit/BaseAssembler.h
js/src/vm/ObjectImpl.cpp
js/src/vm/ObjectImpl.h
--- a/js/src/ion/CodeGenerator.cpp
+++ b/js/src/ion/CodeGenerator.cpp
@@ -861,18 +861,19 @@ CodeGenerator::visitConvertElementsToDou
 {
     Register elements = ToRegister(lir->elements());
 
     OutOfLineCode *ool = oolCallVM(ConvertElementsToDoublesInfo, lir,
                                    (ArgList(), elements), StoreNothing());
     if (!ool)
         return false;
 
-    Address convertedAddress(elements, ObjectElements::offsetOfConvertDoubleElements());
-    masm.branch32(Assembler::Equal, convertedAddress, Imm32(0), ool->entry());
+    Address convertedAddress(elements, ObjectElements::offsetOfFlags());
+    Imm32 bit(ObjectElements::CONVERT_DOUBLE_ELEMENTS);
+    masm.branchTest32(Assembler::Zero, convertedAddress, bit, ool->entry());
     masm.bind(ool->rejoin());
     return true;
 }
 
 bool
 CodeGenerator::visitFunctionEnvironment(LFunctionEnvironment *lir)
 {
     Address environment(ToRegister(lir->function()), JSFunction::offsetOfEnvironment());
--- a/js/src/ion/IonMacroAssembler.cpp
+++ b/js/src/ion/IonMacroAssembler.cpp
@@ -502,18 +502,20 @@ MacroAssembler::initGCThing(const Regist
 
         // Fill in the elements header.
         store32(Imm32(templateObject->getDenseCapacity()),
                 Address(obj, elementsOffset + ObjectElements::offsetOfCapacity()));
         store32(Imm32(templateObject->getDenseInitializedLength()),
                 Address(obj, elementsOffset + ObjectElements::offsetOfInitializedLength()));
         store32(Imm32(templateObject->getArrayLength()),
                 Address(obj, elementsOffset + ObjectElements::offsetOfLength()));
-        store32(Imm32(templateObject->shouldConvertDoubleElements() ? 1 : 0),
-                Address(obj, elementsOffset + ObjectElements::offsetOfConvertDoubleElements()));
+        store32(Imm32(templateObject->shouldConvertDoubleElements()
+                      ? ObjectElements::CONVERT_DOUBLE_ELEMENTS
+                      : 0),
+                Address(obj, elementsOffset + ObjectElements::offsetOfFlags()));
     } else {
         storePtr(ImmWord(emptyObjectElements), Address(obj, JSObject::offsetOfElements()));
 
         // Fixed slots of non-array objects are required to be initialized.
         // Use the values currently in the template object.
         size_t nslots = Min(templateObject->numFixedSlots(), templateObject->slotSpan());
         for (unsigned i = 0; i < nslots; i++) {
             storeValue(templateObject->getFixedSlot(i),
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -417,24 +417,24 @@ JSObject::getDenseCapacity()
     JS_ASSERT(isNative());
     return getElementsHeader()->capacity;
 }
 
 inline bool
 JSObject::shouldConvertDoubleElements()
 {
     JS_ASSERT(isNative());
-    return getElementsHeader()->convertDoubleElements;
+    return getElementsHeader()->shouldConvertDoubleElements();
 }
 
 inline void
 JSObject::setShouldConvertDoubleElements()
 {
     JS_ASSERT(isArray() && !hasEmptyElements());
-    getElementsHeader()->convertDoubleElements = 1;
+    getElementsHeader()->setShouldConvertDoubleElements();
 }
 
 inline bool
 JSObject::ensureElements(JSContext *cx, uint32_t capacity)
 {
     if (capacity > getDenseCapacity())
         return growElements(cx, capacity);
     return true;
@@ -1071,19 +1071,24 @@ JSObject::hasShapeTable() const
 inline size_t
 JSObject::computedSizeOfThisSlotsElements() const
 {
     size_t n = sizeOfThis();
 
     if (hasDynamicSlots())
         n += numDynamicSlots() * sizeof(js::Value);
 
-    if (hasDynamicElements())
-        n += (js::ObjectElements::VALUES_PER_HEADER + getElementsHeader()->capacity) *
-             sizeof(js::Value);
+    if (hasDynamicElements()) {
+        if (isArrayBuffer()) {
+            n += getElementsHeader()->initializedLength;
+        } else {
+            n += (js::ObjectElements::VALUES_PER_HEADER + getElementsHeader()->capacity) *
+                 sizeof(js::Value);
+        }
+    }
 
     return n;
 }
 
 inline void
 JSObject::sizeOfExcludingThis(JSMallocSizeOfFun mallocSizeOf, JS::ObjectsExtraSizes *sizes)
 {
     if (hasDynamicSlots())
--- a/js/src/jstypedarrayinlines.h
+++ b/js/src/jstypedarrayinlines.h
@@ -15,25 +15,23 @@
 
 // Sentinel value used to initialize ArrayBufferViews' NEXT_BUFFER_SLOTs to
 // show that they have not yet been added to any ArrayBuffer list
 JSObject * const UNSET_BUFFER_LINK = (JSObject*)0x2;
 
 inline void
 js::ArrayBufferObject::setElementsHeader(js::ObjectElements *header, uint32_t bytes)
 {
-    /*
-     * Note that |bytes| may not be a multiple of |sizeof(Value)|, so
-     * |capacity * sizeof(Value)| may underestimate the size by up to
-     * |sizeof(Value) - 1| bytes.
-     */
-    header->capacity = bytes / sizeof(js::Value);
+    header->flags = 0;
     header->initializedLength = bytes;
+
+    // NB: one or both of these fields is clobbered by GetViewList to store the
+    // 'views' link. Set them to 0 to effectively initialize 'views' to NULL.
     header->length = 0;
-    header->convertDoubleElements = 0;
+    header->capacity = 0;
 }
 
 inline uint32_t
 js::ArrayBufferObject::byteLength() const
 {
     JS_ASSERT(isArrayBuffer());
     return getElementsHeader()->initializedLength;
 }
--- a/js/src/methodjit/BaseAssembler.h
+++ b/js/src/methodjit/BaseAssembler.h
@@ -1407,18 +1407,20 @@ static const JSC::MacroAssembler::Regist
         if (templateObject->isArray()) {
             /* Fill in the elements header. */
             store32(Imm32(templateObject->getDenseCapacity()),
                     Address(result, elementsOffset + ObjectElements::offsetOfCapacity()));
             store32(Imm32(templateObject->getDenseInitializedLength()),
                     Address(result, elementsOffset + ObjectElements::offsetOfInitializedLength()));
             store32(Imm32(templateObject->getArrayLength()),
                     Address(result, elementsOffset + ObjectElements::offsetOfLength()));
-            store32(Imm32(templateObject->shouldConvertDoubleElements() ? 1 : 0),
-                    Address(result, elementsOffset + ObjectElements::offsetOfConvertDoubleElements()));
+            store32(Imm32(templateObject->shouldConvertDoubleElements()
+                          ? ObjectElements::CONVERT_DOUBLE_ELEMENTS
+                          : 0),
+                    Address(result, elementsOffset + ObjectElements::offsetOfFlags()));
         } else {
             /*
              * Fixed slots of non-array objects are required to be initialized;
              * Use the values currently in the template object.
              */
             for (unsigned i = 0; i < templateObject->slotSpan(); i++) {
                 storeValue(templateObject->getFixedSlot(i),
                            Address(result, JSObject::getFixedSlotOffset(i)));
--- a/js/src/vm/ObjectImpl.cpp
+++ b/js/src/vm/ObjectImpl.cpp
@@ -164,25 +164,25 @@ ObjectElements::ConvertElementsToDoubles
      * This function is infallible, but has a fallible interface so that it can
      * be called directly from Ion code. Only arrays can have their dense
      * elements converted to doubles, and arrays never have empty elements.
      */
     HeapSlot *elementsHeapPtr = (HeapSlot *) elementsPtr;
     JS_ASSERT(elementsHeapPtr != emptyObjectElements);
 
     ObjectElements *header = ObjectElements::fromElements(elementsHeapPtr);
-    JS_ASSERT(!header->convertDoubleElements);
+    JS_ASSERT(!header->shouldConvertDoubleElements());
 
     Value *vp = (Value *) elementsPtr;
     for (size_t i = 0; i < header->initializedLength; i++) {
         if (vp[i].isInt32())
             vp[i].setDouble(vp[i].toInt32());
     }
 
-    header->convertDoubleElements = 1;
+    header->setShouldConvertDoubleElements();
     return true;
 }
 
 #ifdef DEBUG
 void
 js::ObjectImpl::checkShapeConsistency()
 {
     static int throttle = -1;
--- a/js/src/vm/ObjectImpl.h
+++ b/js/src/vm/ObjectImpl.h
@@ -911,65 +911,84 @@ class ArrayBufferObject;
  * "packed" and can be accessed much faster by JIT code.
  *
  * Elements do not track property creation order, so enumerating the elements
  * of an object does not necessarily visit indexes in the order they were
  * created.
  */
 class ObjectElements
 {
+  public:
+    enum Flags {
+        CONVERT_DOUBLE_ELEMENTS = 0x1
+    };
+
+  private:
     friend class ::JSObject;
     friend class ObjectImpl;
     friend class ArrayBufferObject;
 
-    /* Number of allocated slots. */
-    uint32_t capacity;
+    /* See Flags enum above. */
+    uint32_t flags;
 
     /*
      * Number of initialized elements. This is <= the capacity, and for arrays
      * is <= the length. Memory for elements above the initialized length is
      * uninitialized, but values between the initialized length and the proper
      * length are conceptually holes.
+     *
+     * ArrayBufferObject uses this field to store byteLength.
      */
     uint32_t initializedLength;
 
+    /*
+     * Beware, one or both of the following fields is clobbered by
+     * ArrayBufferObject. See GetViewList.
+     */
+
+    /* Number of allocated slots. */
+    uint32_t capacity;
+
     /* 'length' property of array objects, unused for other objects. */
     uint32_t length;
 
-    /* If non-zero, integer elements should be converted to doubles. */
-    uint32_t convertDoubleElements;
-
     void staticAsserts() {
         MOZ_STATIC_ASSERT(sizeof(ObjectElements) == VALUES_PER_HEADER * sizeof(Value),
                           "Elements size and values-per-Elements mismatch");
     }
 
-  public:
+    bool shouldConvertDoubleElements() const {
+        return flags & CONVERT_DOUBLE_ELEMENTS;
+    }
+    void setShouldConvertDoubleElements() {
+        flags |= CONVERT_DOUBLE_ELEMENTS;
+    }
 
+  public:
     ObjectElements(uint32_t capacity, uint32_t length)
-      : capacity(capacity), initializedLength(0), length(length), convertDoubleElements(0)
+      : flags(0), initializedLength(0), capacity(capacity), length(length)
     {}
 
     HeapSlot *elements() { return (HeapSlot *)(uintptr_t(this) + sizeof(ObjectElements)); }
     static ObjectElements * fromElements(HeapSlot *elems) {
         return (ObjectElements *)(uintptr_t(elems) - sizeof(ObjectElements));
     }
 
-    static int offsetOfCapacity() {
-        return (int)offsetof(ObjectElements, capacity) - (int)sizeof(ObjectElements);
+    static int offsetOfFlags() {
+        return (int)offsetof(ObjectElements, flags) - (int)sizeof(ObjectElements);
     }
     static int offsetOfInitializedLength() {
         return (int)offsetof(ObjectElements, initializedLength) - (int)sizeof(ObjectElements);
     }
+    static int offsetOfCapacity() {
+        return (int)offsetof(ObjectElements, capacity) - (int)sizeof(ObjectElements);
+    }
     static int offsetOfLength() {
         return (int)offsetof(ObjectElements, length) - (int)sizeof(ObjectElements);
     }
-    static int offsetOfConvertDoubleElements() {
-        return (int)offsetof(ObjectElements, convertDoubleElements) - (int)sizeof(ObjectElements);
-    }
 
     static bool ConvertElementsToDoubles(JSContext *cx, uintptr_t elements);
 
     static const size_t VALUES_PER_HEADER = 2;
 };
 
 /* Shared singleton for objects with no elements. */
 extern HeapSlot *emptyObjectElements;