Bug 1097507 - Make libxul independent of libdmd when DMD is enabled. r=njn

This also effectively changes how DMD is enabled from requiring both
replace-malloc initialization and the DMD environment variable to
requiring only the former. The DMD environment variable can still be
used to specify options, but not to disable entirely.

This however doesn't touch all the parts that do enable DMD by setting
the DMD environment variable to 1, so the code to handle this value
is kept.

memory/build/replace_malloc_bridge.h

@@ -58,9 +58,18 @@ struct ReplaceMallocBridge;
 extern "C" MFBT_API ReplaceMallocBridge* get_bridge();
 #endif
 
+namespace mozilla {
+namespace dmd {
+struct DMDFuncs;
+}
+}
+
 struct ReplaceMallocBridge
 {
-  ReplaceMallocBridge() : mVersion(0) {}
+  ReplaceMallocBridge() : mVersion(1) {}
+
+  /* This method was added in version 1 of the bridge. */
+  virtual mozilla::dmd::DMDFuncs* GetDMDFuncs() { return nullptr; }
 
 #ifndef REPLACE_MALLOC_IMPL
   /* Returns the replace-malloc bridge if its version is at least the
@@ -85,6 +94,13 @@ protected:
  * names to be identical. */
 struct ReplaceMalloc
 {
+  /* Don't call this method from performance critical code. Use
+   * mozilla::dmd::DMDFuncs::Get() instead, it has less overhead. */
+  static mozilla::dmd::DMDFuncs* GetDMDFuncs()
+  {
+    auto singleton = ReplaceMallocBridge::Get(/* minimumVersion */ 1);
+    return singleton ? singleton->GetDMDFuncs() : nullptr;
+  }
 };
 #endif
 

memory/replace/dmd/DMD.cpp

@@ -4,8 +4,6 @@
  * 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/. */
 
-#include "DMD.h"
-
 #include <ctype.h>
 #include <errno.h>
 #include <limits.h>
@@ -45,6 +43,8 @@
 // DMD depend on XPCOM's object file.
 #include "CodeAddressService.h"
 
+// replace_malloc.h needs to be included before replace_malloc_bridge.h,
+// which DMD.h includes, so DMD.h needs to be included after replace_malloc.h.
 // MOZ_REPLACE_ONLY_MEMALIGN saves us from having to define
 // replace_{posix_memalign,aligned_alloc,valloc}.  It requires defining
 // PAGE_SIZE.  Nb: sysconf() is expensive, but it's only used for (the obsolete
@@ -66,9 +66,34 @@ static long GetPageSize()
 #undef MOZ_REPLACE_ONLY_MEMALIGN
 #undef PAGE_SIZE
 
+#include "DMD.h"
+
 namespace mozilla {
 namespace dmd {
 
+class DMDBridge : public ReplaceMallocBridge
+{
+  virtual DMDFuncs* GetDMDFuncs() MOZ_OVERRIDE;
+};
+
+static DMDBridge sDMDBridge;
+static DMDFuncs sDMDFuncs;
+
+DMDFuncs*
+DMDBridge::GetDMDFuncs()
+{
+  return &sDMDFuncs;
+}
+
+inline void
+StatusMsg(const char* aFmt, ...)
+{
+  va_list ap;
+  va_start(ap, aFmt);
+  sDMDFuncs.StatusMsg(aFmt, ap);
+  va_end(ap);
+}
+
 //---------------------------------------------------------------------------
 // Utilities
 //---------------------------------------------------------------------------
@@ -81,8 +106,8 @@ namespace dmd {
 
 static const malloc_table_t* gMallocTable = nullptr;
 
-// This enables/disables DMD.
-static bool gIsDMDRunning = false;
+// Whether DMD finished initializing.
+static bool gIsDMDInitialized = false;
 
 // This provides infallible allocations (they abort on OOM).  We use it for all
 // of DMD's own allocations, which fall into the following three cases.
@@ -197,26 +222,23 @@ MallocSizeOf(const void* aPtr)
   return gMallocTable->malloc_usable_size(const_cast<void*>(aPtr));
 }
 
-MOZ_EXPORT void
-StatusMsg(const char* aFmt, ...)
+void
+DMDFuncs::StatusMsg(const char* aFmt, va_list aAp)
 {
-  va_list ap;
-  va_start(ap, aFmt);
 #ifdef ANDROID
 #ifdef MOZ_B2G_LOADER
   // Don't call __android_log_vprint() during initialization, or the magic file
   // descriptors will be occupied by android logcat.
-  if (gIsDMDRunning)
+  if (gIsDMDInitialized)
 #endif
-    __android_log_vprint(ANDROID_LOG_INFO, "DMD", aFmt, ap);
+    __android_log_vprint(ANDROID_LOG_INFO, "DMD", aFmt, aAp);
 #else
   // The +64 is easily enough for the "DMD[<pid>] " prefix and the NUL.
   char* fmt = (char*) InfallibleAllocPolicy::malloc_(strlen(aFmt) + 64);
   sprintf(fmt, "DMD[%d] %s", getpid(), aFmt);
-  vfprintf(stderr, fmt, ap);
+  vfprintf(stderr, fmt, aAp);
   InfallibleAllocPolicy::free_(fmt);
 #endif
-  va_end(ap);
 }
 
 /* static */ void
@@ -954,8 +976,6 @@ static size_t gSmallBlockActualSizeCounter = 0;
 static void
 AllocCallback(void* aPtr, size_t aReqSize, Thread* aT)
 {
-  MOZ_ASSERT(gIsDMDRunning);
-
   if (!aPtr) {
     return;
   }
@@ -988,8 +1008,6 @@ AllocCallback(void* aPtr, size_t aReqSize, Thread* aT)
 static void
 FreeCallback(void* aPtr, Thread* aT)
 {
-  MOZ_ASSERT(gIsDMDRunning);
-
   if (!aPtr) {
     return;
   }
@@ -1019,12 +1037,18 @@ replace_init(const malloc_table_t* aMallocTable)
   mozilla::dmd::Init(aMallocTable);
 }
 
+ReplaceMallocBridge*
+replace_get_bridge()
+{
+  return &mozilla::dmd::sDMDBridge;
+}
+
 void*
 replace_malloc(size_t aSize)
 {
   using namespace mozilla::dmd;
 
-  if (!gIsDMDRunning) {
+  if (!gIsDMDInitialized) {
     // DMD hasn't started up, either because it wasn't enabled by the user, or
     // we're still in Init() and something has indirectly called malloc.  Do a
     // vanilla malloc.  (In the latter case, if it fails we'll crash.  But
@@ -1050,7 +1074,7 @@ replace_calloc(size_t aCount, size_t aSize)
 {
   using namespace mozilla::dmd;
 
-  if (!gIsDMDRunning) {
+  if (!gIsDMDInitialized) {
     return gMallocTable->calloc(aCount, aSize);
   }
 
@@ -1069,7 +1093,7 @@ replace_realloc(void* aOldPtr, size_t aSize)
 {
   using namespace mozilla::dmd;
 
-  if (!gIsDMDRunning) {
+  if (!gIsDMDInitialized) {
     return gMallocTable->realloc(aOldPtr, aSize);
   }
 
@@ -1106,7 +1130,7 @@ replace_memalign(size_t aAlignment, size_t aSize)
 {
   using namespace mozilla::dmd;
 
-  if (!gIsDMDRunning) {
+  if (!gIsDMDInitialized) {
     return gMallocTable->memalign(aAlignment, aSize);
   }
 
@@ -1125,7 +1149,7 @@ replace_free(void* aPtr)
 {
   using namespace mozilla::dmd;
 
-  if (!gIsDMDRunning) {
+  if (!gIsDMDInitialized) {
     gMallocTable->free(aPtr);
     return;
   }
@@ -1266,11 +1290,8 @@ Options::BadArg(const char* aArg)
   StatusMsg("\n");
   StatusMsg("Bad entry in the $DMD environment variable: '%s'.\n", aArg);
   StatusMsg("\n");
-  StatusMsg("Valid values of $DMD are:\n");
-  StatusMsg("- undefined or \"\" or \"0\", which disables DMD, or\n");
-  StatusMsg("- \"1\", which enables it with the default options, or\n");
-  StatusMsg("- a whitespace-separated list of |--option=val| entries, which\n");
-  StatusMsg("  enables it with non-default options.\n");
+  StatusMsg("$DMD must be a whitespace-separated list of |--option=val|\n");
+  StatusMsg("entries.\n");
   StatusMsg("\n");
   StatusMsg("The following options are allowed;  defaults are shown in [].\n");
   StatusMsg("  --sample-below=<1..%d> Sample blocks smaller than this [%d]\n",
@@ -1304,18 +1325,13 @@ NopStackWalkCallback(uint32_t aFrameNumber, void* aPc, void* aSp,
 static void
 Init(const malloc_table_t* aMallocTable)
 {
-  MOZ_ASSERT(!gIsDMDRunning);
-
   gMallocTable = aMallocTable;
 
   // DMD is controlled by the |DMD| environment variable.
-  // - If it's unset or empty or "0", DMD doesn't run.
-  // - Otherwise, the contents dictate DMD's behaviour.
+  const char* e = getenv("DMD");
 
-  char* e = getenv("DMD");
-
-  if (!e || strcmp(e, "") == 0 || strcmp(e, "0") == 0) {
-    return;
+  if (!e) {
+    e = "1";
   }
 
   StatusMsg("$DMD = '%s'\n", e);
@@ -1350,7 +1366,7 @@ Init(const malloc_table_t* aMallocTable)
     gBlockTable->init(8192);
   }
 
-  gIsDMDRunning = true;
+  gIsDMDInitialized = true;
 }
 
 //---------------------------------------------------------------------------
@@ -1360,7 +1376,7 @@ Init(const malloc_table_t* aMallocTable)
 static void
 ReportHelper(const void* aPtr, bool aReportedOnAlloc)
 {
-  if (!gIsDMDRunning || !aPtr) {
+  if (!aPtr) {
     return;
   }
 
@@ -1379,14 +1395,14 @@ ReportHelper(const void* aPtr, bool aReportedOnAlloc)
   }
 }
 
-MOZ_EXPORT void
-Report(const void* aPtr)
+void
+DMDFuncs::Report(const void* aPtr)
 {
   ReportHelper(aPtr, /* onAlloc */ false);
 }
 
-MOZ_EXPORT void
-ReportOnAlloc(const void* aPtr)
+void
+DMDFuncs::ReportOnAlloc(const void* aPtr)
 {
   ReportHelper(aPtr, /* onAlloc */ true);
 }
@@ -1420,10 +1436,6 @@ SizeOfInternal(Sizes* aSizes)
 
   aSizes->Clear();
 
-  if (!gIsDMDRunning) {
-    return;
-  }
-
   StackTraceSet usedStackTraces;
   GatherUsedStackTraces(usedStackTraces);
 
@@ -1445,27 +1457,19 @@ SizeOfInternal(Sizes* aSizes)
   aSizes->mBlockTable = gBlockTable->sizeOfIncludingThis(MallocSizeOf);
 }
 
-MOZ_EXPORT void
-SizeOf(Sizes* aSizes)
+void
+DMDFuncs::SizeOf(Sizes* aSizes)
 {
   aSizes->Clear();
 
-  if (!gIsDMDRunning) {
-    return;
-  }
-
   AutoBlockIntercepts block(Thread::Fetch());
   AutoLockState lock;
   SizeOfInternal(aSizes);
 }
 
-MOZ_EXPORT void
-ClearReports()
+void
+DMDFuncs::ClearReports()
 {
-  if (!gIsDMDRunning) {
-    return;
-  }
-
   AutoLockState lock;
 
   // Unreport all blocks that were marked reported by a memory reporter.  This
@@ -1476,12 +1480,6 @@ ClearReports()
   }
 }
 
-MOZ_EXPORT bool
-IsRunning()
-{
-  return gIsDMDRunning;
-}
-
 class ToIdStringConverter MOZ_FINAL
 {
 public:
@@ -1551,10 +1549,6 @@ private:
 static void
 AnalyzeReportsImpl(UniquePtr<JSONWriteFunc> aWriter)
 {
-  if (!gIsDMDRunning) {
-    return;
-  }
-
   AutoBlockIntercepts block(Thread::Fetch());
   AutoLockState lock;
 
@@ -1727,8 +1721,8 @@ AnalyzeReportsImpl(UniquePtr<JSONWriteFunc> aWriter)
   StatusMsg("}\n");
 }
 
-MOZ_EXPORT void
-AnalyzeReports(UniquePtr<JSONWriteFunc> aWriter)
+void
+DMDFuncs::AnalyzeReports(UniquePtr<JSONWriteFunc> aWriter)
 {
   AnalyzeReportsImpl(Move(aWriter));
   ClearReports();
@@ -1738,14 +1732,14 @@ AnalyzeReports(UniquePtr<JSONWriteFunc> aWriter)
 // Testing
 //---------------------------------------------------------------------------
 
-MOZ_EXPORT void
-SetSampleBelowSize(size_t aSize)
+void
+DMDFuncs::SetSampleBelowSize(size_t aSize)
 {
   gOptions->SetSampleBelowSize(aSize);
 }
 
-MOZ_EXPORT void
-ClearBlocks()
+void
+DMDFuncs::ClearBlocks()
 {
   gBlockTable->clear();
   gSmallBlockActualSizeCounter = 0;

memory/replace/dmd/DMD.h

@@ -8,23 +8,117 @@
 #define DMD_h___
 
 #include <string.h>
+#include <stdarg.h>
 
+#include "mozilla/DebugOnly.h"
+#include "mozilla/Move.h"
 #include "mozilla/Types.h"
 #include "mozilla/UniquePtr.h"
 
+#include "replace_malloc_bridge.h"
+
 namespace mozilla {
 
 class JSONWriteFunc;
 
 namespace dmd {
 
+struct Sizes
+{
+  size_t mStackTracesUsed;
+  size_t mStackTracesUnused;
+  size_t mStackTraceTable;
+  size_t mBlockTable;
+
+  Sizes() { Clear(); }
+  void Clear() { memset(this, 0, sizeof(Sizes)); }
+};
+
+// See further below for a description of each method. The DMDFuncs class
+// should contain a virtual method for each of them (except IsRunning,
+// which can be inferred from the DMDFuncs singleton existing).
+struct DMDFuncs
+{
+  virtual void Report(const void*);
+
+  virtual void ReportOnAlloc(const void*);
+
+  virtual void ClearReports();
+
+  virtual void AnalyzeReports(UniquePtr<JSONWriteFunc>);
+
+  virtual void SizeOf(Sizes*);
+
+  virtual void StatusMsg(const char*, va_list);
+
+  virtual void SetSampleBelowSize(size_t);
+
+  virtual void ClearBlocks();
+
+#ifndef REPLACE_MALLOC_IMPL
+  // We deliberately don't use ReplaceMalloc::GetDMDFuncs here, because if we
+  // did, the following would happen.
+  // - The code footprint of each call to Get() larger as GetDMDFuncs ends
+  //   up inlined.
+  // - When no replace-malloc library is loaded, the number of instructions
+  //   executed is equivalent, but don't necessarily fit in the same cache
+  //   line.
+  // - When a non-DMD replace-malloc library is loaded, the overhead is
+  //   higher because there is first a check for the replace malloc bridge
+  //   and then for the DMDFuncs singleton.
+  // Initializing the DMDFuncs singleton on the first access makes the
+  // overhead even worse. Either Get() is inlined and massive, or it isn't
+  // and a simple value check becomes a function call.
+  static DMDFuncs* Get() { return sSingleton.Get(); }
+
+private:
+  // Wrapper class keeping a pointer to the DMD functions. It is statically
+  // initialized because it needs to be set early enough.
+  // Debug builds also check that it's never accessed before the static
+  // initialization actually occured, which could be the case if some other
+  // static initializer ended up calling into DMD.
+  class Singleton
+  {
+  public:
+    Singleton() : mValue(ReplaceMalloc::GetDMDFuncs()), mInitialized(true) {}
+
+    DMDFuncs* Get()
+    {
+      MOZ_ASSERT(mInitialized);
+      return mValue;
+    }
+
+  private:
+    DMDFuncs* mValue;
+    DebugOnly<bool> mInitialized;
+  };
+
+  // This singleton pointer must be defined on the program side. In Gecko,
+  // this is done in xpcom/base/nsMemoryInfoDumper.cpp.
+  static /* DMDFuncs:: */Singleton sSingleton;
+#endif
+};
+
+#ifndef REPLACE_MALLOC_IMPL
 // Mark a heap block as reported by a memory reporter.
-MOZ_EXPORT void
-Report(const void* aPtr);
+inline void
+Report(const void* aPtr)
+{
+  DMDFuncs* funcs = DMDFuncs::Get();
+  if (funcs) {
+    funcs->Report(aPtr);
+  }
+}
 
 // Mark a heap block as reported immediately on allocation.
-MOZ_EXPORT void
-ReportOnAlloc(const void* aPtr);
+inline void
+ReportOnAlloc(const void* aPtr)
+{
+  DMDFuncs* funcs = DMDFuncs::Get();
+  if (funcs) {
+    funcs->ReportOnAlloc(aPtr);
+  }
+}
 
 // Clears existing reportedness data from any prior runs of the memory
 // reporters.  The following sequence should be used.
@@ -32,8 +126,14 @@ ReportOnAlloc(const void* aPtr);
 // - run the memory reporters
 // - AnalyzeReports()
 // This sequence avoids spurious twice-reported warnings.
-MOZ_EXPORT void
-ClearReports();
+inline void
+ClearReports()
+{
+  DMDFuncs* funcs = DMDFuncs::Get();
+  if (funcs) {
+    funcs->ClearReports();
+  }
+}
 
 // Determines which heap blocks have been reported, and dumps JSON output
 // (via |aWriter|) describing the heap.
@@ -121,42 +221,74 @@ ClearReports();
 //     "H": "#00: quuux (Quux.cpp:567)"
 //   }
 // }
-MOZ_EXPORT void
-AnalyzeReports(mozilla::UniquePtr<mozilla::JSONWriteFunc>);
-
-struct Sizes
+// Implementation note: normally, this wouldn't be templated, but in that case,
+// the function is compiled, which makes the destructor for the UniquePtr fire up,
+// and that needs JSONWriteFunc to be fully defined. That, in turn, requires to
+// include JSONWriter.h, which includes double-conversion.h, which ends up breaking
+// various things built with -Werror for various reasons.
+template <typename JSONWriteFunc>
+inline void
+AnalyzeReports(UniquePtr<JSONWriteFunc> aWriteFunc)
 {
-  size_t mStackTracesUsed;
-  size_t mStackTracesUnused;
-  size_t mStackTraceTable;
-  size_t mBlockTable;
-
-  Sizes() { Clear(); }
-  void Clear() { memset(this, 0, sizeof(Sizes)); }
-};
+  DMDFuncs* funcs = DMDFuncs::Get();
+  if (funcs) {
+    funcs->AnalyzeReports(Move(aWriteFunc));
+  }
+}
 
 // Gets the size of various data structures.  Used to implement a memory
 // reporter for DMD.
-MOZ_EXPORT void
-SizeOf(Sizes* aSizes);
+inline void
+SizeOf(Sizes* aSizes)
+{
+  DMDFuncs* funcs = DMDFuncs::Get();
+  if (funcs) {
+    funcs->SizeOf(aSizes);
+  }
+}
 
 // Prints a status message prefixed with "DMD[<pid>]". Use sparingly.
-MOZ_EXPORT void
-StatusMsg(const char* aFmt, ...);
+inline void
+StatusMsg(const char* aFmt, ...)
+{
+  DMDFuncs* funcs = DMDFuncs::Get();
+  if (funcs) {
+    va_list ap;
+    va_start(ap, aFmt);
+    funcs->StatusMsg(aFmt, ap);
+    va_end(ap);
+  }
+}
 
 // Indicates whether or not DMD is running.
-MOZ_EXPORT bool
-IsRunning();
+inline bool
+IsRunning()
+{
+  return !!DMDFuncs::Get();
+}
 
 // Sets the sample-below size. Only used for testing purposes.
-MOZ_EXPORT void
-SetSampleBelowSize(size_t aSize);
+inline void
+SetSampleBelowSize(size_t aSize)
+{
+  DMDFuncs* funcs = DMDFuncs::Get();
+  if (funcs) {
+    funcs->SetSampleBelowSize(aSize);
+  }
+}
 
 // Clears all records of live allocations. Only used for testing purposes.
-MOZ_EXPORT void
-ClearBlocks();
+inline void
+ClearBlocks()
+{
+  DMDFuncs* funcs = DMDFuncs::Get();
+  if (funcs) {
+    funcs->ClearBlocks();
+  }
+}
+#endif
 
-} // namespace mozilla
 } // namespace dmd
+} // namespace mozilla
 
 #endif /* DMD_h___ */

memory/replace/dmd/test/SmokeDMD.cpp

@@ -26,6 +26,8 @@ using mozilla::JSONWriter;
 using mozilla::MakeUnique;
 using namespace mozilla::dmd;
 
+DMDFuncs::Singleton DMDFuncs::sSingleton;
+
 class FpWriteFunc : public mozilla::JSONWriteFunc
 {
 public:

memory/replace/dmd/test/moz.build

@@ -18,8 +18,6 @@ DEFINES['MOZ_NO_MOZALLOC'] = True
 
 DISABLE_STL_WRAPPING = True
 
-USE_LIBS += ['dmd']
-
 XPCSHELL_TESTS_MANIFESTS += [
     'xpcshell.ini',
 ]

toolkit/library/moz.build

@@ -111,11 +111,6 @@ if CONFIG['MOZ_SANDBOX'] and CONFIG['OS_ARCH'] == 'WINNT':
         'sandboxbroker',
     ]
 
-if CONFIG['MOZ_DMD']:
-    USE_LIBS += [
-        'dmd',
-    ]
-
 USE_LIBS += [
     'gkmedias',
     'mozalloc',

xpcom/base/nsMemoryInfoDumper.cpp

@@ -760,6 +760,8 @@ nsMemoryInfoDumper::DumpMemoryInfoToTempDir(const nsAString& aIdentifier,
 }
 
 #ifdef MOZ_DMD
+dmd::DMDFuncs::Singleton dmd::DMDFuncs::sSingleton;
+
 nsresult
 nsMemoryInfoDumper::OpenDMDFile(const nsAString& aIdentifier, int aPid,
                                 FILE** aOutFile)

xpcom/tests/TestDeadlockDetectorScalability.cpp

@@ -4,6 +4,9 @@
  * 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/. */
 
+// Avoid DMD-specific parts of MOZ_DEFINE_MALLOC_SIZE_OF
+#undef MOZ_DMD
+
 #include "TestHarness.h"
 #include "nsIMemoryReporter.h"
 

xpcom/tests/moz.build

@@ -122,8 +122,3 @@ LOCAL_INCLUDES += [
 RESOURCE_FILES += [
     'test.properties',
 ]
-
-if CONFIG['MOZ_DMD']:
-    USE_LIBS += [
-        'dmd'
-    ]