Bug 1470771: Part 2 - Update StringBundle memory reporters to account for sharing. r?erahm draft
authorKris Maglione <maglione.k@gmail.com>
Sun, 24 Jun 2018 15:47:10 -0700
changeset 812729 30e2e195018e1a17acaabc318327ccc798b934fd
parent 812728 dd719a2c7210358995c14fee72dd8e1291d8e5b3
push id114661
push usermaglione.k@gmail.com
push dateFri, 29 Jun 2018 22:31:07 +0000
reviewerserahm
bugs1470771
milestone62.0a1
Bug 1470771: Part 2 - Update StringBundle memory reporters to account for sharing. r?erahm MozReview-Commit-ID: 5xRsvLj4Klu
dom/base/nsContentUtils.cpp
intl/strres/nsStringBundle.cpp
intl/strres/nsStringBundle.h
intl/strres/nsStringBundleService.h
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -519,45 +519,16 @@ class SameOriginCheckerImpl final : publ
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSICHANNELEVENTSINK
   NS_DECL_NSIINTERFACEREQUESTOR
 };
 
 } // namespace
 
-class nsContentUtils::nsContentUtilsReporter final : public nsIMemoryReporter
-{
-  MOZ_DEFINE_MALLOC_SIZE_OF(MallocSizeOf);
-
-  ~nsContentUtilsReporter() = default;
-
-public:
-  NS_DECL_ISUPPORTS
-
-  NS_IMETHOD
-  CollectReports(nsIHandleReportCallback* aHandleReport,
-                 nsISupports* aData, bool aAnonymize) override
-  {
-    int64_t amt = 0;
-    for (int32_t i = 0; i < PropertiesFile_COUNT; ++i) {
-      if (sStringBundles[i]) {
-        amt += sStringBundles[i]->SizeOfIncludingThisIfUnshared(MallocSizeOf);
-      }
-    }
-
-    MOZ_COLLECT_REPORT("explicit/dom/content-utils-string-bundles", KIND_HEAP, UNITS_BYTES,
-                       amt, "string-bundles in ContentUtils");
-
-    return NS_OK;
-  }
-};
-
-NS_IMPL_ISUPPORTS(nsContentUtils::nsContentUtilsReporter, nsIMemoryReporter)
-
 /**
  * This class is used to determine whether or not the user is currently
  * interacting with the browser. It listens to observer events to toggle the
  * value of the sUserActive static.
  *
  * This class is an internal implementation detail.
  * nsContentUtils::GetUserIsInteracting() should be used to access current
  * user interaction status.
@@ -648,17 +619,16 @@ nsContentUtils::Init()
       EventListenerManagerHashClearEntry,
       EventListenerManagerHashInitEntry
     };
 
     sEventListenerManagersHash =
       new PLDHashTable(&hash_table_ops, sizeof(EventListenerManagerMapEntry));
 
     RegisterStrongMemoryReporter(new DOMEventListenerManagersHashReporter());
-    RegisterStrongMemoryReporter(new nsContentUtilsReporter());
   }
 
   sBlockedScriptRunners = new AutoTArray<nsCOMPtr<nsIRunnable>, 8>;
 
   Preferences::AddBoolVarCache(&sAllowXULXBL_for_file,
                                "dom.allow_XUL_XBL_for_file");
 
   Preferences::AddBoolVarCache(&sIsFullScreenApiEnabled,
--- a/intl/strres/nsStringBundle.cpp
+++ b/intl/strres/nsStringBundle.cpp
@@ -129,26 +129,22 @@ class StringBundleProxy : public nsIStri
   void Retarget(nsIStringBundle* aTarget)
   {
     ReentrantMonitorAutoEnter automon(mReentrantMonitor);
     mTarget = aTarget;
   }
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const override
   {
-    return mTarget->SizeOfIncludingThis(aMallocSizeOf) + aMallocSizeOf(this);
+    return aMallocSizeOf(this);
   }
 
   size_t SizeOfIncludingThisIfUnshared(mozilla::MallocSizeOf aMallocSizeOf) const override
   {
-    size_t size = mTarget->SizeOfIncludingThisIfUnshared(aMallocSizeOf);
-    if (mRefCnt == 1) {
-      size += aMallocSizeOf(this);
-    }
-    return size;
+    return mRefCnt == 1 ? SizeOfIncludingThis(aMallocSizeOf) : 0;
   }
 
 protected:
   virtual ~StringBundleProxy() = default;
 
 private:
   ReentrantMonitor mReentrantMonitor;
   nsCOMPtr<nsIStringBundle> mTarget;
@@ -178,20 +174,16 @@ NS_IMPL_ISUPPORTS(StringBundleProxy, nsI
  *
  * Important: The memory allocated by these string bundles will never be freed
  * before process shutdown, per the restrictions in SharedStringMap.h, so they
  * should never be used for short-lived bundles.
  */
 class SharedStringBundle final : public nsStringBundleBase
 {
 public:
-  SharedStringBundle(const char* aURLSpec)
-    : nsStringBundleBase(aURLSpec)
-  {}
-
   /**
    * Initialize the string bundle with a file descriptor pointing to a
    * pre-populated key-value store for this string bundle. This should only be
    * called in child processes, for bundles initially created in the parent
    * process.
    */
   void SetMapFile(const FileDescriptor& aFile, size_t aSize);
 
@@ -239,16 +231,22 @@ public:
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const override;
 
   static SharedStringBundle* Cast(nsIStringBundle* aStringBundle)
   {
     return static_cast<SharedStringBundle*>(aStringBundle);
   }
 
 protected:
+  friend class nsStringBundleBase;
+
+  SharedStringBundle(const char* aURLSpec)
+    : nsStringBundleBase(aURLSpec)
+  {}
+
   ~SharedStringBundle() override = default;
 
   nsresult GetStringImpl(const nsACString& aName, nsAString& aResult) override;
 
   nsresult GetSimpleEnumerationImpl(nsISimpleEnumerator** elements) override;
 
 private:
   RefPtr<SharedStringMap> mStringMap;
@@ -275,33 +273,64 @@ protected:
 private:
   RefPtr<SharedStringMap> mStringMap;
 
   uint32_t mIndex = 0;
 };
 
 NS_IMPL_ISUPPORTS(StringMapEnumerator, nsISimpleEnumerator)
 
