Bug 1978645 - Part 2: Use buffer allocator for wasm array data r=jseward

jonco3 · jonco3 · commit 1884530df58f · 2025-10-06T09:54:00.000Z
Differential Revision: https://phabricator.services.mozilla.com/D265784
diff --git a/js/src/wasm/WasmGcObject-inl.h b/js/src/wasm/WasmGcObject-inl.h
@@ -193,32 +193,23 @@ MOZ_ALWAYS_INLINE WasmArrayObject* WasmArrayObject::createArrayOOL(
   // arrays use createArrayIL.
   MOZ_ASSERT(storageBytes > WasmArrayObject_MaxInlineBytes);
 
-  // Allocate the outline data before allocating the object so that we can
-  // infallibly initialize the pointer on the array object after it is
-  // allocated.
-  Nursery& nursery = cx->nursery();
-  PointerAndUint7 outlineAlloc(nullptr, 0);
-  outlineAlloc = nursery.mallocedBlockCache().alloc(storageBytes);
-  if (MOZ_UNLIKELY(!outlineAlloc.pointer())) {
+  auto* arrayObj = (WasmArrayObject*)cx->newCell<WasmGcObject>(
+      allocKind, initialHeap, typeDefData->clasp, allocSite);
+  if (MOZ_UNLIKELY(!arrayObj)) {
     ReportOutOfMemory(cx);
     return nullptr;
   }
 
-  // It's unfortunate that `arrayObj` has to be rooted, since this is a hot
-  // path and rooting costs around 15 instructions.  It is the call to
-  // registerTrailer that makes it necessary.
-  Rooted<WasmArrayObject*> arrayObj(cx);
-  arrayObj = (WasmArrayObject*)cx->newCell<WasmGcObject>(
-      allocKind, initialHeap, typeDefData->clasp, allocSite);
-  if (MOZ_UNLIKELY(!arrayObj)) {
+  uint8_t* outlineAlloc = AllocateCellBuffer<uint8_t>(
+      cx, arrayObj, storageBytes, MaxNurseryTrailerSize);
+  if (MOZ_UNLIKELY(!outlineAlloc)) {
+    arrayObj->numElements_ = 0;
+    arrayObj->data_ = nullptr;
     ReportOutOfMemory(cx);
-    if (outlineAlloc.pointer()) {
-      nursery.mallocedBlockCache().free(outlineAlloc);
-    }
     return nullptr;
   }
 
-  DataHeader* outlineHeader = (DataHeader*)outlineAlloc.pointer();
+  DataHeader* outlineHeader = (DataHeader*)outlineAlloc;
   uint8_t* outlineData = (uint8_t*)(outlineHeader + 1);
   *outlineHeader = DataIsOOL;
 
@@ -234,25 +225,6 @@ MOZ_ALWAYS_INLINE WasmArrayObject* WasmArrayObject::createArrayOOL(
 
   MOZ_ASSERT(!arrayObj->isDataInline());
 
-  if (MOZ_LIKELY(js::gc::IsInsideNursery(arrayObj))) {
-    // We need to register the OOL area with the nursery, so it will be freed
-    // after GCing of the nursery if `arrayObj_` doesn't make it into the
-    // tenured heap.  Note, the nursery will keep a running total of the
-    // current trailer block sizes, so it can decide to do a (minor)
-    // collection if that becomes excessive.
-    if (MOZ_UNLIKELY(!nursery.registerTrailer(outlineAlloc, storageBytes))) {
-      nursery.mallocedBlockCache().free(outlineAlloc);
-      ReportOutOfMemory(cx);
-      return nullptr;
-    }
-  } else {
-    MOZ_ASSERT(arrayObj->isTenured());
-    // Register the trailer size with the major GC mechanism, so that can also
-    // is able to decide if that space use warrants a (major) collection.
-    AddCellMemory(arrayObj, storageBytes + wasm::TrailerBlockOverhead,
-                  MemoryUse::WasmTrailerBlock);
-  }
-
   MOZ_ASSERT(typeDefData->clasp->shouldDelayMetadataBuilder());
   cx->realm()->setObjectPendingMetadata(arrayObj);
 
diff --git a/js/src/wasm/WasmGcObject.cpp b/js/src/wasm/WasmGcObject.cpp
@@ -8,7 +8,7 @@
 
 #include <algorithm>
 
-#include "gc/Marking.h"
+#include "gc/Tracer.h"
 #include "js/CharacterEncoding.h"
 #include "js/friend/ErrorMessages.h"  // js::GetErrorMessage, JSMSG_*
 #include "js/PropertySpec.h"
@@ -36,126 +36,21 @@ using namespace wasm;
 
 // [SMDOC] Management of OOL storage areas for Wasm{Array,Struct}Object.
 //
-// WasmArrayObject always has its payload data stored in a block the C++-heap,
-// which is pointed to from the WasmArrayObject.  The same is true for
-// WasmStructObject in the case where the fields cannot fit in the object
-// itself.  These C++ blocks are in some places referred to as "trailer blocks".
+// WasmArrayObject always has its payload data stored in a block which is
+// pointed to from the WasmArrayObject. The same is true for WasmStructObject in
+// the case where the fields cannot fit in the object itself. These blocks are
+// in some places referred to as "trailer blocks".
 //
-// The presence of trailer blocks complicates the use of generational GC (that
-// is, Nursery allocation) of Wasm{Array,Struct}Object.  In particular:
+// These blocks are allocated in the same way as JSObects slots and element
+// buffers, either using the GC's buffer allocator or directly in the nursery if
+// they are small enough.
 //
-// (1) For objects which do not get tenured at minor collection, there must be
-//     a way to free the associated trailer, but there is no way to visit
-//     non-tenured blocks during minor collection.
+// They require the use of WasmArrayObject/WasmStructObject::obj_moved hooks to
+// update the pointer and mark the allocation when the object gets tenured, and
+// also update the pointer in case it is an internal pointer when the object is
+// moved.
 //
-// (2) Even if (1) were solved, calling js_malloc/js_free for every object
-//     creation-death cycle is expensive, possibly around 400 machine
-//     instructions, and we expressly want to avoid that in a generational GC
-//     scenario.
-//
-// The following scheme is therefore employed.
-//
-// (a) gc::Nursery maintains a pool of available C++-heap-allocated blocks --
-//     a js::MallocedBlockCache -- and the intention is that trailers are
-//     allocated from this pool and freed back into it whenever possible.
-//
-// (b) WasmArrayObject::createArrayNonEmpty and
-//     WasmStructObject::createStructOOL always request trailer allocation
-//     from the nursery's cache (a).  If the cache cannot honour the request
-//     directly it will allocate directly from js_malloc; we hope this happens
-//     only infrequently.
-//
-// (c) The allocated block is returned as a js::PointerAndUint7, a pair that
-//     holds the trailer block pointer and an auxiliary tag that the
-//     js::MallocedBlockCache needs to see when the block is freed.
-//
-//     The raw trailer block pointer (a `void*`) is stored in the
-//     Wasm{Array,Struct}Object OOL data field.  These objects are not aware
-//     of and do not interact with js::PointerAndUint7, and nor does any
-//     JIT-generated code.
-//
-// (d) Still in WasmArrayObject::createArrayNonEmpty and
-//     WasmStructObject::createStructOOL, if the object was allocated in the
-//     nursery, then the resulting js::PointerAndUint7 is "registered" with
-//     the nursery by handing it to Nursery::registerTrailer.
-//
-// (e) When a minor collection happens (Nursery::doCollection), we are
-//     notified of objects that are moved by calls to the ::obj_moved methods
-//     in this file.  For those objects that have been tenured, the raw
-//     trailer pointer is "unregistered" with the nursery by handing it to
-//     Nursery::unregisterTrailer.
-//
-// (f) Still during minor collection: The nursery now knows both the set of
-//     trailer blocks added, and those removed because the corresponding
-//     object has been tenured.  The difference between these two sets (that
-//     is, `added - removed`) is the set of trailer blocks corresponding to
-//     blocks that didn't get tenured.  That set is computed and freed (back
-//     to the nursery's js::MallocedBlockCache) by Nursery::freeTrailerBlocks.
-//
-// (g) At the end of minor collection, the added and removed sets are made
-//     empty, and the cycle begins again.
-//
-// (h) Also at the end of minor collection, a call to
-//     `mallocedBlockCache_.preen` hands a few blocks in the cache back to
-//     js_free.  This mechanism exists so as to ensure that unused blocks do
-//     not remain in the cache indefinitely.
-//
-// (i) In order that the tenured heap is collected "often enough" in the case
-//     where the trailer blocks are large (relative to their owning objects),
-//     we have to tell the tenured heap about the sizes of trailers entering
-//     and leaving it.  This is done via calls to AddCellMemory and
-//     GCContext::removeCellMemory.
-//
-// (j) For objects that got tenured, we are eventually notified of their death
-//     by a call to the ::obj_finalize methods below.  At that point we hand
-//     their block pointers to js_free.
-//
-// (k) When the nursery is eventually destroyed, all blocks in its block cache
-//     are handed to js_free.  Hence, at process exit, provided all nurseries
-//     are first collected and then their destructors run, no C++ heap blocks
-//     are leaked.
-//
-// As a result of this scheme, trailer blocks associated with what we hope is
-// the frequent case -- objects that are allocated but never make it out of
-// the nursery -- are cycled through the nursery's block cache.
-//
-// Trailers associated with tenured blocks cannot participate though; they are
-// always returned to js_free.  Making them participate is difficult: it would
-// require changing their owning object's OOL data pointer to be a
-// js::PointerAndUint7 rather than a raw `void*`, so that then the blocks
-// could be released to the cache in the ::obj_finalize methods.  This would
-// however require changes in the generated code for array element and OOL
-// struct element accesses.
-//
-// It would also lead to threading difficulties, because the ::obj_finalize
-// methods run on a background thread, whilst allocation from the cache
-// happens on the main thread, but the MallocedBlockCache is not thread safe.
-// Making it thread safe would entail adding a locking mechanism, but that's
-// potentially slow and so negates the point of having a cache at all.
-//
-// Here's a short summary of the trailer block life cycle:
-//
-// * allocated:
-//
-//   - in WasmArrayObject::createArrayNonEmpty
-//     and WasmStructObject::createStructOOL
-//
-//   - by calling the nursery's MallocBlockCache alloc method
-//
-// * deallocated:
-//
-//   - for non-tenured objects, in the collector itself,
-//     in Nursery::doCollection calling Nursery::freeTrailerBlocks,
-//     releasing to the nursery's block cache
-//
-//   - for tenured objects, in the ::obj_finalize methods, releasing directly
-//     to js_free
-//
-// If this seems confusing ("why is it ok to allocate from the cache but
-// release to js_free?"), remember that the cache holds blocks previously
-// obtained from js_malloc but which are *not* currently in use.  Hence it is
-// fine to give them back to js_free; that just makes the cache a bit emptier
-// but has no effect on correctness.
+// The blocks are freed by the GC when no longer referenced.
 
 //=========================================================================
 // WasmGcObject
@@ -371,6 +266,15 @@ void WasmArrayObject::obj_trace(JSTracer* trc, JSObject* object) {
   WasmArrayObject& arrayObj = object->as<WasmArrayObject>();
   uint8_t* data = arrayObj.data_;
 
+  if (!arrayObj.isDataInline()) {
+    uint8_t* outlineAlloc = (uint8_t*)dataHeaderFromDataPointer(arrayObj.data_);
+    uint8_t* prior = outlineAlloc;
+    TraceBufferEdge(trc, &arrayObj, &outlineAlloc, "WasmArrayObject storage");
+    if (outlineAlloc != prior) {
+      arrayObj.data_ = (uint8_t*)(((DataHeader*)outlineAlloc) + 1);
+    }
+  }
+
   const auto& typeDef = arrayObj.typeDef();
   const auto& arrayType = typeDef.arrayType();
   if (!arrayType.elementType().isRefRepr()) {
@@ -385,34 +289,6 @@ void WasmArrayObject::obj_trace(JSTracer* trc, JSObject* object) {
   }
 }
 
-/* static */
-void WasmArrayObject::obj_finalize(JS::GCContext* gcx, JSObject* object) {
-  // This method, and also ::obj_moved and the WasmStructObject equivalents,
-  // assumes that the object's TypeDef (as reachable via its SuperTypeVector*)
-  // stays alive at least as long as the object.
-  WasmArrayObject& arrayObj = object->as<WasmArrayObject>();
-  if (!arrayObj.isDataInline()) {
-    // Free the trailer block.  Unfortunately we can't give it back to the
-    // malloc'd block cache because we might not be running on the main
-    // thread, and the cache isn't thread-safe.
-    js_free(arrayObj.dataHeader());
-    // And tell the tenured-heap accounting machinery that the trailer has
-    // been freed.
-    const TypeDef& typeDef = arrayObj.typeDef();
-    MOZ_ASSERT(typeDef.isArrayType());
-    // arrayObj.numElements_ was validated to not overflow when constructing the
-    // array
-    size_t trailerSize = calcStorageBytesUnchecked(
-        typeDef.arrayType().elementType().size(), arrayObj.numElements_);
-    // Ensured by WasmArrayObject::createArrayNonEmpty.
-    MOZ_RELEASE_ASSERT(trailerSize <= size_t(MaxArrayPayloadBytes));
-    gcx->removeCellMemory(&arrayObj, trailerSize + TrailerBlockOverhead,
-                          MemoryUse::WasmTrailerBlock);
-    // For safety
-    arrayObj.data_ = nullptr;
-  }
-}
-
 /* static */
 size_t WasmArrayObject::obj_moved(JSObject* obj, JSObject* old) {
   // Moving inline arrays requires us to update the data pointer.
@@ -437,9 +313,13 @@ size_t WasmArrayObject::obj_moved(JSObject* obj, JSObject* old) {
           typeDef.arrayType().elementType().size(), arrayObj.numElements_);
       // Ensured by WasmArrayObject::createArrayOOL.
       MOZ_RELEASE_ASSERT(trailerSize <= size_t(MaxArrayPayloadBytes));
-      nursery.trackTrailerOnPromotion(arrayObj.dataHeader(), obj, trailerSize,
-                                      TrailerBlockOverhead,
-                                      MemoryUse::WasmTrailerBlock);
+      uint8_t* outlineAlloc =
+          (uint8_t*)dataHeaderFromDataPointer(arrayObj.data_);
+      uint8_t* prior = outlineAlloc;
+      nursery.maybeMoveBufferOnPromotion(&outlineAlloc, obj, trailerSize);
+      if (outlineAlloc != prior) {
+        arrayObj.data_ = (uint8_t*)(((DataHeader*)outlineAlloc) + 1);
+      }
     }
   }
 
@@ -471,20 +351,19 @@ static const JSClassOps WasmArrayObjectClassOps = {
     nullptr, /* delProperty */
     nullptr, /* enumerate   */
     WasmGcObject::obj_newEnumerate,
-    nullptr,                       /* resolve     */
-    nullptr,                       /* mayResolve  */
-    WasmArrayObject::obj_finalize, /* finalize    */
-    nullptr,                       /* call        */
-    nullptr,                       /* construct   */
+    nullptr, /* resolve     */
+    nullptr, /* mayResolve  */
+    nullptr, /* finalize    */
+    nullptr, /* call        */
+    nullptr, /* construct   */
     WasmArrayObject::obj_trace,
 };
 static const ClassExtension WasmArrayObjectClassExt = {
     WasmArrayObject::obj_moved, /* objectMovedOp */
 };
 const JSClass WasmArrayObject::class_ = {
     "WasmArrayObject",
-    JSClass::NON_NATIVE | JSCLASS_DELAY_METADATA_BUILDER |
-        JSCLASS_BACKGROUND_FINALIZE | JSCLASS_SKIP_NURSERY_FINALIZE,
+    JSClass::NON_NATIVE | JSCLASS_DELAY_METADATA_BUILDER,
     &WasmArrayObjectClassOps,
     JS_NULL_CLASS_SPEC,
     &WasmArrayObjectClassExt,
