Bug 1229458 - Remove SizeOfIncludingThisMustBeUnshared() from string classes. r=mccr8.
authorNicholas Nethercote <nnethercote@mozilla.com>
Tue, 01 Dec 2015 15:36:26 -0800
changeset 275170 d746378ad672
parent 275169 6df8e622a79d
child 275171 329a4087f6d7
push id29747
push usercbook@mozilla.com
push dateWed, 02 Dec 2015 14:21:19 +0000
treeherdermozilla-central@f6ac392322b3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1229458
milestone45.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 1229458 - Remove SizeOfIncludingThisMustBeUnshared() from string classes. r=mccr8. The patch changes all uses of SizeOfIncludingThisMustBeUnshared() to SizeOfIncludingThisIfUnshared(). This incurs the (tiny) cost of an unnecessary IsReadonly() check for guaranteed-unshared strings, but avoids the possible assertion failures that would occur when MustBeUnshared() was used incorrectly on shared strings, which is an easy mistake to make.
dom/media/webaudio/MediaBufferDecoder.cpp
layout/style/nsCSSValue.cpp
netwerk/dns/nsDNSService2.cpp
netwerk/dns/nsHostResolver.cpp
xpcom/components/nsComponentManager.cpp
xpcom/glue/nsHashKeys.h
xpcom/string/nsStringBuffer.h
xpcom/string/nsSubstring.cpp
xpcom/string/nsTSubstring.cpp
xpcom/string/nsTSubstring.h
--- a/dom/media/webaudio/MediaBufferDecoder.cpp
+++ b/dom/media/webaudio/MediaBufferDecoder.cpp
@@ -606,17 +606,17 @@ WebAudioDecodeJob::OnFailure(ErrorCode a
 
   mContext->RemoveFromDecodeQueue(this);
 }
 
 size_t
 WebAudioDecodeJob::SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const
 {
   size_t amount = 0;
-  amount += mContentType.SizeOfExcludingThisMustBeUnshared(aMallocSizeOf);
+  amount += mContentType.SizeOfExcludingThisIfUnshared(aMallocSizeOf);
   if (mSuccessCallback) {
     amount += mSuccessCallback->SizeOfIncludingThis(aMallocSizeOf);
   }
   if (mFailureCallback) {
     amount += mFailureCallback->SizeOfIncludingThis(aMallocSizeOf);
   }
   if (mOutput) {
     amount += mOutput->SizeOfIncludingThis(aMallocSizeOf);
--- a/layout/style/nsCSSValue.cpp
+++ b/layout/style/nsCSSValue.cpp
@@ -2457,18 +2457,17 @@ css::URLValue::GetURI() const
   return mURI;
 }
 
 size_t
 css::URLValue::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
 {
   size_t n = aMallocSizeOf(this);
 
-  // This string is unshared.
-  n += mString->SizeOfIncludingThisMustBeUnshared(aMallocSizeOf);
+  n += mString->SizeOfIncludingThisIfUnshared(aMallocSizeOf);
 
   // Measurement of the following members may be added later if DMD finds it is
   // worthwhile:
   // - mURI
   // - mReferrer
   // - mOriginPrincipal
 
   return n;
--- a/netwerk/dns/nsDNSService2.cpp
+++ b/netwerk/dns/nsDNSService2.cpp
@@ -1036,17 +1036,17 @@ nsDNSService::SizeOfIncludingThis(mozill
 {
     // Measurement of the following members may be added later if DMD finds it
     // is worthwhile:
     // - mIDN
     // - mLock
 
     size_t n = mallocSizeOf(this);
     n += mResolver->SizeOfIncludingThis(mallocSizeOf);
-    n += mIPv4OnlyDomains.SizeOfExcludingThisMustBeUnshared(mallocSizeOf);
+    n += mIPv4OnlyDomains.SizeOfExcludingThisIfUnshared(mallocSizeOf);
     n += mLocalDomains.SizeOfExcludingThis(mallocSizeOf);
     return n;
 }
 
 MOZ_DEFINE_MALLOC_SIZE_OF(DNSServiceMallocSizeOf)
 
 NS_IMETHODIMP
 nsDNSService::CollectReports(nsIHandleReportCallback* aHandleReport,
--- a/netwerk/dns/nsHostResolver.cpp
+++ b/netwerk/dns/nsHostResolver.cpp
@@ -344,17 +344,17 @@ nsHostRecord::SizeOfIncludingThis(Malloc
     // |mallocSizeOf(this)| call above.
 
     n += SizeOfResolveHostCallbackListExcludingHead(&callbacks, mallocSizeOf);
     n += addr_info ? addr_info->SizeOfIncludingThis(mallocSizeOf) : 0;
     n += mallocSizeOf(addr);
 
     n += mBlacklistedItems.ShallowSizeOfExcludingThis(mallocSizeOf);
     for (size_t i = 0; i < mBlacklistedItems.Length(); i++) {
-        n += mBlacklistedItems[i].SizeOfExcludingThisMustBeUnshared(mallocSizeOf);
+        n += mBlacklistedItems[i].SizeOfExcludingThisIfUnshared(mallocSizeOf);
     }
     return n;
 }
 
 nsHostRecord::DnsPriority
 nsHostRecord::GetPriority(uint16_t aFlags)
 {
     if (IsHighPriority(aFlags)){
--- a/xpcom/components/nsComponentManager.cpp
+++ b/xpcom/components/nsComponentManager.cpp
@@ -1841,17 +1841,17 @@ nsComponentManagerImpl::SizeOfIncludingT
   for (auto iter = mFactories.ConstIter(); !iter.Done(); iter.Next()) {
     n += iter.Data()->SizeOfIncludingThis(aMallocSizeOf);
   }
 
   n += mContractIDs.ShallowSizeOfExcludingThis(aMallocSizeOf);
   for (auto iter = mContractIDs.ConstIter(); !iter.Done(); iter.Next()) {
     // We don't measure the nsFactoryEntry data because it's owned by
     // mFactories (which is measured above).
-    n += iter.Key().SizeOfExcludingThisMustBeUnshared(aMallocSizeOf);
+    n += iter.Key().SizeOfExcludingThisIfUnshared(aMallocSizeOf);
   }
 
   n += sStaticModules->ShallowSizeOfIncludingThis(aMallocSizeOf);
   n += sModuleLocations->ShallowSizeOfIncludingThis(aMallocSizeOf);
 
   n += mKnownStaticModules.ShallowSizeOfExcludingThis(aMallocSizeOf);
   n += mKnownModules.ShallowSizeOfExcludingThis(aMallocSizeOf);
 
--- a/xpcom/glue/nsHashKeys.h
+++ b/xpcom/glue/nsHashKeys.h
@@ -92,17 +92,17 @@ public:
   {
     return mozilla::HashString(*aKey);
   }
 
 #ifdef MOZILLA_INTERNAL_API
   // To avoid double-counting, only measure the string if it is unshared.
   size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
   {
-    return GetKey().SizeOfExcludingThisMustBeUnshared(aMallocSizeOf);
+    return GetKey().SizeOfExcludingThisIfUnshared(aMallocSizeOf);
   }
 #endif
 
   enum { ALLOW_MEMMOVE = true };
 
 private:
   const nsString mStr;
 };
@@ -147,17 +147,17 @@ public:
     ToLowerCase(tmKey);
     return mozilla::HashString(tmKey);
   }
   enum { ALLOW_MEMMOVE = true };
 
   // To avoid double-counting, only measure the string if it is unshared.
   size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
   {
-    return GetKey().SizeOfExcludingThisMustBeUnshared(aMallocSizeOf);
+    return GetKey().SizeOfExcludingThisIfUnshared(aMallocSizeOf);
   }
 
 private:
   const nsString mStr;
 };
 
 #endif
 
@@ -184,17 +184,17 @@ public:
   {
     return mozilla::HashString(*aKey);
   }
 
 #ifdef MOZILLA_INTERNAL_API
   // To avoid double-counting, only measure the string if it is unshared.
   size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
   {
-    return GetKey().SizeOfExcludingThisMustBeUnshared(aMallocSizeOf);
+    return GetKey().SizeOfExcludingThisIfUnshared(aMallocSizeOf);
   }
 #endif
 
   enum { ALLOW_MEMMOVE = true };
 
 private:
   const nsCString mStr;
 };
--- a/xpcom/string/nsStringBuffer.h
+++ b/xpcom/string/nsStringBuffer.h
@@ -136,22 +136,16 @@ public:
    * NOTE: storage size is measured in bytes even for wide strings;
    *       however, string length is always measured in storage units
    *       (2-byte units for wide strings).
    */
   void ToString(uint32_t aLen, nsAString& aStr, bool aMoveOwnership = false);
   void ToString(uint32_t aLen, nsACString& aStr, bool aMoveOwnership = false);
 
   /**
-   * This measures the size.  It should only be used if the StringBuffer is
-   * unshared.  This is checked.
-   */
-  size_t SizeOfIncludingThisMustBeUnshared(mozilla::MallocSizeOf aMallocSizeOf) const;
-
-  /**
    * This measures the size only if the StringBuffer is unshared.
    */
   size_t SizeOfIncludingThisIfUnshared(mozilla::MallocSizeOf aMallocSizeOf) const;
 
   /**
    * This measures the size regardless of whether the StringBuffer is
    * unshared.
    *
--- a/xpcom/string/nsSubstring.cpp
+++ b/xpcom/string/nsSubstring.cpp
@@ -312,30 +312,19 @@ nsStringBuffer::ToString(uint32_t aLen, 
 
   if (!aMoveOwnership) {
     AddRef();
   }
   accessor->set(data, aLen, flags);
 }
 
 size_t
-nsStringBuffer::SizeOfIncludingThisMustBeUnshared(mozilla::MallocSizeOf aMallocSizeOf) const
-{
-  MOZ_ASSERT(!IsReadonly(),
-             "shared StringBuffer in SizeOfIncludingThisMustBeUnshared");
-  return aMallocSizeOf(this);
-}
-
-size_t
 nsStringBuffer::SizeOfIncludingThisIfUnshared(mozilla::MallocSizeOf aMallocSizeOf) const
 {
-  if (!IsReadonly()) {
-    return SizeOfIncludingThisMustBeUnshared(aMallocSizeOf);
-  }
-  return 0;
+  return IsReadonly() ? 0 : aMallocSizeOf(this);
 }
 
 size_t
 nsStringBuffer::SizeOfIncludingThisEvenIfShared(mozilla::MallocSizeOf aMallocSizeOf) const
 {
   return aMallocSizeOf(this);
 }
 
--- a/xpcom/string/nsTSubstring.cpp
+++ b/xpcom/string/nsTSubstring.cpp
@@ -996,22 +996,22 @@ void
 nsTSubstring_CharT::AppendFloat(double aFloat)
 {
   char buf[40];
   int length = FormatWithoutTrailingZeros(buf, aFloat, 15);
   AppendASCII(buf, length);
 }
 
 size_t
-nsTSubstring_CharT::SizeOfExcludingThisMustBeUnshared(
+nsTSubstring_CharT::SizeOfExcludingThisIfUnshared(
     mozilla::MallocSizeOf aMallocSizeOf) const
 {
   if (mFlags & F_SHARED) {
     return nsStringBuffer::FromData(mData)->
-      SizeOfIncludingThisMustBeUnshared(aMallocSizeOf);
+      SizeOfIncludingThisIfUnshared(aMallocSizeOf);
   }
   if (mFlags & F_OWNED) {
     return aMallocSizeOf(mData);
   }
 
   // If we reach here, exactly one of the following must be true:
   // - F_VOIDED is set, and mData points to sEmptyBuffer;
   // - F_FIXED is set, and mData points to a buffer within a string
@@ -1019,56 +1019,32 @@ nsTSubstring_CharT::SizeOfExcludingThisM
   // - None of F_SHARED, F_OWNED, F_FIXED is set, and mData points to a buffer
   //   owned by something else.
   //
   // In all three cases, we don't measure it.
   return 0;
 }
 
 size_t
-nsTSubstring_CharT::SizeOfExcludingThisIfUnshared(
-    mozilla::MallocSizeOf aMallocSizeOf) const
-{
-  // This is identical to SizeOfExcludingThisMustBeUnshared except for the
-  // F_SHARED case.
-  if (mFlags & F_SHARED) {
-    return nsStringBuffer::FromData(mData)->
-           SizeOfIncludingThisIfUnshared(aMallocSizeOf);
-  }
-  if (mFlags & F_OWNED) {
-    return aMallocSizeOf(mData);
-  }
-  return 0;
-}
-
-size_t
 nsTSubstring_CharT::SizeOfExcludingThisEvenIfShared(
     mozilla::MallocSizeOf aMallocSizeOf) const
 {
-  // This is identical to SizeOfExcludingThisMustBeUnshared except for the
+  // This is identical to SizeOfExcludingThisIfUnshared except for the
   // F_SHARED case.
   if (mFlags & F_SHARED) {
     return nsStringBuffer::FromData(mData)->
       SizeOfIncludingThisEvenIfShared(aMallocSizeOf);
   }
   if (mFlags & F_OWNED) {
     return aMallocSizeOf(mData);
   }
   return 0;
 }
 
 size_t
-nsTSubstring_CharT::SizeOfIncludingThisMustBeUnshared(
-    mozilla::MallocSizeOf aMallocSizeOf) const
-{
-  return aMallocSizeOf(this) +
-         SizeOfExcludingThisMustBeUnshared(aMallocSizeOf);
-}
-
-size_t
 nsTSubstring_CharT::SizeOfIncludingThisIfUnshared(
     mozilla::MallocSizeOf aMallocSizeOf) const
 {
   return aMallocSizeOf(this) + SizeOfExcludingThisIfUnshared(aMallocSizeOf);
 }
 
 size_t
 nsTSubstring_CharT::SizeOfIncludingThisEvenIfShared(
--- a/xpcom/string/nsTSubstring.h
+++ b/xpcom/string/nsTSubstring.h
@@ -875,21 +875,16 @@ public:
   nsTSubstring_CharT(char_type* aData, size_type aLength, uint32_t aFlags)
     : mData(aData)
     , mLength(aLength)
     , mFlags(aFlags)
   {
   }
 #endif /* DEBUG || FORCE_BUILD_REFCNT_LOGGING */
 
-  size_t SizeOfExcludingThisMustBeUnshared(mozilla::MallocSizeOf aMallocSizeOf)
-  const;
-  size_t SizeOfIncludingThisMustBeUnshared(mozilla::MallocSizeOf aMallocSizeOf)
-  const;
-
   size_t SizeOfExcludingThisIfUnshared(mozilla::MallocSizeOf aMallocSizeOf)
   const;
   size_t SizeOfIncludingThisIfUnshared(mozilla::MallocSizeOf aMallocSizeOf)
   const;
 
   /**
    * WARNING: Only use these functions if you really know what you are
    * doing, because they can easily lead to double-counting strings.  If