+template <typename T, typename... Args>
+already_AddRefed<T>
+MakeBundle(Args... args)
+{
+  return nsStringBundleBase::Create<T>(args...);
+}
+
+template <typename T, typename... Args>
+RefPtr<T>
+MakeBundleRefPtr(Args... args)
+{
+  return nsStringBundleBase::Create<T>(args...);
+}
+
 } // anonymous namespace
 
-NS_IMPL_ISUPPORTS(nsStringBundleBase, nsIStringBundle)
+NS_IMPL_ISUPPORTS(nsStringBundleBase, nsIStringBundle, nsIMemoryReporter)
 
 NS_IMPL_ISUPPORTS_INHERITED0(nsStringBundle, nsStringBundleBase)
 NS_IMPL_ISUPPORTS_INHERITED(SharedStringBundle, nsStringBundleBase, SharedStringBundle)
 
 nsStringBundleBase::nsStringBundleBase(const char* aURLSpec) :
   mPropertiesURL(aURLSpec),
   mReentrantMonitor("nsStringBundle.mReentrantMonitor"),
   mAttemptedLoad(false),
   mLoaded(false)
 {
 }
 
 nsStringBundleBase::~nsStringBundleBase()
-{}
+{
+  UnregisterWeakMemoryReporter(this);
+}
+
+void
+nsStringBundleBase::RegisterMemoryReporter()
+{
+  RegisterWeakMemoryReporter(this);
+}
+
+template <typename T, typename... Args>
+/* static */ already_AddRefed<T>
+nsStringBundleBase::Create(Args... args)
+{
+  RefPtr<T> bundle = new T(args...);
+  bundle->RegisterMemoryReporter();
+  return bundle.forget();
+}
 
 nsStringBundle::nsStringBundle(const char* aURLSpec)
   : nsStringBundleBase(aURLSpec)
 {}
 
 nsStringBundle::~nsStringBundle()
 {
 }
@@ -311,45 +340,106 @@ nsStringBundleBase::AsyncPreload()
 {
   return NS_IdleDispatchToCurrentThread(
     NewIdleRunnableMethod("nsStringBundleBase::LoadProperties",
                           this,
                           &nsStringBundleBase::LoadProperties));
 }
 
 size_t
-nsStringBundle::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const
+nsStringBundle::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
 {
   size_t n = 0;
   if (mProps) {
     n += mProps->SizeOfIncludingThis(aMallocSizeOf);
   }
   return aMallocSizeOf(this) + n;
 }
 
 size_t
-nsStringBundleBase::SizeOfIncludingThisIfUnshared(MallocSizeOf aMallocSizeOf) const
+nsStringBundleBase::SizeOfIncludingThisIfUnshared(mozilla::MallocSizeOf aMallocSizeOf) const
 {
   if (mRefCnt == 1) {
     return SizeOfIncludingThis(aMallocSizeOf);
   } else {
     return 0;
   }
 }
 
 size_t
-SharedStringBundle::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const
+SharedStringBundle::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
 {
   size_t n = 0;
   if (mStringMap) {
     n += aMallocSizeOf(mStringMap);
   }
   return aMallocSizeOf(this) + n;
 }
 
