Bug 1536154 - Update JS_updateMallocCounter callers in xpconnect to use the new APIs r=mccr8
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 24 Apr 2019 15:58:42 +0100
changeset 530898 4cf9348fa63c5c1b226578fd888d341f7a07b10a
parent 530897 5159ad4a890bf2e2fd94972d798ac9e22f929168
child 530899 a1cf373a4e9fe74741442c2f8d747c080b80175a
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1536154
milestone68.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 1536154 - Update JS_updateMallocCounter callers in xpconnect to use the new APIs r=mccr8 Differential Revision: https://phabricator.services.mozilla.com/D28693
js/src/vm/JSScript.cpp
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/src/XPCWrappedNative.cpp
js/xpconnect/src/xpcprivate.h
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -1679,18 +1679,26 @@ class ScriptSource::LoadSourceMatcher {
     MOZ_ASSERT(ss_->sourceRetrievable(),
                "should be retrievable if Retrievable");
 
     if (!cx_->runtime()->sourceHook.ref()) {
       *loaded_ = false;
       return true;
     }
 
-    // The argument is just for overloading -- its value doesn't matter.
-    return tryLoadAndSetSource(Unit('0'));
+    size_t length;
+
+    // The first argument is just for overloading -- its value doesn't matter.
+    if (!tryLoadAndSetSource(Unit('0'), &length)) {
+      return false;
+    }
+
+    cx_->updateMallocCounter(length * sizeof(Unit));
+
+    return true;
   }
 
   bool operator()(const Missing&) const {
     MOZ_ASSERT(!ss_->sourceRetrievable(),
                "should have Retrievable<Unit> source, not Missing source, if "
                "retrievable");
     *loaded_ = false;
     return true;
@@ -1698,55 +1706,53 @@ class ScriptSource::LoadSourceMatcher {
 
   bool operator()(const BinAST&) const {
     MOZ_ASSERT(!ss_->sourceRetrievable(), "binast source is never retrievable");
     *loaded_ = false;
     return true;
   }
 
  private:
-  bool tryLoadAndSetSource(const Utf8Unit&) const {
+  bool tryLoadAndSetSource(const Utf8Unit&, size_t* length) const {
     char* utf8Source;
-    size_t length;
     if (!cx_->runtime()->sourceHook->load(cx_, ss_->filename(), nullptr,
-                                          &utf8Source, &length)) {
+                                          &utf8Source, length)) {
       return false;
     }
 
     if (!utf8Source) {
       *loaded_ = false;
       return true;
     }
 
     if (!ss_->setRetrievedSource(
              cx_,
              EntryUnits<Utf8Unit>(reinterpret_cast<Utf8Unit*>(utf8Source)),
-             length)) {
+             *length)) {
       return false;
     }
 
     *loaded_ = true;
     return true;
   }
 
-  bool tryLoadAndSetSource(const char16_t&) const {
+  bool tryLoadAndSetSource(const char16_t&, size_t* length) const {
     char16_t* utf16Source;
-    size_t length;
     if (!cx_->runtime()->sourceHook->load(cx_, ss_->filename(), &utf16Source,
-                                          nullptr, &length)) {
+                                          nullptr, length)) {
       return false;
     }
 
     if (!utf16Source) {
       *loaded_ = false;
       return true;
     }
 
     if (!ss_->setRetrievedSource(cx_, EntryUnits<char16_t>(utf16Source),
-                                 length)) {
+                                 *length)) {
       return false;
     }
 
     *loaded_ = true;
     return true;
   }
 };
 
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -2898,21 +2898,16 @@ static nsresult ReadSourceFromFilename(J
 
     if (!*twoByteSource) {
       return NS_ERROR_FAILURE;
     }
 
     bytesAllocated = *len * sizeof(char16_t);
   }
 
-  // Historically this method used JS_malloc() which updates the GC memory
-  // accounting.  Since ConvertToUTF16() and js::MakeUnique now use js_malloc()
-  // instead we update the accounting manually after the fact.
-  JS_updateMallocCounter(cx, bytesAllocated);
-
   return NS_OK;
 }
 
 // The JS engine calls this object's 'load' member function when it needs
 // the source for a chrome JS function. See the comment in the XPCJSRuntime
 // constructor.
 class XPCJSSourceHook : public js::SourceHook {
   bool load(JSContext* cx, const char* filename, char16_t** twoByteSource,
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -217,18 +217,17 @@ nsresult XPCWrappedNative::WrapNewGlobal
   //
   // We don't call ::Init() on this wrapper, because our setup requirements
   // are different for globals. We do our setup inline here, instead.
   //
 
   wrapper->mScriptable = scrWrapper;
 
   // Set the JS object to the global we already created.
-  wrapper->mFlatJSObject = global;
-  wrapper->mFlatJSObject.setFlags(FLAT_JS_OBJECT_VALID);
+  wrapper->SetFlatJSObject(global);
 
   // Set the private to the XPCWrappedNative.
   JS_SetPrivate(global, wrapper);
 
   // There are dire comments elsewhere in the code about how a GC can
   // happen somewhere after wrapper initialization but before the wrapper is
   // added to the hashtable in FinishCreate(). It's not clear if that can
   // happen here, but let's just be safe for now.
@@ -538,16 +537,42 @@ void XPCWrappedNative::Destroy() {
     } else {
       mIdentity = nullptr;
     }
   }
 
   mMaybeScope = nullptr;
 }
 
