Bug 1533755 - Add SharedScriptData constructor. r=jwalden
authorTed Campbell <tcampbell@mozilla.com>
Tue, 12 Mar 2019 02:40:59 +0000
changeset 521522 68d86a527385
parent 521521 65c88218fbb2
child 521523 5ec94981916f
child 521559 aecb76a0cd77
push id10867
push userdvarga@mozilla.com
push dateThu, 14 Mar 2019 15:20:45 +0000
treeherdermozilla-beta@abad13547875 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwalden
bugs1533755
milestone67.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 1533755 - Add SharedScriptData constructor. r=jwalden This is based on PrivateScriptData approach. We make sure to call all default constructors even if the compiler will optimize away. Differential Revision: https://phabricator.services.mozilla.com/D22717
js/src/vm/JSScript.cpp
js/src/vm/JSScript.h
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -667,16 +667,76 @@ XDRResult js::PrivateScriptData::XDR(XDR
     for (uint32_t& elem : data->resumeOffsets()) {
       MOZ_TRY(xdr->codeUint32(&elem));
     }
   }
 
   return Ok();
 }
 
+// Placement-new elements of an array. This should optimize away for types with
+// trivial default initiation.
+template <typename T>
+static void DefaultInitializeElements(void* arrayPtr, size_t length) {
+  uintptr_t elem = reinterpret_cast<uintptr_t>(arrayPtr);
+  MOZ_ASSERT(elem % alignof(T) == 0);
+
+  for (size_t i = 0; i < length; ++i) {
+    new (reinterpret_cast<void*>(elem)) T;
+    elem += sizeof(T);
+  }
+}
+
+/* static */ size_t SharedScriptData::AllocationSize(uint32_t codeLength,
+                                                     uint32_t noteLength,
+                                                     uint32_t natoms) {
+  size_t size = sizeof(SharedScriptData);
+
+  size += natoms * sizeof(GCPtrAtom);
+  size += codeLength * sizeof(jsbytecode);
+  size += noteLength * sizeof(jssrcnote);
+
+  return size;
+}
+
+// Placement-new elements of an array. This should optimize away for types with
+// trivial default initiation.
+template <typename T>
+void SharedScriptData::initElements(size_t offset, size_t length) {
+  uintptr_t base = reinterpret_cast<uintptr_t>(this);
+  DefaultInitializeElements<T>(reinterpret_cast<void*>(base + offset), length);
+}
+
+SharedScriptData::SharedScriptData(uint32_t codeLength, uint32_t noteLength,
+                                   uint32_t natoms)
+    : codeLength_(codeLength), noteLength_(noteLength), natoms_(natoms) {
+  // Variable-length data begins immediately after SharedScriptData itself.
+  size_t cursor = sizeof(*this);
+
+  // Default-initialize trailing arrays.
+
+  static_assert(alignof(SharedScriptData) >= alignof(GCPtrAtom),
+                "Incompatible alignment");
+  initElements<GCPtrAtom>(cursor, natoms);
+  cursor += natoms * sizeof(GCPtrAtom);
+
+  static_assert(alignof(GCPtrAtom) >= alignof(jsbytecode),
+                "Incompatible alignment");
+  initElements<jsbytecode>(cursor, codeLength);
+  cursor += codeLength * sizeof(jsbytecode);
+
+  static_assert(alignof(jsbytecode) >= alignof(jssrcnote),
+                "Incompatible alignment");
+  initElements<jssrcnote>(cursor, noteLength);
+  cursor += noteLength * sizeof(jssrcnote);
+
+  // Sanity check
+  MOZ_ASSERT(AllocationSize(codeLength, noteLength, natoms) == cursor);
+}
+
 template <XDRMode mode>
 /* static */
 XDRResult SharedScriptData::XDR(XDRState<mode>* xdr, HandleScript script) {
   uint32_t natoms = 0;
   uint32_t codeLength = 0;
   uint32_t noteLength = 0;
 
   JSContext* cx = xdr->cx();
@@ -2864,51 +2924,31 @@ bool ScriptSource::setSourceMapURL(JSCon
  * Array elements   Pointed to by         Length
  * --------------   -------------         ------
  * GCPtrAtom        atoms()               natoms()
  * jsbytecode       code()                codeLength()
  * jsscrnote        notes()               numNotes()
  */
 
 SharedScriptData* js::SharedScriptData::new_(JSContext* cx, uint32_t codeLength,
-                                             uint32_t srcnotesLength,
+                                             uint32_t noteLength,
                                              uint32_t natoms) {
-  size_t dataLength = natoms * sizeof(GCPtrAtom) + codeLength + srcnotesLength;
-  size_t allocLength = offsetof(SharedScriptData, data_) + dataLength;
-  auto entry =
-      reinterpret_cast<SharedScriptData*>(cx->pod_malloc<uint8_t>(allocLength));
-  if (!entry) {
-    ReportOutOfMemory(cx);
+  // Compute size including trailing arrays
+  size_t size = AllocationSize(codeLength, noteLength, natoms);
+
+  // Allocate contiguous raw buffer
+  void* raw = cx->pod_malloc<uint8_t>(size);
+  MOZ_ASSERT(uintptr_t(raw) % alignof(SharedScriptData) == 0);
+  if (!raw) {
     return nullptr;
   }
 
-  /* Diagnostic for Bug 1399373.
-   * We expect bytecode is always non-empty. */
-  MOZ_DIAGNOSTIC_ASSERT(codeLength > 0);
-
-  entry->refCount_ = 0;
-  entry->natoms_ = natoms;
-  entry->codeLength_ = codeLength;
-  entry->noteLength_ = srcnotesLength;
-
-  /*
-   * Call constructors to initialize the storage that will be accessed as a
-   * GCPtrAtom array via atoms().
-   */
-  static_assert(offsetof(SharedScriptData, data_) % sizeof(GCPtrAtom) == 0,
-                "atoms must have GCPtrAtom alignment");
-  GCPtrAtom* atoms = entry->atoms();
-  for (unsigned i = 0; i < natoms; ++i) {
-    new (&atoms[i]) GCPtrAtom();
-  }
-
-  // Sanity check the dataLength() computation
-  MOZ_ASSERT(entry->dataLength() == dataLength);
-
-  return entry;
+  // Constuct the SharedScriptData. Trailing arrays are uninitialized but
+  // GCPtrs are put into a safe state.
+  return new (raw) SharedScriptData(codeLength, noteLength, natoms);
 }
 
 inline js::ScriptBytecodeHasher::Lookup::Lookup(SharedScriptData* data)
     : scriptData(data),
       hash(mozilla::HashBytes(scriptData->data(), scriptData->dataLength())) {}
 
 bool JSScript::createSharedScriptData(JSContext* cx, uint32_t codeLength,
                                       uint32_t noteLength, uint32_t natoms) {
@@ -2989,29 +3029,16 @@ void js::FreeScriptData(JSRuntime* rt) {
             scriptData, scriptData->refCount());
 #endif
     js_free(e.front());
   }
 
   table.clear();
 }
 
-// Placement-new elements of an array. This should optimize away for types with
-// trivial default initiation.
-template <typename T>
-static void DefaultInitializeElements(void* arrayPtr, size_t length) {
-  uintptr_t elem = reinterpret_cast<uintptr_t>(arrayPtr);
-  MOZ_ASSERT(elem % alignof(T) == 0);
-
-  for (size_t i = 0; i < length; ++i) {
-    new (reinterpret_cast<T*>(elem)) T;
-    elem += sizeof(T);
-  }
-}
-
 /* static */
 size_t PrivateScriptData::AllocationSize(uint32_t nscopes, uint32_t nconsts,
                                          uint32_t nobjects, uint32_t ntrynotes,
                                          uint32_t nscopenotes,
                                          uint32_t nresumeoffsets) {
   size_t size = sizeof(PrivateScriptData);
 
   if (nconsts) {
--- a/js/src/vm/JSScript.h
+++ b/js/src/vm/JSScript.h
@@ -1456,50 +1456,59 @@ class alignas(JS::Value) PrivateScriptDa
   // PrivateScriptData has trailing data so isn't copyable or movable.
   PrivateScriptData(const PrivateScriptData&) = delete;
   PrivateScriptData& operator=(const PrivateScriptData&) = delete;
 };
 
 /*
  * Common data that can be shared between many scripts in a single runtime.
  */
-class SharedScriptData {
+class alignas(uintptr_t) SharedScriptData final {
   // This class is reference counted as follows: each pointer from a JSScript
   // counts as one reference plus there may be one reference from the shared
   // script data table.
   mozilla::Atomic<uint32_t, mozilla::SequentiallyConsistent,
                   mozilla::recordreplay::Behavior::DontPreserve>
-      refCount_;
-
-  uint32_t natoms_;
-  uint32_t codeLength_;
-  uint32_t noteLength_;
-  uintptr_t data_[1];
+      refCount_ = {};
+
+  uint32_t codeLength_ = 0;
+  uint32_t noteLength_ = 0;
+  uint32_t natoms_ = 0;
+
+  // Size to allocate
+  static size_t AllocationSize(uint32_t codeLength, uint32_t noteLength,
+                               uint32_t natoms);
+
+  template <typename T>
+  void initElements(size_t offset, size_t length);
+
+  // Initialize to GC-safe state
+  SharedScriptData(uint32_t codeLength, uint32_t noteLength, uint32_t natoms);
 
  public:
   static SharedScriptData* new_(JSContext* cx, uint32_t codeLength,
-                                uint32_t srcnotesLength, uint32_t natoms);
+                                uint32_t noteLength, uint32_t natoms);
 
   uint32_t refCount() const { return refCount_; }
   void AddRef() { refCount_++; }
   void Release() {
     MOZ_ASSERT(refCount_ != 0);
     uint32_t remain = --refCount_;
     if (remain == 0) {
       js_free(this);
     }
   }
 
   size_t dataLength() const {
     return (natoms_ * sizeof(GCPtrAtom)) + codeLength_ + noteLength_;
   }
   const uint8_t* data() const {
-    return reinterpret_cast<const uint8_t*>(data_);
+    return reinterpret_cast<const uint8_t*>(this + 1);
   }
-  uint8_t* data() { return reinterpret_cast<uint8_t*>(data_); }
+  uint8_t* data() { return reinterpret_cast<uint8_t*>(this + 1); }
 
   uint32_t natoms() const { return natoms_; }
   GCPtrAtom* atoms() {
     if (!natoms_) {
       return nullptr;
     }
     return reinterpret_cast<GCPtrAtom*>(data());
   }
@@ -1512,35 +1521,31 @@ class SharedScriptData {
   uint32_t numNotes() const { return noteLength_; }
   jssrcnote* notes() {
     return reinterpret_cast<jssrcnote*>(data() + natoms_ * sizeof(GCPtrAtom) +
                                         codeLength_);
   }
 
   void traceChildren(JSTracer* trc);
 
-  static constexpr size_t offsetOfData() {
-    return offsetof(SharedScriptData, data_);
-  }
   static constexpr size_t offsetOfNatoms() {
     return offsetof(SharedScriptData, natoms_);
   }
 
   template <XDRMode mode>
   static MOZ_MUST_USE XDRResult XDR(js::XDRState<mode>* xdr,
                                     js::HandleScript script);
 
   static bool InitFromEmitter(JSContext* cx, js::HandleScript script,
                               js::frontend::BytecodeEmitter* bce);
 
   // Mark this SharedScriptData for use in a new zone
   void markForCrossZone(JSContext* cx);
 
- private:
-  SharedScriptData() = delete;
+  // SharedScriptData has trailing data so isn't copyable or movable.
   SharedScriptData(const SharedScriptData&) = delete;
   SharedScriptData& operator=(const SharedScriptData&) = delete;
 };
 
 struct ScriptBytecodeHasher {
   class Lookup {
     friend struct ScriptBytecodeHasher;