+NS_IMETHODIMP
+nsStringBundleBase::CollectReports(nsIHandleReportCallback* aHandleReport,
+                                   nsISupports* aData,
+                                   bool aAnonymize)
+{
+  // String bundle URLs are always local, and part of the distribution.
+  // There's no need to anonymize.
+  nsAutoCStringN<64> escapedURL(mPropertiesURL);
+  escapedURL.ReplaceChar('/', '\\');
+
+  size_t sharedSize = 0;
+  size_t heapSize = SizeOfIncludingThis(MallocSizeOf);
+
+  nsAutoCStringN<256> path("explicit/string-bundles/");
+  if (RefPtr<SharedStringBundle> shared = do_QueryObject(this)) {
+    path.AppendLiteral("SharedStringBundle");
+    if (XRE_IsParentProcess()) {
+      sharedSize = shared->MapSize();
+    }
+  } else {
+    path.AppendLiteral("nsStringBundle");
+  }
+
+  path.AppendLiteral("(url=\"");
+  path.Append(escapedURL);
+
+  // Note: The memory reporter service holds a strong reference to reporters
+  // while collecting reports, so we want to ignore the extra ref in reports.
+  path.AppendLiteral("\", shared=");
+  path.AppendASCII(mRefCnt > 2 ? "true" : "false");
+  path.AppendLiteral(", refCount=");
+  path.AppendInt(mRefCnt - 1);
+
+  if (sharedSize) {
+    path.AppendLiteral(", sharedMemorySize=");
+    path.AppendInt(sharedSize);
+  }
+
+  path.AppendLiteral(")");
+
+  NS_NAMED_LITERAL_CSTRING(
+      desc,
+      "A StringBundle instance representing the data in a (probably "
+      "localized) .properties file. Data may be shared between "
+      "processes.");
+
+  aHandleReport->Callback(
+    EmptyCString(), path, KIND_HEAP, UNITS_BYTES,
+    heapSize, desc, aData);
+
+  if (sharedSize) {
+    path.ReplaceLiteral(0, sizeof("explicit/") - 1,
+                        "shared-");
+
+    aHandleReport->Callback(
+      EmptyCString(), path, KIND_OTHER, UNITS_BYTES,
+      sharedSize, desc, aData);
+  }
+
+  return NS_OK;
+}
 
 nsresult
 nsStringBundleBase::ParseProperties(nsIPersistentProperties** aProps)
 {
   // this is different than mLoaded, because we only want to attempt
   // to load once
   // we only want to load once, but if we've tried once and failed,
   // continue to throw an error!
@@ -699,17 +789,17 @@ nsStringBundleService::Init()
   return NS_OK;
 }
 
 size_t
 nsStringBundleService::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
 {
   size_t n = mBundleMap.ShallowSizeOfExcludingThis(aMallocSizeOf);
   for (auto iter = mBundleMap.ConstIter(); !iter.Done(); iter.Next()) {
-    n += iter.Data()->mBundle->SizeOfIncludingThis(aMallocSizeOf);
+    n += aMallocSizeOf(iter.Data());
     n += iter.Data()->mHashKey.SizeOfExcludingThisIfUnshared(aMallocSizeOf);
   }
   return aMallocSizeOf(this) + n;
 }
 
 NS_IMETHODIMP
 nsStringBundleService::Observe(nsISupports* aSubject,
                                const char* aTopic,
@@ -782,17 +872,17 @@ nsStringBundleService::RegisterContentBu
       return;
     }
 
     proxy = do_QueryObject(cacheEntry->mBundle);
     MOZ_ASSERT(proxy);
     cacheEntry->remove();
   }
 
-  auto bundle = MakeRefPtr<SharedStringBundle>(aBundleURL.get());
+  auto bundle = MakeBundleRefPtr<SharedStringBundle>(aBundleURL.get());
   bundle->SetMapFile(aMapFile, aMapSize);
 
   if (proxy) {
     proxy->Retarget(bundle);
   }
 
   cacheEntry = insertIntoCache(bundle.forget(), aBundleURL);
   mSharedBundles.insertBack(cacheEntry);
