Bug 1249174 (part 7.5) - Avoid wasted space around XPT strings. r=khuey.
authorNicholas Nethercote <nnethercote@mozilla.com>
Tue, 23 Feb 2016 16:17:58 +1100
changeset 324234 ca3090cb8e36dd77612ffdc84cd8953897ff72fd
parent 324233 6a5f30460690b5089f648e8a38eed27d52a8d597
child 324235 b5d5f42ed917c073f2edaeb01e97f88bdaa4d1fd
push id1128
push userjlund@mozilla.com
push dateWed, 01 Jun 2016 01:31:59 +0000
treeherdermozilla-release@fe0d30de989d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey
bugs1249174
milestone47.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 1249174 (part 7.5) - Avoid wasted space around XPT strings. r=khuey. This patch: - Removes XPTArena's ability to support arbitrary alignments. - Hardwires two sub-arenas into XPTArena, one with alignment of 8 and one with alignment of 1. - Uses the first sub-arena for most allocations and the second sub-arena for C string allocations. These changes reduce "xpti-working-set" by 56 KiB. The patch also renames all the used of "malloc" in XPT identifiers with "calloc", to make clearer that the result is always zeroed.
xpcom/reflect/xptinfo/xptiInterfaceInfo.cpp
xpcom/reflect/xptinfo/xptiInterfaceInfoManager.cpp
xpcom/reflect/xptinfo/xptiTypelibGuts.cpp
xpcom/reflect/xptinfo/xptiWorkingSet.cpp
xpcom/typelib/xpt/xpt_arena.cpp
xpcom/typelib/xpt/xpt_arena.h
xpcom/typelib/xpt/xpt_struct.cpp
xpcom/typelib/xpt/xpt_xdr.cpp
--- a/xpcom/reflect/xptinfo/xptiInterfaceInfo.cpp
+++ b/xpcom/reflect/xptinfo/xptiInterfaceInfo.cpp
@@ -43,19 +43,20 @@ static int DEBUG_ReentrantMonitorEntryCo
 #endif /* SHOW_INFO_COUNT_STATS */
 
 /* static */ xptiInterfaceEntry*
 xptiInterfaceEntry::Create(const char* name, const nsID& iid,
                            XPTInterfaceDescriptor* aDescriptor,
                            xptiTypelibGuts* aTypelib)
 {
     int namelen = strlen(name);
-    return new (XPT_MALLOC(gXPTIStructArena,
-                           sizeof(xptiInterfaceEntry) + namelen))
-        xptiInterfaceEntry(name, namelen, iid, aDescriptor, aTypelib);
+    void* place =
+        XPT_CALLOC8(gXPTIStructArena, sizeof(xptiInterfaceEntry) + namelen);
+    return new (place) xptiInterfaceEntry(name, namelen, iid, aDescriptor,
+                                          aTypelib);
 }
 
 xptiInterfaceEntry::xptiInterfaceEntry(const char* name,
                                        size_t nameLength,
                                        const nsID& iid,
                                        XPTInterfaceDescriptor* aDescriptor,
                                        xptiTypelibGuts* aTypelib)
     : mIID(iid)
--- a/xpcom/reflect/xptinfo/xptiInterfaceInfoManager.cpp
+++ b/xpcom/reflect/xptinfo/xptiInterfaceInfoManager.cpp
@@ -47,17 +47,17 @@ NS_IMETHODIMP
 XPTInterfaceInfoManager::CollectReports(nsIHandleReportCallback* aHandleReport,
                                         nsISupports* aData, bool aAnonymize)
 {
     size_t amount = SizeOfIncludingThis(XPTIMallocSizeOf);
 
     // Measure gXPTIStructArena here, too.  This is a bit grotty because it
     // doesn't belong to the XPTIInterfaceInfoManager, but there's no
     // obviously better place to measure it.
-    amount += XPT_SizeOfArena(gXPTIStructArena, XPTIMallocSizeOf);
+    amount += XPT_SizeOfArenaIncludingThis(gXPTIStructArena, XPTIMallocSizeOf);
 
     return MOZ_COLLECT_REPORT(
         "explicit/xpti-working-set", KIND_HEAP, UNITS_BYTES, amount,
         "Memory used by the XPCOM typelib system.");
 }
 
 // static
 XPTInterfaceInfoManager*
