Bug 1358753 - Allocate ProxyValueArray inline in the object instead of using malloc. r=bhackett
authorJan de Mooij <jdemooij@mozilla.com>
Tue, 25 Apr 2017 08:51:30 +0200
changeset 354710 3d8027e7b9d7a849391bd928069d5259f19d8b3a
parent 354709 3e42f84996ea9a81fd68ce7ed102de0d40d0eab8
child 354711 f4513b54917c69b1317f96982f87d92b4ac7e716
push id89526
push userjandemooij@gmail.com
push dateTue, 25 Apr 2017 06:53:48 +0000
treeherdermozilla-inbound@3d8027e7b9d7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs1358753
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1358753 - Allocate ProxyValueArray inline in the object instead of using malloc. r=bhackett
js/src/gc/Marking.cpp
js/src/gc/Nursery.cpp
js/src/jsgc.cpp
js/src/jsgcinlines.h
js/src/jsobj.cpp
js/src/proxy/Proxy.cpp
js/src/vm/ProxyObject.cpp
js/src/vm/ProxyObject.h
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -2958,17 +2958,20 @@ js::TenuringTracer::moveObjectToTenured(
         InlineTypedObject::objectMovedDuringMinorGC(this, dst, src);
     } else if (src->is<TypedArrayObject>()) {
         tenuredSize += TypedArrayObject::objectMovedDuringMinorGC(this, dst, src, dstKind);
     } else if (src->is<UnboxedArrayObject>()) {
         tenuredSize += UnboxedArrayObject::objectMovedDuringMinorGC(this, dst, src, dstKind);
     } else if (src->is<ArgumentsObject>()) {
         tenuredSize += ArgumentsObject::objectMovedDuringMinorGC(this, dst, src);
     } else if (src->is<ProxyObject>()) {
-        tenuredSize += ProxyObject::objectMovedDuringMinorGC(this, dst, src);
+        // Objects in the nursery are never swapped so the proxy must have an
+        // inline ProxyValueArray.
+        MOZ_ASSERT(src->as<ProxyObject>().usingInlineValueArray());
+        dst->as<ProxyObject>().setInlineValueArray();
     } else if (JSObjectMovedOp op = dst->getClass()->extObjectMovedOp()) {
         op(dst, src);
     } else if (src->getClass()->hasFinalize()) {
         // Such objects need to be handled specially above to ensure any
         // additional nursery buffers they hold are moved.
         MOZ_RELEASE_ASSERT(CanNurseryAllocateFinalizedClass(src->getClass()));
         MOZ_CRASH("Unhandled JSCLASS_SKIP_NURSERY_FINALIZE Class");
     }