@@ -811,17 +901,17 @@ nsStringBundleService::getStringBundle(c
     // Remove the entry from the list so it can be re-inserted at the back.
     cacheEntry->remove();
 
     shared = do_QueryObject(cacheEntry->mBundle);
   } else {
     nsCOMPtr<nsIStringBundle> bundle;
     bool isContent = IsContentBundle(key);
     if (!isContent || !XRE_IsParentProcess()) {
-      bundle = new nsStringBundle(aURLSpec);
+      bundle = MakeBundle<nsStringBundle>(aURLSpec);
     }
 
     // If this is a bundle which is used by the content processes, we want to
     // load it into a shared memory region.
     //
     // If we're in the parent process, just create a new SharedStringBundle,
     // and populate it from the properties file.
     //
@@ -829,17 +919,17 @@ nsStringBundleService::getStringBundle(c
     // the cache means that we haven't received its shared memory descriptor
     // from the parent yet. There's not much we can do about that besides
     // wait, but we need to return a bundle now. So instead of a shared memory
     // bundle, we create a temporary proxy, which points to a non-shared
     // bundle initially, and is retarged to a shared memory bundle when it
     // becomes available.
     if (isContent) {
       if (XRE_IsParentProcess()) {
-        shared = new SharedStringBundle(aURLSpec);
+        shared = MakeBundle<SharedStringBundle>(aURLSpec);
         bundle = shared;
       } else {
         bundle = new StringBundleProxy(bundle.forget());
       }
     }
 
     cacheEntry = insertIntoCache(bundle.forget(), key);
   }
--- a/intl/strres/nsStringBundle.h
+++ b/intl/strres/nsStringBundle.h
@@ -3,79 +3,92 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef nsStringBundle_h__
 #define nsStringBundle_h__
 
 #include "mozilla/ReentrantMonitor.h"
 #include "nsIStringBundle.h"
+#include "nsIMemoryReporter.h"
 #include "nsCOMPtr.h"
 #include "nsString.h"
 #include "nsCOMArray.h"
 
 class nsIPersistentProperties;
 
+
 class nsStringBundleBase : public nsIStringBundle
+                         , public nsIMemoryReporter
 {
 public:
-    nsStringBundleBase(const char* aURLSpec);
+    MOZ_DEFINE_MALLOC_SIZE_OF(MallocSizeOf)
 
     nsresult ParseProperties(nsIPersistentProperties**);
 
     NS_DECL_THREADSAFE_ISUPPORTS
     NS_DECL_NSISTRINGBUNDLE
+    NS_DECL_NSIMEMORYREPORTER
 
     virtual nsresult LoadProperties() = 0;
 
     const nsCString& BundleURL() const { return mPropertiesURL; }
 
     // Returns true if this bundle has more than one reference. If it has only
     // a single reference, it is assumed to be held alive by the bundle cache.
     bool IsShared() const { return mRefCnt > 1; }
 
     static nsStringBundleBase* Cast(nsIStringBundle* aBundle)
     {
       return static_cast<nsStringBundleBase*>(aBundle);
     }
 
+    template <typename T, typename... Args>
+    static already_AddRefed<T> Create(Args... args);
+
 protected:
+    nsStringBundleBase(const char* aURLSpec);
+
     virtual ~nsStringBundleBase();
 
     virtual nsresult GetStringImpl(const nsACString& aName, nsAString& aResult) = 0;
 
     virtual nsresult GetSimpleEnumerationImpl(nsISimpleEnumerator** elements) = 0;
 
+    void RegisterMemoryReporter();
+
     nsCString              mPropertiesURL;
     mozilla::ReentrantMonitor    mReentrantMonitor;
     bool                         mAttemptedLoad;
     bool                         mLoaded;
 
     size_t SizeOfIncludingThisIfUnshared(mozilla::MallocSizeOf aMallocSizeOf) const override;
 
 public:
     static nsresult FormatString(const char16_t *formatStr,
                                  const char16_t **aParams, uint32_t aLength,
                                  nsAString& aResult);
 };
 
 class nsStringBundle : public nsStringBundleBase
 {
 public:
-    nsStringBundle(const char* aURLSpec);
-
     NS_DECL_ISUPPORTS_INHERITED
 
     nsCOMPtr<nsIPersistentProperties> mProps;
 
     nsresult LoadProperties() override;
 
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const override;
 
 protected:
+    friend class nsStringBundleBase;
+
+    nsStringBundle(const char* aURLSpec);
+
     virtual ~nsStringBundle();
 
     nsresult GetStringImpl(const nsACString& aName, nsAString& aResult) override;
 
     nsresult GetSimpleEnumerationImpl(nsISimpleEnumerator** elements) override;
 };
 
 #endif
--- a/intl/strres/nsStringBundleService.h
+++ b/intl/strres/nsStringBundleService.h
@@ -38,19 +38,19 @@ public:
   NS_DECL_NSIOBSERVER
 
   NS_IMETHOD CollectReports(nsIHandleReportCallback* aHandleReport,
                             nsISupports* aData, bool anonymize) override
   {
     size_t amt = SizeOfIncludingThis(MallocSizeOf);
 
     MOZ_COLLECT_REPORT(
-      "explicit/string-bundle-service", KIND_HEAP, UNITS_BYTES,
+      "explicit/string-bundles/service", KIND_HEAP, UNITS_BYTES,
       amt,
-      "Memory used for StringBundleService bundles");
+      "Memory used for StringBundleService overhead");
     return NS_OK;
   };
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const override;
 
   void SendContentBundles(mozilla::dom::ContentParent* aContentParent) const override;
 
   void RegisterContentBundle(const nsCString& aBundleURL,