--- a/xpcom/reflect/xptinfo/xptiTypelibGuts.cpp
+++ b/xpcom/reflect/xptinfo/xptiTypelibGuts.cpp
@@ -17,21 +17,20 @@ class MOZ_NEEDS_NO_VTABLE_TYPE CheckNoVT
 };
 CheckNoVTable<xptiTypelibGuts> gChecker;
 
 // static 
 xptiTypelibGuts* 
 xptiTypelibGuts::Create(XPTHeader* aHeader)
 {
     NS_ASSERTION(aHeader, "bad param");
-    void* place = XPT_MALLOC(gXPTIStructArena,
-                             sizeof(xptiTypelibGuts) + 
-                             (sizeof(xptiInterfaceEntry*) *
-                              (aHeader->num_interfaces - 1)));
-    if(!place)
+    size_t n = sizeof(xptiTypelibGuts) +
+               sizeof(xptiInterfaceEntry*) * (aHeader->num_interfaces - 1);
+    void* place = XPT_CALLOC8(gXPTIStructArena, n);
+    if (!place)
         return nullptr;
     return new(place) xptiTypelibGuts(aHeader);
 }
 
 xptiInterfaceEntry*
 xptiTypelibGuts::GetEntryAt(uint16_t i)
 {
     static const nsID zeroIID =
--- a/xpcom/reflect/xptinfo/xptiWorkingSet.cpp
+++ b/xpcom/reflect/xptinfo/xptiWorkingSet.cpp
@@ -8,28 +8,30 @@
 
 #include "mozilla/XPTInterfaceInfoManager.h"
 
 #include "xptiprivate.h"
 #include "nsString.h"
 
 using namespace mozilla;
 
-#define XPTI_STRUCT_ARENA_BLOCK_SIZE    (1024 * 16)
-#define XPTI_HASHTABLE_LENGTH           1024
+static const size_t XPTI_ARENA8_BLOCK_SIZE = 16 * 1024;
+static const size_t XPTI_ARENA1_BLOCK_SIZE =  8 * 1024;
+
+static const uint32_t XPTI_HASHTABLE_LENGTH = 1024;
 
 XPTInterfaceInfoManager::xptiWorkingSet::xptiWorkingSet()
     : mTableReentrantMonitor("xptiWorkingSet::mTableReentrantMonitor")
     , mIIDTable(XPTI_HASHTABLE_LENGTH)
     , mNameTable(XPTI_HASHTABLE_LENGTH)
 {
     MOZ_COUNT_CTOR(xptiWorkingSet);
 
-    gXPTIStructArena =
-        XPT_NewArena(XPTI_STRUCT_ARENA_BLOCK_SIZE, sizeof(double));
+    gXPTIStructArena = XPT_NewArena(XPTI_ARENA8_BLOCK_SIZE,
+                                    XPTI_ARENA1_BLOCK_SIZE);
 }
 
 void
 XPTInterfaceInfoManager::xptiWorkingSet::InvalidateInterfaceInfos()
 {
     ReentrantMonitorAutoEnter monitor(mTableReentrantMonitor);
     for (auto iter = mNameTable.Iter(); !iter.Done(); iter.Next()) {
         xptiInterfaceEntry* entry = iter.UserData();
--- a/xpcom/typelib/xpt/xpt_arena.cpp
+++ b/xpcom/typelib/xpt/xpt_arena.cpp
@@ -24,150 +24,173 @@ struct BLK_HDR
     BLK_HDR *next;
 };
 
 #define XPT_MIN_BLOCK_SIZE 32
 
 /* XXX this is lame. Should clone the code to do this bitwise */
 #define ALIGN_RND(s,a) ((a)==1?(s):((((s)+(a)-1)/(a))*(a)))
 
-struct XPTArena
+struct XPTSubArena
 {
     BLK_HDR *first;
     uint8_t *next;
     size_t   space;
-    size_t   alignment;
     size_t   block_size;
 };
 
+struct XPTArena
+{
+    // We have one sub-arena with 8-byte alignment for most allocations, and
+    // one with 1-byte alignment for C string allocations. The latter sub-arena
+    // avoids significant amounts of unnecessary padding between C strings.
+    XPTSubArena subarena8;
+    XPTSubArena subarena1;
+};
+
 XPT_PUBLIC_API(XPTArena *)
-XPT_NewArena(uint32_t block_size, size_t alignment)
+XPT_NewArena(size_t block_size8, size_t block_size1)
 {
-    XPTArena *arena = (XPTArena*)calloc(1, sizeof(XPTArena));
+    XPTArena *arena = static_cast<XPTArena*>(calloc(1, sizeof(XPTArena)));
     if (arena) {
-        XPT_ASSERT(alignment);
-        if (alignment > sizeof(double))
-            alignment = sizeof(double);
-        arena->alignment = alignment;
+        if (block_size8 < XPT_MIN_BLOCK_SIZE)
+            block_size8 = XPT_MIN_BLOCK_SIZE;
+        arena->subarena8.block_size = ALIGN_RND(block_size8, 8);
 
-        if (block_size < XPT_MIN_BLOCK_SIZE)
-            block_size = XPT_MIN_BLOCK_SIZE;
-        arena->block_size = ALIGN_RND(block_size, alignment);
+        if (block_size1 < XPT_MIN_BLOCK_SIZE)
+            block_size1 = XPT_MIN_BLOCK_SIZE;
+        arena->subarena1.block_size = block_size1;
+    }
+    return arena;
+}
 
-        /* must have room for at least one item! */
-        XPT_ASSERT(arena->block_size >= 
-                   ALIGN_RND(sizeof(BLK_HDR), alignment) +
-                   ALIGN_RND(1, alignment));
+static void
+DestroySubArena(XPTSubArena *subarena)
+{
+    BLK_HDR* cur = subarena->first;
+    while (cur) {
+        BLK_HDR* next = cur->next;
+        free(cur);
+        cur = next;
     }
-    return arena;        
 }
 
 XPT_PUBLIC_API(void)
 XPT_DestroyArena(XPTArena *arena)
 {
-    BLK_HDR* cur;
-    BLK_HDR* next;
-        
-    cur = arena->first;
-    while (cur) {
-        next = cur->next;
-        free(cur);
-        cur = next;
-    }
+    DestroySubArena(&arena->subarena8);
+    DestroySubArena(&arena->subarena1);
     free(arena);
 }
 
 /* 
 * Our alignment rule is that we always round up the size of each allocation 
 * so that the 'arena->next' pointer one will point to properly aligned space.
 */
 
 XPT_PUBLIC_API(void *)
-XPT_ArenaMalloc(XPTArena *arena, size_t size)
+XPT_ArenaCalloc(XPTArena *arena, size_t size, size_t alignment)
 {
-    uint8_t *cur;
-    size_t bytes;
-
     if (!size)
         return NULL;
 
     if (!arena) {
         XPT_ASSERT(0);
         return NULL;
     }
 
-    bytes = ALIGN_RND(size, arena->alignment);
+    XPTSubArena *subarena;
+    if (alignment == 8) {
+        subarena = &arena->subarena8;
+    } else if (alignment == 1) {
+        subarena = &arena->subarena1;
+    } else {
+        XPT_ASSERT(0);
+        return NULL;
+    }
 
-    if (bytes > arena->space) {
+    size_t bytes = ALIGN_RND(size, alignment);
+
+    if (bytes > subarena->space) {
         BLK_HDR* new_block;
-        size_t block_header_size = ALIGN_RND(sizeof(BLK_HDR), arena->alignment);
-        size_t new_space = arena->block_size;
-         
+        size_t block_header_size = ALIGN_RND(sizeof(BLK_HDR), alignment);
+        size_t new_space = subarena->block_size;
+
         while (bytes > new_space - block_header_size)
-            new_space += arena->block_size;
+            new_space += subarena->block_size;
 
-        new_block = (BLK_HDR*) calloc(new_space/arena->alignment, 
-                                      arena->alignment);
+        new_block =
+            static_cast<BLK_HDR*>(calloc(new_space / alignment, alignment));
         if (!new_block) {
-            arena->next = NULL;
-            arena->space = 0;
+            subarena->next = NULL;
+            subarena->space = 0;
             return NULL;
         }
 
         /* link block into the list of blocks for use when we destroy */
-        new_block->next = arena->first;
-        arena->first = new_block;
+        new_block->next = subarena->first;
+        subarena->first = new_block;
 
         /* set info for current block */
-        arena->next  = ((uint8_t*)new_block) + block_header_size;
-        arena->space = new_space - block_header_size;
+        subarena->next =
+            reinterpret_cast<uint8_t*>(new_block) + block_header_size;
+        subarena->space = new_space - block_header_size;
 
 #ifdef DEBUG
         /* mark block for corruption check */
-        memset(arena->next, 0xcd, arena->space);
+        memset(subarena->next, 0xcd, subarena->space);
 #endif
-    } 
-    
+    }
+
 #ifdef DEBUG
     {
         /* do corruption check */
         size_t i;
         for (i = 0; i < bytes; ++i) {
-            XPT_ASSERT(arena->next[i] == 0xcd);        
+            XPT_ASSERT(subarena->next[i] == 0xcd);
         }
         /* we guarantee that the block will be filled with zeros */
-        memset(arena->next, 0, bytes);
-    }        
+        memset(subarena->next, 0, bytes);
+    }
 #endif
 
-    cur = arena->next;
-    arena->next  += bytes;
-    arena->space -= bytes;
-    
-    return cur;    
+    uint8_t* p = subarena->next;
+    subarena->next  += bytes;
+    subarena->space -= bytes;
+
+    return p;
 }
 
 /***************************************************************************/
 
 #ifdef DEBUG
 XPT_PUBLIC_API(void)
 XPT_AssertFailed(const char *s, const char *file, uint32_t lineno)
 {
     fprintf(stderr, "Assertion failed: %s, file %s, line %d\n",
             s, file, lineno);
     abort();
 }        
 #endif
 
-XPT_PUBLIC_API(size_t)
-XPT_SizeOfArena(XPTArena *arena, MozMallocSizeOf mallocSizeOf)
+static size_t
+SizeOfSubArenaExcludingThis(XPTSubArena *subarena, MozMallocSizeOf mallocSizeOf)
 {
-    size_t n = mallocSizeOf(arena);
+    size_t n = 0;
 
-    BLK_HDR* cur = arena->first;
+    BLK_HDR* cur = subarena->first;
     while (cur) {
         BLK_HDR* next = cur->next;
         n += mallocSizeOf(cur);
         cur = next;
     }
 
     return n;
 }
+
+XPT_PUBLIC_API(size_t)
+XPT_SizeOfArenaIncludingThis(XPTArena *arena, MozMallocSizeOf mallocSizeOf)
+{
+    size_t n = mallocSizeOf(arena);
+    n += SizeOfSubArenaExcludingThis(&arena->subarena8, mallocSizeOf);
+    n += SizeOfSubArenaExcludingThis(&arena->subarena1, mallocSizeOf);
+    return n;
+}
--- a/xpcom/typelib/xpt/xpt_arena.h
+++ b/xpcom/typelib/xpt/xpt_arena.h
@@ -30,34 +30,32 @@ extern "C" {
 
 /*
  * Simple Arena support. Use with caution!
  */ 
 
 typedef struct XPTArena XPTArena;
 
 XPT_PUBLIC_API(XPTArena *)
-XPT_NewArena(uint32_t block_size, size_t alignment);
+XPT_NewArena(size_t block_size8, size_t block_size1);
 
 XPT_PUBLIC_API(void)
 XPT_DestroyArena(XPTArena *arena);
 
 XPT_PUBLIC_API(void *)
-XPT_ArenaMalloc(XPTArena *arena, size_t size);
+XPT_ArenaCalloc(XPTArena *arena, size_t size, size_t alignment);
 
 XPT_PUBLIC_API(size_t)
-XPT_SizeOfArena(XPTArena *arena, MozMallocSizeOf mallocSizeOf);
+XPT_SizeOfArenaIncludingThis(XPTArena *arena, MozMallocSizeOf mallocSizeOf);
 
 /* --------------------------------------------------------- */
 
-#define XPT_MALLOC(_arena, _bytes) \
-    XPT_ArenaMalloc((_arena), (_bytes))
-
-#define XPT_CALLOC(_arena, _size) XPT_MALLOC((_arena), (_size))
-#define XPT_NEW(_arena, _struct) ((_struct *) XPT_MALLOC((_arena), sizeof(_struct)))
+#define XPT_CALLOC8(_arena, _bytes) XPT_ArenaCalloc((_arena), (_bytes), 8)
+#define XPT_CALLOC1(_arena, _bytes) XPT_ArenaCalloc((_arena), (_bytes), 1)
+#define XPT_NEW(_arena, _struct) ((_struct *) XPT_CALLOC8((_arena), sizeof(_struct)))
 #define XPT_NEWZAP(_arena, _struct) XPT_NEW((_arena), _struct)
 
 /* --------------------------------------------------------- */
 
 #ifdef DEBUG
 XPT_PUBLIC_API(void)
 XPT_AssertFailed(const char *s, const char *file, uint32_t lineno)
   MOZ_PRETEND_NORETURN_FOR_STATIC_ANALYSIS;
--- a/xpcom/typelib/xpt/xpt_struct.cpp
+++ b/xpcom/typelib/xpt/xpt_struct.cpp
@@ -103,19 +103,19 @@ XPT_DoHeader(XPTArena *arena, XPTCursor 
 
     uint32_t data_pool;
     if (!XPT_Do32(cursor, &data_pool))
         return PR_FALSE;
 
     XPT_SetDataOffset(cursor->state, data_pool);
 
     if (header->num_interfaces) {
-        header->interface_directory = 
-            (XPTInterfaceDirectoryEntry*)XPT_CALLOC(arena, header->num_interfaces * 
-                                                    sizeof(XPTInterfaceDirectoryEntry));
+        size_t n = header->num_interfaces * sizeof(XPTInterfaceDirectoryEntry);
+        header->interface_directory =
+            static_cast<XPTInterfaceDirectoryEntry*>(XPT_CALLOC8(arena, n));
         if (!header->interface_directory)
             return PR_FALSE;
     }
 
     /*
      * Iterate through the annotations rather than recurring, to avoid blowing
      * the stack on large xpt files. We don't actually store annotations, we
      * just skip over them.
@@ -169,17 +169,17 @@ XPT_InterfaceDescriptorAddTypes(XPTArena
                                 uint16_t num)
 {
     XPTTypeDescriptor *old = id->additional_types;
     XPTTypeDescriptor *new_;
     size_t old_size = id->num_additional_types * sizeof(XPTTypeDescriptor);
     size_t new_size = (num * sizeof(XPTTypeDescriptor)) + old_size;
 
     /* XXX should grow in chunks to minimize alloc overhead */
-    new_ = (XPTTypeDescriptor*)XPT_CALLOC(arena, new_size);
+    new_ = static_cast<XPTTypeDescriptor*>(XPT_CALLOC8(arena, new_size));
     if (!new_)
         return PR_FALSE;
     if (old) {
         memcpy(new_, old, old_size);
     }
     id->additional_types = new_;
     id->num_additional_types += num;
     return PR_TRUE;
@@ -208,34 +208,36 @@ DoInterfaceDescriptor(XPTArena *arena, X
         return PR_TRUE;
     }
     if(!XPT_Do16(cursor, &id->parent_interface) ||
        !XPT_Do16(cursor, &id->num_methods)) {
         return PR_FALSE;
     }
 
     if (id->num_methods) {
-        id->method_descriptors = (XPTMethodDescriptor*)XPT_CALLOC(arena, id->num_methods *
-                                                                  sizeof(XPTMethodDescriptor));
+        size_t n = id->num_methods * sizeof(XPTMethodDescriptor);
+        id->method_descriptors =
+            static_cast<XPTMethodDescriptor*>(XPT_CALLOC8(arena, n));
         if (!id->method_descriptors)
             return PR_FALSE;
     }
     
     for (i = 0; i < id->num_methods; i++) {
         if (!DoMethodDescriptor(arena, cursor, &id->method_descriptors[i], id))
             return PR_FALSE;
     }
     
     if (!XPT_Do16(cursor, &id->num_constants)) {
         return PR_FALSE;
     }
     
     if (id->num_constants) {
-        id->const_descriptors = (XPTConstDescriptor*)XPT_CALLOC(arena, id->num_constants * 
-                                                                sizeof(XPTConstDescriptor));
+        size_t n = id->num_constants * sizeof(XPTConstDescriptor);
+        id->const_descriptors =
+            static_cast<XPTConstDescriptor*>(XPT_CALLOC8(arena, n));
         if (!id->const_descriptors)
             return PR_FALSE;
     }
     
     for (i = 0; i < id->num_constants; i++) {
         if (!DoConstDescriptor(arena, cursor, &id->const_descriptors[i], id)) {
             return PR_FALSE;
         }
@@ -308,17 +310,18 @@ DoMethodDescriptor(XPTArena *arena, XPTC
     int i;
 
     if (!XPT_Do8(cursor, &md->flags) ||
         !XPT_DoCString(arena, cursor, &md->name) ||
         !XPT_Do8(cursor, &md->num_args))
         return PR_FALSE;
 
     if (md->num_args) {
-        md->params = (XPTParamDescriptor*)XPT_CALLOC(arena, md->num_args * sizeof(XPTParamDescriptor));
+        size_t n = md->num_args * sizeof(XPTParamDescriptor);
+        md->params = static_cast<XPTParamDescriptor*>(XPT_CALLOC8(arena, n));
         if (!md->params)
             return PR_FALSE;
     }
 
     for(i = 0; i < md->num_args; i++) {
         if (!DoParamDescriptor(arena, cursor, &md->params[i], id))
             return PR_FALSE;
     }
--- a/xpcom/typelib/xpt/xpt_xdr.cpp
+++ b/xpcom/typelib/xpt/xpt_xdr.cpp
@@ -120,17 +120,17 @@ XPT_DoCString(XPTArena *arena, XPTCursor
     if (!end) {
         fprintf(stderr, "didn't find end of string on decode!\n");
         return PR_FALSE;
     }
     int len = end - start;
     XPT_ASSERT(len > 0);
 
     if (!ignore) {
-        char *ident = (char*)XPT_MALLOC(arena, len + 1u);
+        char *ident = (char*)XPT_CALLOC1(arena, len + 1u);
         if (!ident)
             return PR_FALSE;
 
         memcpy(ident, start, (size_t)len);
         ident[len] = 0;
         *identp = ident;
     }