Bug 1281257 - part 2 - make WalkTheStackCached an internal implementation detail; r=erahm
authorNathan Froyd <froydnj@gmail.com>
Thu, 23 Jun 2016 17:21:27 -0400
changeset 327549 500d5c18be7bf9367cce05e899acc9c685cad517
parent 327548 8bd7333536ed08b3db743c40dc98bfb53c20b75a
child 327550 b4f222b71be2dcd6e1231e25bc8a4f08b6e538bf
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerserahm
bugs1281257
milestone50.0a1
Bug 1281257 - part 2 - make WalkTheStackCached an internal implementation detail; r=erahm Nothing needs to call this outside nsTraceRefcnt, and given the potential memory concerns, keeping it private is a better idea anyway.
xpcom/base/nsTraceRefcnt.cpp
xpcom/base/nsTraceRefcnt.h
--- a/xpcom/base/nsTraceRefcnt.cpp
+++ b/xpcom/base/nsTraceRefcnt.cpp
@@ -921,18 +921,26 @@ void
 nsTraceRefcnt::WalkTheStack(FILE* aStream)
 {
 #ifdef MOZ_STACKWALKING
   MozStackWalk(PrintStackFrame, /* skipFrames */ 2, /* maxFrames */ 0, aStream,
                0, nullptr);
 #endif
 }
 
-void
-nsTraceRefcnt::WalkTheStackCached(FILE* aStream)
+/**
+ * This is a variant of |WalkTheStack| that uses |CodeAddressService| to cache
+ * the results of |NS_DescribeCodeAddress|. If |WalkTheStackCached| is being
+ * called frequently, it will be a few orders of magnitude faster than
+ * |WalkTheStack|. However, the cache uses a lot of memory, which can cause
+ * OOM crashes. Therefore, this should only be used for things like refcount
+ * logging which walk the stack extremely frequently.
+ */
+static void
+WalkTheStackCached(FILE* aStream)
 {
 #ifdef MOZ_STACKWALKING
   if (!gCodeAddressService) {
     gCodeAddressService = new WalkTheStackCodeAddressService();
   }
   MozStackWalk(PrintStackFrameCached, /* skipFrames */ 2, /* maxFrames */ 0,
                aStream, 0, nullptr);
 #endif
@@ -1090,24 +1098,24 @@ NS_LogAddRef(void* aPtr, nsrefcnt aRefcn
         (*count)++;
       }
 
     }
 
     bool loggingThisObject = (!gObjectsToLog || LogThisObj(serialno));
     if (aRefcnt == 1 && gAllocLog && loggingThisType && loggingThisObject) {
       fprintf(gAllocLog, "\n<%s> %p %" PRIdPTR " Create [thread %p]\n", aClass, aPtr, serialno, PR_GetCurrentThread());
-      nsTraceRefcnt::WalkTheStackCached(gAllocLog);
+      WalkTheStackCached(gAllocLog);
     }
 
     if (gRefcntsLog && loggingThisType && loggingThisObject) {
       // Can't use MOZ_LOG(), b/c it truncates the line
       fprintf(gRefcntsLog, "\n<%s> %p %" PRIuPTR " AddRef %" PRIuPTR " [thread %p]\n",
               aClass, aPtr, serialno, aRefcnt, PR_GetCurrentThread());
-      nsTraceRefcnt::WalkTheStackCached(gRefcntsLog);
+      WalkTheStackCached(gRefcntsLog);
       fflush(gRefcntsLog);
     }
   }
 #endif
 }
 
 EXPORT_XPCOM_API(void)
 NS_LogRelease(void* aPtr, nsrefcnt aRefcnt, const char* aClass)
@@ -1145,26 +1153,26 @@ NS_LogRelease(void* aPtr, nsrefcnt aRefc
     }
 
     bool loggingThisObject = (!gObjectsToLog || LogThisObj(serialno));
     if (gRefcntsLog && loggingThisType && loggingThisObject) {
       // Can't use MOZ_LOG(), b/c it truncates the line
       fprintf(gRefcntsLog,
               "\n<%s> %p %" PRIuPTR " Release %" PRIuPTR " [thread %p]\n",
               aClass, aPtr, serialno, aRefcnt, PR_GetCurrentThread());
-      nsTraceRefcnt::WalkTheStackCached(gRefcntsLog);
+      WalkTheStackCached(gRefcntsLog);
       fflush(gRefcntsLog);
     }
 
     // Here's the case where MOZ_COUNT_DTOR was not used,
     // yet we still want to see deletion information:
 
     if (aRefcnt == 0 && gAllocLog && loggingThisType && loggingThisObject) {
       fprintf(gAllocLog, "\n<%s> %p %" PRIdPTR " Destroy [thread %p]\n", aClass, aPtr, serialno, PR_GetCurrentThread());
-      nsTraceRefcnt::WalkTheStackCached(gAllocLog);
+      WalkTheStackCached(gAllocLog);
     }
 
     if (aRefcnt == 0 && gSerialNumbers && loggingThisType) {
       RecycleSerialNumberPtr(aPtr);
     }
   }
 #endif
 }