+// A hack for bug 517665, increase the probability for GC.
+// TODO: Try removing this and just using the actual size of the object.
+static const size_t GCMemoryFactor = 2;
+
+inline void XPCWrappedNative::SetFlatJSObject(JSObject* object) {
+  MOZ_ASSERT(!mFlatJSObject);
+  MOZ_ASSERT(object);
+
+  JS::AddAssociatedMemory(object, sizeof(*this) * GCMemoryFactor,
+                          JS::MemoryUse::XPCWrappedNative);
+
+  mFlatJSObject = object;
+  mFlatJSObject.setFlags(FLAT_JS_OBJECT_VALID);
+}
+
+inline void XPCWrappedNative::UnsetFlatJSObject() {
+  MOZ_ASSERT(mFlatJSObject);
+
+  JS::RemoveAssociatedMemory(mFlatJSObject.unbarrieredGetPtr(),
+                             sizeof(*this) * GCMemoryFactor,
+                             JS::MemoryUse::XPCWrappedNative);
+
+  mFlatJSObject = nullptr;
+  mFlatJSObject.unsetFlags(FLAT_JS_OBJECT_VALID);
+}
+
 // This is factored out so that it can be called publicly.
 // static
 nsIXPCScriptable* XPCWrappedNative::GatherProtoScriptable(
     nsIClassInfo* classInfo) {
   MOZ_ASSERT(classInfo, "bad param");
 
   nsCOMPtr<nsIXPCScriptable> helper;
   nsresult rv = classInfo->GetScriptableHelper(getter_AddRefs(helper));
@@ -621,38 +646,35 @@ bool XPCWrappedNative::Init(JSContext* c
              "bad class");
 
   RootedObject protoJSObject(cx, HasProto() ? GetProto()->GetJSProtoObject()
                                             : JS::GetRealmObjectPrototype(cx));
   if (!protoJSObject) {
     return false;
   }
 
-  mFlatJSObject = JS_NewObjectWithGivenProto(cx, jsclazz, protoJSObject);
-  if (!mFlatJSObject) {
-    mFlatJSObject.unsetFlags(FLAT_JS_OBJECT_VALID);
+  JSObject* object = JS_NewObjectWithGivenProto(cx, jsclazz, protoJSObject);
+  if (!object) {
     return false;
   }
 
-  mFlatJSObject.setFlags(FLAT_JS_OBJECT_VALID);
+  SetFlatJSObject(object);
+
   JS_SetPrivate(mFlatJSObject, this);
 
   return FinishInit(cx);
 }
 
 bool XPCWrappedNative::FinishInit(JSContext* cx) {
   // This reference will be released when mFlatJSObject is finalized.
   // Since this reference will push the refcount to 2 it will also root
   // mFlatJSObject;
   MOZ_ASSERT(1 == mRefCnt, "unexpected refcount value");
   NS_ADDREF(this);
 
-  // A hack for bug 517665, increase the probability for GC.
-  JS_updateMallocCounter(cx, 2 * sizeof(XPCWrappedNative));
-
   return true;
 }
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(XPCWrappedNative)
   NS_INTERFACE_MAP_ENTRY(nsIXPConnectWrappedNative)
   NS_INTERFACE_MAP_ENTRY(nsIXPConnectJSObjectHolder)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIXPConnectWrappedNative)
 NS_INTERFACE_MAP_END
@@ -751,18 +773,17 @@ void XPCWrappedNative::FlatJSObjectFinal
   }
 
   nsWrapperCache* cache = nullptr;
   CallQueryInterface(mIdentity, &cache);
   if (cache) {
     cache->ClearWrapper(mFlatJSObject.unbarrieredGetPtr());
   }
 
-  mFlatJSObject = nullptr;
-  mFlatJSObject.unsetFlags(FLAT_JS_OBJECT_VALID);
+  UnsetFlatJSObject();
 
   MOZ_ASSERT(mIdentity, "bad pointer!");
 #ifdef XP_WIN
   // Try to detect free'd pointer
   MOZ_ASSERT(*(int*)mIdentity.get() != (int)0xdddddddd, "bad pointer!");
   MOZ_ASSERT(*(int*)mIdentity.get() != (int)0, "bad pointer!");
 #endif
 
@@ -797,18 +818,17 @@ void XPCWrappedNative::SystemIsBeingShut
   // The long standing strategy is to leak some objects still held at shutdown.
   // The general problem is that propagating release out of xpconnect at
   // shutdown time causes a world of problems.
 
   // We leak mIdentity (see above).
 
   // Short circuit future finalization.
   JS_SetPrivate(mFlatJSObject, nullptr);
-  mFlatJSObject = nullptr;
-  mFlatJSObject.unsetFlags(FLAT_JS_OBJECT_VALID);
+  UnsetFlatJSObject();
 
   XPCWrappedNativeProto* proto = GetProto();
 
   if (HasProto()) {
     proto->SystemIsBeingShutDown();
   }
 
   // We don't clear mScriptable here. The destructor will do it.
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -1431,16 +1431,19 @@ class XPCWrappedNative final : public ns
   void SetSet(already_AddRefed<XPCNativeSet> set) { mSet = set; }
 
   static XPCWrappedNative* Get(JSObject* obj) {
     MOZ_ASSERT(IS_WN_REFLECTOR(obj));
     return (XPCWrappedNative*)js::GetObjectPrivate(obj);
   }
 
  private:
+  void SetFlatJSObject(JSObject* object);
+  void UnsetFlatJSObject();
+
   inline void ExpireWrapper() {
     mMaybeScope = (XPCWrappedNativeScope*)(XPC_SCOPE_WORD(mMaybeScope) |
                                            XPC_WRAPPER_EXPIRED);
   }
 
  public:
   nsIXPCScriptable* GetScriptable() const { return mScriptable; }