--- a/js/src/gc/Nursery.cpp
+++ b/js/src/gc/Nursery.cpp
@@ -267,17 +267,17 @@ js::Nursery::allocateObject(JSContext* c
     /* Make the object allocation. */
     JSObject* obj = static_cast<JSObject*>(allocate(size));
     if (!obj)
         return nullptr;
 
     /* If we want external slots, add them. */
     HeapSlot* slots = nullptr;
     if (numDynamic) {
-        MOZ_ASSERT(clasp->isNative() || clasp->isProxy());
+        MOZ_ASSERT(clasp->isNative());
         slots = static_cast<HeapSlot*>(allocateBuffer(cx->zone(), numDynamic * sizeof(HeapSlot)));
         if (!slots) {
             /*
              * It is safe to leave the allocated object uninitialized, since we
              * do not visit unallocated things in the nursery.
              */
             return nullptr;
         }
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -1951,16 +1951,19 @@ RelocateCell(Zone* zone, TenuredCell* sr
 
             // For copy-on-write objects that own their elements, fix up the
             // owner pointer to point to the relocated object.
             if (srcNative->denseElementsAreCopyOnWrite()) {
                 GCPtrNativeObject& owner = dstNative->getElementsHeader()->ownerObject();
                 if (owner == srcNative)
                     owner = dstNative;
             }
+        } else if (srcObj->is<ProxyObject>()) {
+            if (srcObj->as<ProxyObject>().usingInlineValueArray())
+                dstObj->as<ProxyObject>().setInlineValueArray();
         }
 
         // Call object moved hook if present.
         if (JSObjectMovedOp op = srcObj->getClass()->extObjectMovedOp())
             op(dstObj, srcObj);
 
         MOZ_ASSERT_IF(dstObj->isNative(),
                       !PtrIsInRange((const Value*)dstObj->as<NativeObject>().getDenseElements(),
--- a/js/src/jsgcinlines.h
+++ b/js/src/jsgcinlines.h
@@ -27,16 +27,19 @@ MakeAccessibleAfterMovingGC(JSObject* ob
         obj->as<NativeObject>().updateShapeAfterMovingGC();
 }
 
 static inline AllocKind
 GetGCObjectKind(const Class* clasp)
 {
     if (clasp == FunctionClassPtr)
         return AllocKind::FUNCTION;
+
+    MOZ_ASSERT(!clasp->isProxy(), "Proxies should use GetProxyGCObjectKind");
+
     uint32_t nslots = JSCLASS_RESERVED_SLOTS(clasp);
     if (clasp->flags & JSCLASS_HAS_PRIVATE)
         nslots++;
     return GetGCObjectKind(nslots);
 }
 
 inline void
 GCRuntime::poke()
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -1523,16 +1523,25 @@ void
 JSObject::fixDictionaryShapeAfterSwap()
 {
     // Dictionary shapes can point back to their containing objects, so after
     // swapping the guts of those objects fix the pointers up.
     if (isNative() && as<NativeObject>().inDictionaryMode())
         as<NativeObject>().shape_->listp = &as<NativeObject>().shape_;
 }
 
+static void
+RemoveFromStoreBuffer(JSContext* cx, js::detail::ProxyValueArray* values)
+{
+    StoreBuffer& sb = cx->zone()->group()->storeBuffer();
+    sb.unputValue(&values->privateSlot);
+    for (size_t i = 0; i < js::detail::PROXY_EXTRA_SLOTS; i++)
+        sb.unputValue(&values->extraSlots[i]);
+}
+
 /* Use this method with extreme caution. It trades the guts of two objects. */
 bool
 JSObject::swap(JSContext* cx, HandleObject a, HandleObject b)
 {
     // Ensure swap doesn't cause a finalizer to not be run.
     MOZ_ASSERT(IsBackgroundFinalized(a->asTenured().getAllocKind()) ==
                IsBackgroundFinalized(b->asTenured().getAllocKind()));
     MOZ_ASSERT(a->compartment() == b->compartment());
@@ -1567,30 +1576,40 @@ JSObject::swap(JSContext* cx, HandleObje
     // Watch for oddball objects that have special organizational issues and
     // can't be swapped.
     MOZ_ASSERT(!a->is<RegExpObject>() && !b->is<RegExpObject>());
     MOZ_ASSERT(!a->is<ArrayObject>() && !b->is<ArrayObject>());
     MOZ_ASSERT(!a->is<ArrayBufferObject>() && !b->is<ArrayBufferObject>());
     MOZ_ASSERT(!a->is<TypedArrayObject>() && !b->is<TypedArrayObject>());
     MOZ_ASSERT(!a->is<TypedObject>() && !b->is<TypedObject>());
 
+    bool aIsProxyWithInlineValues =
+        a->is<ProxyObject>() && a->as<ProxyObject>().usingInlineValueArray();
+    bool bIsProxyWithInlineValues =
+        b->is<ProxyObject>() && b->as<ProxyObject>().usingInlineValueArray();
+
     if (a->tenuredSizeOfThis() == b->tenuredSizeOfThis()) {
         // When both objects are the same size, just do a plain swap of their
         // contents.
         size_t size = a->tenuredSizeOfThis();
 
         char tmp[mozilla::tl::Max<sizeof(JSFunction), sizeof(JSObject_Slots16)>::value];
         MOZ_ASSERT(size <= sizeof(tmp));
 
         js_memcpy(tmp, a, size);
         js_memcpy(a, b, size);
         js_memcpy(b, tmp, size);
 
         a->fixDictionaryShapeAfterSwap();
         b->fixDictionaryShapeAfterSwap();
+
+        if (aIsProxyWithInlineValues)
+            b->as<ProxyObject>().setInlineValueArray();
+        if (bIsProxyWithInlineValues)
+            a->as<ProxyObject>().setInlineValueArray();
     } else {
         // Avoid GC in here to avoid confusing the tracing code with our
         // intermediate state.
         AutoSuppressGC suppress(cx);
 
         // When the objects have different sizes, they will have different
         // numbers of fixed slots before and after the swap, so the slots for
         // native objects will need to be rearranged.
@@ -1612,16 +1631,32 @@ JSObject::swap(JSContext* cx, HandleObje
         if (nb) {
             bpriv = nb->hasPrivate() ? nb->getPrivate() : nullptr;
             for (size_t i = 0; i < nb->slotSpan(); i++) {
                 if (!bvals.append(nb->getSlot(i)))
                     oomUnsafe.crash("JSObject::swap");
             }
         }
 
+        // Do the same for proxies storing ProxyValueArray inline.
+        ProxyObject* proxyA = a->is<ProxyObject>() ? &a->as<ProxyObject>() : nullptr;
+        ProxyObject* proxyB = b->is<ProxyObject>() ? &b->as<ProxyObject>() : nullptr;
+
+        Maybe<js::detail::ProxyValueArray> proxyAVals, proxyBVals;
+        if (aIsProxyWithInlineValues) {
+            js::detail::ProxyValueArray* values = js::detail::GetProxyDataLayout(proxyA)->values;
+            proxyAVals.emplace(*values);
+            RemoveFromStoreBuffer(cx, values);
+        }
+        if (bIsProxyWithInlineValues) {
+            js::detail::ProxyValueArray* values = js::detail::GetProxyDataLayout(proxyB)->values;
+            proxyBVals.emplace(*values);
+            RemoveFromStoreBuffer(cx, values);
+        }
+
         // Swap the main fields of the objects, whether they are native objects or proxies.
         char tmp[sizeof(JSObject_Slots0)];
         js_memcpy(&tmp, a, sizeof tmp);
         js_memcpy(a, b, sizeof tmp);
         js_memcpy(b, &tmp, sizeof tmp);
 
         a->fixDictionaryShapeAfterSwap();
         b->fixDictionaryShapeAfterSwap();
@@ -1629,16 +1664,24 @@ JSObject::swap(JSContext* cx, HandleObje
         if (na) {
             if (!NativeObject::fillInAfterSwap(cx, b.as<NativeObject>(), avals, apriv))
                 oomUnsafe.crash("fillInAfterSwap");
         }
         if (nb) {
             if (!NativeObject::fillInAfterSwap(cx, a.as<NativeObject>(), bvals, bpriv))
                 oomUnsafe.crash("fillInAfterSwap");
         }
+        if (aIsProxyWithInlineValues) {
+            if (!b->as<ProxyObject>().initExternalValueArrayAfterSwap(cx, proxyAVals.ref()))
+                oomUnsafe.crash("initExternalValueArray");
+        }
+        if (bIsProxyWithInlineValues) {
+            if (!a->as<ProxyObject>().initExternalValueArrayAfterSwap(cx, proxyBVals.ref()))
+                oomUnsafe.crash("initExternalValueArray");
+        }
     }
 
     // Swapping the contents of two objects invalidates type sets which contain
     // either of the objects, so mark all such sets as unknown.
     MarkObjectGroupUnknownProperties(cx, a->group());
     MarkObjectGroupUnknownProperties(cx, b->group());
 
     /*
--- a/js/src/proxy/Proxy.cpp
+++ b/js/src/proxy/Proxy.cpp
@@ -707,17 +707,19 @@ js::proxy_WeakmapKeyDelegate(JSObject* o
 static void
 proxy_Finalize(FreeOp* fop, JSObject* obj)
 {
     // Suppress a bogus warning about finalize().
     JS::AutoSuppressGCAnalysis nogc;
 
     MOZ_ASSERT(obj->is<ProxyObject>());
     obj->as<ProxyObject>().handler()->finalize(fop, obj);
-    js_free(js::detail::GetProxyDataLayout(obj)->values);
+
+    if (!obj->as<ProxyObject>().usingInlineValueArray())
+        js_free(js::detail::GetProxyDataLayout(obj)->values);
 }
 
 static void
 proxy_ObjectMoved(JSObject* obj, const JSObject* old)
 {
     MOZ_ASSERT(obj->is<ProxyObject>());
     obj->as<ProxyObject>().handler()->objectMoved(obj, old);
 }
--- a/js/src/vm/ProxyObject.cpp
+++ b/js/src/vm/ProxyObject.cpp
@@ -9,16 +9,31 @@
 #include "jscompartment.h"
 
 #include "proxy/DeadObjectProxy.h"
 
 #include "jsobjinlines.h"
 
 using namespace js;
 
+static gc::AllocKind
+GetProxyGCObjectKind(const BaseProxyHandler* handler, const Value& priv)
+{
+    static_assert(sizeof(js::detail::ProxyValueArray) % sizeof(js::HeapSlot) == 0,
+                  "ProxyValueArray must be a multiple of HeapSlot");
+
+    uint32_t nslots = sizeof(js::detail::ProxyValueArray) / sizeof(HeapSlot);
+
+    gc::AllocKind kind = gc::GetGCObjectKind(nslots);
+    if (handler->finalizeInBackground(priv))
+        kind = GetBackgroundAllocKind(kind);
+
+    return kind;
+}
+
 /* static */ ProxyObject*
 ProxyObject::New(JSContext* cx, const BaseProxyHandler* handler, HandleValue priv, TaggedProto proto_,
                  const ProxyOptions& options)
 {
     Rooted<TaggedProto> proto(cx, proto_);
 
     const Class* clasp = options.clasp();
 
@@ -48,59 +63,42 @@ ProxyObject::New(JSContext* cx, const Ba
         newKind = SingletonObject;
     } else if ((priv.isGCThing() && priv.toGCThing()->isTenured()) ||
                !handler->canNurseryAllocate() ||
                !handler->finalizeInBackground(priv))
     {
         newKind = TenuredObject;
     }
 
-    gc::AllocKind allocKind = gc::GetGCObjectKind(clasp);
-    if (handler->finalizeInBackground(priv))
-        allocKind = GetBackgroundAllocKind(allocKind);
+    gc::AllocKind allocKind = GetProxyGCObjectKind(handler, priv);
 
     AutoSetNewObjectMetadata metadata(cx);
     // Note: this will initialize the object's |data| to strange values, but we
     // will immediately overwrite those below.
     ProxyObject* proxy;
     JS_TRY_VAR_OR_RETURN_NULL(cx, proxy, create(cx, clasp, proto, allocKind, newKind));
 
+    proxy->setInlineValueArray();
     new (proxy->data.values) detail::ProxyValueArray;
     proxy->data.handler = handler;
     proxy->setCrossCompartmentPrivate(priv);
 
     /* Don't track types of properties of non-DOM and non-singleton proxies. */
     if (newKind != SingletonObject && !clasp->isDOMClass())
         MarkObjectGroupUnknownProperties(cx, proxy->group());
 
     return proxy;
 }
 
 gc::AllocKind
 ProxyObject::allocKindForTenure() const
 {
-    gc::AllocKind allocKind = gc::GetGCObjectKind(group()->clasp());
-    if (data.handler->finalizeInBackground(const_cast<ProxyObject*>(this)->private_()))
-        allocKind = GetBackgroundAllocKind(allocKind);
-    return allocKind;
-}
-
-/* static */ size_t
-ProxyObject::objectMovedDuringMinorGC(TenuringTracer* trc, JSObject* dst, JSObject* src)
-{
-    ProxyObject& psrc = src->as<ProxyObject>();
-    ProxyObject& pdst = dst->as<ProxyObject>();
-
-    // We're about to sweep the nursery heap, so migrate the inline
-    // ProxyValueArray to the malloc heap if they were nursery allocated.
-    if (dst->zone()->group()->nursery().isInside(psrc.data.values))
-        pdst.data.values = js_new<detail::ProxyValueArray>(*psrc.data.values);
-    else
-        dst->zone()->group()->nursery().removeMallocedBuffer(psrc.data.values);
-    return sizeof(detail::ProxyValueArray);
+    MOZ_ASSERT(usingInlineValueArray());
+    Value priv = const_cast<ProxyObject*>(this)->private_();
+    return GetProxyGCObjectKind(data.handler, priv);
 }
 
 void
 ProxyObject::setCrossCompartmentPrivate(const Value& priv)
 {
     *slotOfPrivate() = priv;
 }
 
@@ -164,22 +162,17 @@ ProxyObject::create(JSContext* cx, const
             return cx->alreadyReportedOOM();
 
         comp->newProxyCache.add(group, shape);
     }
 
     gc::InitialHeap heap = GetInitialHeap(newKind, clasp);
     debugCheckNewObject(group, shape, allocKind, heap);
 
-    // Proxy objects overlay the |slots| field with a ProxyValueArray.
-    static_assert(sizeof(js::detail::ProxyValueArray) % sizeof(js::HeapSlot) == 0,
-                  "ProxyValueArray must be a multiple of HeapSlot");
-    static const size_t NumDynamicSlots = sizeof(js::detail::ProxyValueArray) / sizeof(HeapSlot);
-
-    JSObject* obj = js::Allocate<JSObject>(cx, allocKind, NumDynamicSlots, heap, clasp);
+    JSObject* obj = js::Allocate<JSObject>(cx, allocKind, /* numDynamicSlots = */ 0, heap, clasp);
     if (!obj)
         return cx->alreadyReportedOOM();
 
     ProxyObject* pobj = static_cast<ProxyObject*>(obj);
     pobj->group_.init(group);
     pobj->initShape(shape);
 
     MOZ_ASSERT(clasp->shouldDelayMetadataBuilder());
@@ -192,15 +185,28 @@ ProxyObject::create(JSContext* cx, const
         if (!JSObject::setSingleton(cx, pobjRoot))
             return cx->alreadyReportedOOM();
         pobj = pobjRoot;
     }
 
     return pobj;
 }
 
+bool
+ProxyObject::initExternalValueArrayAfterSwap(JSContext* cx, const detail::ProxyValueArray& src)
+{
+    MOZ_ASSERT(getClass()->isProxy());
+
+    auto* values = cx->zone()->new_<detail::ProxyValueArray>(src);
+    if (!values)
+        return false;
+
+    data.values = values;
+    return true;
+}
+
 JS_FRIEND_API(void)
 js::SetValueInProxy(Value* slot, const Value& value)
 {
     // Slots in proxies are not GCPtrValues, so do a cast whenever assigning
     // values to them which might trigger a barrier.
     *reinterpret_cast<GCPtrValue*>(slot) = value;
 }
--- a/js/src/vm/ProxyObject.h
+++ b/js/src/vm/ProxyObject.h
@@ -34,16 +34,30 @@ class ProxyObject : public ShapedObject
     static JS::Result<ProxyObject*, JS::OOM&>
     create(JSContext* cx, const js::Class* clasp, Handle<TaggedProto> proto,
            js::gc::AllocKind allocKind, js::NewObjectKind newKind);
 
   public:
     static ProxyObject* New(JSContext* cx, const BaseProxyHandler* handler, HandleValue priv,
                             TaggedProto proto_, const ProxyOptions& options);
 
+    // Proxies usually store their ProxyValueArray inline in the object.
+    // There's one unfortunate exception: when a proxy is swapped with another
+    // object, and the sizes don't match, we malloc the ProxyValueArray.
+    void* inlineDataStart() const {
+        return (void*)(uintptr_t(this) + sizeof(ProxyObject));
+    }
+    bool usingInlineValueArray() const {
+        return data.values == inlineDataStart();
+    }
+    void setInlineValueArray() {
+        data.values = reinterpret_cast<detail::ProxyValueArray*>(inlineDataStart());
+    }
+    MOZ_MUST_USE bool initExternalValueArrayAfterSwap(JSContext* cx, const detail::ProxyValueArray& src);
+
     const Value& private_() {
         return GetProxyPrivate(this);
     }
 
     void setCrossCompartmentPrivate(const Value& priv);
     void setSameCompartmentPrivate(const Value& priv);
 
     GCPtrValue* slotOfPrivate() {
@@ -77,17 +91,16 @@ class ProxyObject : public ShapedObject
         return GetProxyExtra(const_cast<ProxyObject*>(this), n);
     }
 
     void setExtra(size_t n, const Value& extra) {
         SetProxyExtra(this, n, extra);
     }
 
     gc::AllocKind allocKindForTenure() const;
-    static size_t objectMovedDuringMinorGC(TenuringTracer* trc, JSObject* dst, JSObject* src);
 
   private:
     GCPtrValue* slotOfExtra(size_t n) {
         MOZ_ASSERT(n < detail::PROXY_EXTRA_SLOTS);
         return reinterpret_cast<GCPtrValue*>(&detail::GetProxyDataLayout(this)->values->extraSlots[n]);
     }
 
     static bool isValidProxyClass(const Class* clasp) {