Bug 1533755 - Use RefPtr for SharedScriptData pointers. r=jandem
authorTed Campbell <tcampbell@mozilla.com>
Tue, 12 Mar 2019 02:40:29 +0000
changeset 521521 65c88218fbb26505573c73e2ff25fdf1a04d1a7d
parent 521520 53c759ed45e4739fafdbb80bbf78376de37d49b0
child 521522 68d86a52738530fd10d3e0fd9b4dd0b270c89b81
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)
reviewersjandem
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 - Use RefPtr for SharedScriptData pointers. r=jandem Leave manual refcounting in the ScriptDataTable for now since it requires a bit of care to make the automatic types do the right thing when sweeping. Differential Revision: https://phabricator.services.mozilla.com/D22718
js/src/vm/JSScript.cpp
js/src/vm/JSScript.h
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -2903,49 +2903,26 @@ SharedScriptData* js::SharedScriptData::
   // Sanity check the dataLength() computation
   MOZ_ASSERT(entry->dataLength() == dataLength);
 
   return entry;
 }
 
 inline js::ScriptBytecodeHasher::Lookup::Lookup(SharedScriptData* data)
     : scriptData(data),
-      hash(mozilla::HashBytes(scriptData->data(), scriptData->dataLength())) {
-  scriptData->incRefCount();
-}
-
-inline js::ScriptBytecodeHasher::Lookup::~Lookup() {
-  scriptData->decRefCount();
-}
+      hash(mozilla::HashBytes(scriptData->data(), scriptData->dataLength())) {}
 
 bool JSScript::createSharedScriptData(JSContext* cx, uint32_t codeLength,
                                       uint32_t noteLength, uint32_t natoms) {
-  MOZ_ASSERT(!scriptData());
-  SharedScriptData* ssd =
-      SharedScriptData::new_(cx, codeLength, noteLength, natoms);
-  if (!ssd) {
-    return false;
-  }
-
-  setScriptData(ssd);
-  return true;
+  MOZ_ASSERT(!scriptData_);
+  scriptData_ = SharedScriptData::new_(cx, codeLength, noteLength, natoms);
+  return !!scriptData_;
 }
 