@@ -1193,17 +1201,17 @@ NS_LogCtor(void* aPtr, const char* aType
     if (gSerialNumbers && loggingThisType) {
       serialno = GetSerialNumber(aPtr, true);
     }
 
     bool loggingThisObject = (!gObjectsToLog || LogThisObj(serialno));
     if (gAllocLog && loggingThisType && loggingThisObject) {
       fprintf(gAllocLog, "\n<%s> %p %" PRIdPTR " Ctor (%d)\n",
               aType, aPtr, serialno, aInstanceSize);
-      nsTraceRefcnt::WalkTheStackCached(gAllocLog);
+      WalkTheStackCached(gAllocLog);
     }
   }
 #endif
 }
 
 
 EXPORT_XPCOM_API(void)
 NS_LogDtor(void* aPtr, const char* aType, uint32_t aInstanceSize)
@@ -1233,17 +1241,17 @@ NS_LogDtor(void* aPtr, const char* aType
 
     bool loggingThisObject = (!gObjectsToLog || LogThisObj(serialno));
 
     // (If we're on a losing architecture, don't do this because we'll be
     // using LogDeleteXPCOM instead to get file and line numbers.)
     if (gAllocLog && loggingThisType && loggingThisObject) {
       fprintf(gAllocLog, "\n<%s> %p %" PRIdPTR " Dtor (%d)\n",
               aType, aPtr, serialno, aInstanceSize);
-      nsTraceRefcnt::WalkTheStackCached(gAllocLog);
+      WalkTheStackCached(gAllocLog);
     }
   }
 #endif
 }
 
 
 EXPORT_XPCOM_API(void)
 NS_LogCOMPtrAddRef(void* aCOMPtr, nsISupports* aObject)
@@ -1274,17 +1282,17 @@ NS_LogCOMPtrAddRef(void* aCOMPtr, nsISup
       (*count)++;
     }
 
     bool loggingThisObject = (!gObjectsToLog || LogThisObj(serialno));
 
     if (gCOMPtrLog && loggingThisObject) {
       fprintf(gCOMPtrLog, "\n<?> %p %" PRIdPTR " nsCOMPtrAddRef %d %p\n",
               object, serialno, count ? (*count) : -1, aCOMPtr);
-      nsTraceRefcnt::WalkTheStackCached(gCOMPtrLog);
+      WalkTheStackCached(gCOMPtrLog);
     }
   }
 #endif
 }
 
 
 EXPORT_XPCOM_API(void)
 NS_LogCOMPtrRelease(void* aCOMPtr, nsISupports* aObject)
@@ -1315,17 +1323,17 @@ NS_LogCOMPtrRelease(void* aCOMPtr, nsISu
       (*count)--;
     }
 
     bool loggingThisObject = (!gObjectsToLog || LogThisObj(serialno));
 
     if (gCOMPtrLog && loggingThisObject) {
       fprintf(gCOMPtrLog, "\n<?> %p %" PRIdPTR " nsCOMPtrRelease %d %p\n",
               object, serialno, count ? (*count) : -1, aCOMPtr);
-      nsTraceRefcnt::WalkTheStackCached(gCOMPtrLog);
+      WalkTheStackCached(gCOMPtrLog);
     }
   }
 #endif
 }
 
 void
 nsTraceRefcnt::Shutdown()
 {
--- a/xpcom/base/nsTraceRefcnt.h
+++ b/xpcom/base/nsTraceRefcnt.h
@@ -22,26 +22,16 @@ public:
   static nsresult DumpStatistics(StatisticsType aType = ALL_STATS,
                                  FILE* aOut = 0);
 
   static void ResetStatistics();
 
   static void WalkTheStack(FILE* aStream);
 
   /**
-   * This is a variant of |WalkTheStack| that uses |CodeAddressService| to cache
-   * the results of |NS_DescribeCodeAddress|. If |WalkTheStackCached| is being
-   * called frequently, it will be a few orders of magnitude faster than
-   * |WalkTheStack|. However, the cache uses a lot of memory, which can cause
-   * OOM crashes. Therefore, this should only be used for things like refcount
-   * logging which walk the stack extremely frequently.
-   */
-  static void WalkTheStackCached(FILE* aStream);
-
-  /**
    * Tell nsTraceRefcnt whether refcounting, allocation, and destruction
    * activity is legal.  This is used to trigger assertions for any such
    * activity that occurs because of static constructors or destructors.
    */
   static void SetActivityIsLegal(bool aLegal);
 };
 
 ////////////////////////////////////////////////////////////////////////////////