Backed out changeset 5b30f38210e1 (bug 1393384)

layout/style/ServoBindings.cpp

@@ -29,7 +29,6 @@
 #include "nsIDocumentInlines.h"
 #include "nsILoadContext.h"
 #include "nsIFrame.h"
-#include "nsIMemoryReporter.h"
 #include "nsINode.h"
 #include "nsIPresShell.h"
 #include "nsIPresShellInlines.h"
@@ -227,23 +226,38 @@ ServoComputedData::GetStyleVariables() const
             "called");
 }
 
-MOZ_DEFINE_MALLOC_ENCLOSING_SIZE_OF(ServoStyleStructsMallocEnclosingSizeOf)
+MOZ_DEFINE_MALLOC_SIZE_OF(ServoStyleStructsMallocSizeOf)
 
 void
 ServoComputedData::AddSizeOfExcludingThis(nsWindowSizes& aSizes) const
 {
-  // Note: GetStyleFoo() returns a pointer to an nsStyleFoo that sits within a
-  // servo_arc::Arc, i.e. it is preceded by a word-sized refcount. So we need
-  // to measure it with a function that can handle an interior pointer. We use
-  // ServoStyleStructsEnclosingMallocSizeOf to clearly identify in DMD's
-  // output the memory measured here.
+  // XXX WARNING: GetStyleFoo() returns an nsStyleFoo pointer. This nsStyleFoo
+  // sits within a servo_arc::Arc, i.e. it is preceded by a word-sized
+  // refcount. So this pointer is an interior pointer. To get the start address
+  // of the heap block we move the pointer back by one word. For this to work,
+  // two things must be true.
+  //
+  // - The layout of servo_arc::Arc must stay the same.
+  //
+  // - The alignment of each nsStyleFoo must not be greater than the size of a
+  //   word (otherwise padding might be inserted between the refcount and the
+  //   struct in the servo_arc::Arc).
+  //
+  // In the long run a better solution here is for mozjemalloc to provide a
+  // function that converts an interior pointer to a start pointer (bug
+  // 1389305), but that's not available right now.
+  //
+  // Also, we use ServoStyleStructsMallocSizeOf rather than
+  // |aSizes.mState.mMallocSizeOf| to better distinguish in DMD's output the
+  // memory measured here.
 #define STYLE_STRUCT(name_, cb_) \
   static_assert(alignof(nsStyle##name_) <= sizeof(size_t), \
                 "alignment will break AddSizeOfExcludingThis()"); \
-  const void* p##name_ = GetStyle##name_(); \
+  const char* p##name_ = reinterpret_cast<const char*>(GetStyle##name_()); \
+  p##name_ -= sizeof(size_t); \
   if (!aSizes.mState.HaveSeenPtr(p##name_)) { \
     aSizes.mServoStyleSizes.NS_STYLE_SIZES_FIELD(name_) += \
-      ServoStyleStructsMallocEnclosingSizeOf(p##name_); \
+      ServoStyleStructsMallocSizeOf(p##name_); \
   }
   #define STYLE_STRUCT_LIST_IGNORE_VARIABLES
 #include "nsStyleStructList.h"

layout/style/ServoStyleContext.h

@@ -18,7 +18,7 @@ namespace dom {
 class Element;
 } // namespace dom
 
-MOZ_DEFINE_MALLOC_ENCLOSING_SIZE_OF(ServoComputedValuesMallocEnclosingSizeOf)
+MOZ_DEFINE_MALLOC_SIZE_OF(ServoComputedValuesMallocSizeOf)
 
 class ServoStyleContext final : public nsStyleContext
 {
@@ -105,11 +105,19 @@ public:
   // the size should be added to.
   void AddSizeOfIncludingThis(nsWindowSizes& aSizes, size_t* aCVsSize) const
   {
-    // Note: |this| sits within a servo_arc::Arc, i.e. it is preceded by a
-    // refcount. So we need to measure it with a function that can handle an
-    // interior pointer. We use ServoComputedValuesMallocEnclosingSizeOf to
-    // clearly identify in DMD's output the memory measured here.
-    *aCVsSize += ServoComputedValuesMallocEnclosingSizeOf(this);
+    // XXX WARNING: similar to ServoComputedData::AddSizeOfExcludingThis(),
+    // but here we need to step back 4 or 8 bytes to get past the servo_arc::Arc
+    // refcount to the base pointer.
+    static_assert(alignof(ServoStyleContext) == 4 ||
+                  alignof(ServoStyleContext) == 8,
+                  "alignment will break AddSizeOfExcludingThis()");
+    const char* p = reinterpret_cast<const char*>(this);
+    p -= std::max(sizeof(size_t), alignof(ServoStyleContext));
+
+    // We use ServoComputedValuesMallocSizeOf rather than
+    // |aState.mMallocSizeOf| to better distinguish in DMD's output the memory
+    // measured here.
+    *aCVsSize += ServoComputedValuesMallocSizeOf(p);
     mSource.AddSizeOfExcludingThis(aSizes);
   }
 

memory/mozjemalloc/mozjemalloc_types.h

@@ -115,7 +115,7 @@ typedef struct {
   size_t size;    // meaning depends on tag; see above
 } jemalloc_ptr_info_t;
 
-static inline bool
+static inline jemalloc_bool
 jemalloc_ptr_is_live(jemalloc_ptr_info_t* info)
 {
   return info->tag == TagLiveSmall ||
@@ -123,7 +123,7 @@ jemalloc_ptr_is_live(jemalloc_ptr_info_t* info)
          info->tag == TagLiveHuge;
 }
 
-static inline bool
+static inline jemalloc_bool
 jemalloc_ptr_is_freed(jemalloc_ptr_info_t* info)
 {
   return info->tag == TagFreedSmall ||
@@ -133,7 +133,7 @@ jemalloc_ptr_is_freed(jemalloc_ptr_info_t* info)
          info->tag == TagFreedPageZeroed;
 }
 
-static inline bool
+static inline jemalloc_bool
 jemalloc_ptr_is_freed_page(jemalloc_ptr_info_t* info)
 {
   return info->tag == TagFreedPageDirty ||

xpcom/base/nsIMemoryReporter.idl

@@ -565,31 +565,6 @@ nsresult RegisterNonJSSizeOfTab(NonJSSizeOfTabFn aSizeOfTabFn);
       return moz_malloc_size_of(aPtr);                                        \
   }
 
-// This is an alternative to MOZ_DEFINE_MALLOC_SIZE_OF that defines a
-// MallocSizeOf function that can handle interior pointers.
-#ifdef MOZ_MEMORY
-
-#include "mozmemory.h"
-
-#define MOZ_DEFINE_MALLOC_ENCLOSING_SIZE_OF(fn)                               \
-  static size_t fn(const void* aPtr)                                          \
-  {                                                                           \
-      jemalloc_ptr_info_t info;                                               \
-      jemalloc_ptr_info(aPtr, &info);                                         \
-      MOZ_REPORT(info.addr);                                                  \
-      return jemalloc_ptr_is_live(&info) ? info.size : 0;                     \
-  }
-
-#else
-
-#define MOZ_DEFINE_MALLOC_ENCLOSING_SIZE_OF(fn)                               \
-  static size_t fn(const void* aPtr)                                          \
-  {                                                                           \
-      return 0;                                                               \
-  }
-
-#endif
-
 // Functions generated by the next two macros should be used by wrapping
 // allocators that report heap blocks as soon as they are allocated and
 // unreport them as soon as they are freed.  Such allocators are used in cases