-void JSScript::freeScriptData() {
-  if (scriptData_) {
-    scriptData_->decRefCount();
-    scriptData_ = nullptr;
-  }
-}
-
-void JSScript::setScriptData(js::SharedScriptData* data) {
-  MOZ_ASSERT(!scriptData_);
-  scriptData_ = data;
-  scriptData_->incRefCount();
-}
+void JSScript::freeScriptData() { scriptData_ = nullptr; }
 
 /*
  * Takes ownership of its *ssd parameter and either adds it into the runtime's
  * ScriptDataTable or frees it if a matching entry already exists.
  *
  * Sets the |code| and |atoms| fields on the given JSScript.
  */
 bool JSScript::shareScriptData(JSContext* cx) {
@@ -2957,44 +2934,44 @@ bool JSScript::shareScriptData(JSContext
   // counted, it also will be freed after releasing the lock if necessary.
   ScriptBytecodeHasher::Lookup lookup(ssd);
 
   AutoLockScriptData lock(cx->runtime());
 
   ScriptDataTable::AddPtr p = cx->scriptDataTable(lock).lookupForAdd(lookup);
   if (p) {
     MOZ_ASSERT(ssd != *p);
-    freeScriptData();
-    setScriptData(*p);
+    scriptData_ = *p;
   } else {
     if (!cx->scriptDataTable(lock).add(p, ssd)) {
-      freeScriptData();
       ReportOutOfMemory(cx);
       return false;
     }
 
     // Being in the table counts as a reference on the script data.
-    scriptData()->incRefCount();
-  }
-
+    ssd->AddRef();
+  }
+
+  // Refs: JSScript, ScriptDataTable
   MOZ_ASSERT(scriptData()->refCount() >= 2);
+
   return true;
 }
 
 void js::SweepScriptData(JSRuntime* rt) {
   // Entries are removed from the table when their reference count is one,
   // i.e. when the only reference to them is from the table entry.
 
   AutoLockScriptData lock(rt);
   ScriptDataTable& table = rt->scriptDataTable(lock);
 
   for (ScriptDataTable::Enum e(table); !e.empty(); e.popFront()) {
     SharedScriptData* scriptData = e.front();
     if (scriptData->refCount() == 1) {
-      scriptData->decRefCount();
+      scriptData->Release();
       e.removeFront();
     }
   }
 }
 
 void js::FreeScriptData(JSRuntime* rt) {
   AutoLockScriptData lock(rt);
 
@@ -3735,19 +3712,17 @@ void JSScript::finalize(FreeOp* fop) {
   }
 #endif
 
   if (data_) {
     AlwaysPoison(data_, 0xdb, computedSizeOfData(), MemCheckKind::MakeNoAccess);
     fop->free_(data_);
   }
 
-  if (scriptData_) {
-    scriptData_->decRefCount();
-  }
+  freeScriptData();
 
   // In most cases, our LazyScript's script pointer will reference this
   // script, and thus be nulled out by normal weakref processing. However, if
   // we unlazified the LazyScript during incremental sweeping, it will have a
   // completely different JSScript.
   MOZ_ASSERT_IF(
       lazyScript && !IsAboutToBeFinalizedUnbarriered(&lazyScript),
       !lazyScript->hasScript() || lazyScript->maybeScriptUnbarriered() != this);
@@ -4238,17 +4213,17 @@ JSScript* js::detail::CopyScript(JSConte
     return nullptr;
   }
 
   // The SharedScriptData can be reused by any zone in the Runtime as long as
   // we make sure to mark first (to sync Atom pointers).
   if (cx->zone() != src->zoneFromAnyThread()) {
     src->scriptData()->markForCrossZone(cx);
   }
-  dst->setScriptData(src->scriptData());
+  dst->scriptData_ = src->scriptData();
 
   return dst;
 }
 
 JSScript* js::CloneGlobalScript(JSContext* cx, ScopeKind scopeKind,
                                 HandleScript src) {
   MOZ_ASSERT(scopeKind == ScopeKind::Global ||
              scopeKind == ScopeKind::NonSyntactic);
--- a/js/src/vm/JSScript.h
+++ b/js/src/vm/JSScript.h
@@ -9,16 +9,17 @@
 #ifndef vm_JSScript_h
 #define vm_JSScript_h
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/Atomics.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/MaybeOneOf.h"
 #include "mozilla/MemoryReporting.h"
+#include "mozilla/RefPtr.h"
 #include "mozilla/Span.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/Utf8.h"
 #include "mozilla/Variant.h"
 
 #include <type_traits>  // std::is_same
 #include <utility>      // std::move
 
@@ -1473,18 +1474,18 @@ class SharedScriptData {
   uint32_t noteLength_;
   uintptr_t data_[1];
 
  public:
   static SharedScriptData* new_(JSContext* cx, uint32_t codeLength,
                                 uint32_t srcnotesLength, uint32_t natoms);
 
   uint32_t refCount() const { return refCount_; }
-  void incRefCount() { refCount_++; }
-  void decRefCount() {
+  void AddRef() { refCount_++; }
+  void Release() {
     MOZ_ASSERT(refCount_ != 0);
     uint32_t remain = --refCount_;
     if (remain == 0) {
       js_free(this);
     }
   }
 
   size_t dataLength() const {
@@ -1538,22 +1539,21 @@ class SharedScriptData {
   SharedScriptData(const SharedScriptData&) = delete;
   SharedScriptData& operator=(const SharedScriptData&) = delete;
 };
 
 struct ScriptBytecodeHasher {
   class Lookup {
     friend struct ScriptBytecodeHasher;
 
-    SharedScriptData* scriptData;
+    RefPtr<SharedScriptData> scriptData;
     HashNumber hash;
 
    public:
     explicit Lookup(SharedScriptData* data);
-    ~Lookup();
   };
 
   static HashNumber hash(const Lookup& l) { return l.hash; }
   static bool match(SharedScriptData* entry, const Lookup& lookup) {
     const SharedScriptData* data = lookup.scriptData;
     if (entry->natoms() != data->natoms()) {
       return false;
     }
@@ -1593,17 +1593,17 @@ class JSScript : public js::gc::TenuredC
  private:
   // Pointer to baseline->method()->raw(), ion->method()->raw(), a wasm jit
   // entry, the JIT's EnterInterpreter stub, or the lazy link stub. Must be
   // non-null.
   uint8_t* jitCodeRaw_ = nullptr;
   uint8_t* jitCodeSkipArgCheck_ = nullptr;
 
   // Shareable script data
-  js::SharedScriptData* scriptData_ = nullptr;
+  RefPtr<js::SharedScriptData> scriptData_ = {};
 
   // Unshared variable-length data
   js::PrivateScriptData* data_ = nullptr;
 
  public:
   JS::Realm* realm_ = nullptr;
 
  private:
@@ -2532,17 +2532,16 @@ class JSScript : public js::gc::TenuredC
 
  private:
   bool makeTypes(JSContext* cx);
 
   bool createSharedScriptData(JSContext* cx, uint32_t codeLength,
                               uint32_t noteLength, uint32_t natoms);
   bool shareScriptData(JSContext* cx);
   void freeScriptData();
-  void setScriptData(js::SharedScriptData* data);
 
  public:
   uint32_t getWarmUpCount() const { return warmUpCount; }
   uint32_t incWarmUpCounter(uint32_t amount = 1) {
     return warmUpCount += amount;
   }
   uint32_t* addressOfWarmUpCounter() {
     return reinterpret_cast<uint32_t*>(&warmUpCount);