Bug 1470771: Part 2 - Update StringBundle memory reporters to account for sharing. r=erahm
authorKris Maglione <maglione.k@gmail.com>
Sun, 24 Jun 2018 15:47:10 -0700
changeset 424550 34f927a06700696cb78789e9e77770054556212c
parent 424549 967409251f5cfdb7acc7616f6da2774677854aeb
child 424566 4af2753129c7eab71a64295f8c9f685d0b5423bf
push id104847
push usermaglione.k@gmail.com
push dateSat, 30 Jun 2018 21:30:16 +0000
treeherdermozilla-inbound@34f927a06700 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerserahm
bugs1470771
milestone63.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 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
@@ -557,45 +557,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.
@@ -686,17 +657,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;
+
+  explicit 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,109 @@ 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/");
+  {
+    RefPtr<SharedStringBundle> shared = do_QueryObject(this);
+    if (shared) {
+      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(uint32_t(mRefCnt - 1));
+
+  if (sharedSize) {
+    path.AppendLiteral(", sharedMemorySize=");
+    path.AppendInt(uint32_t(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 +792,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,
@@ -783,17 +876,17 @@ nsStringBundleService::RegisterContentBu
     }
 
     proxy = do_QueryObject(cacheEntry->mBundle);
     MOZ_ASSERT(proxy);
     cacheEntry->remove();
     delete cacheEntry;
   }
 
-  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);
@@ -812,17 +905,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.
     //
@@ -830,17 +923,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;
+
+    explicit 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,