Bug 1249174 (part 6) - Shrink XPTTypeDescriptor. r=khuey.
authorNicholas Nethercote <nnethercote@mozilla.com>
Tue, 23 Feb 2016 16:17:44 +1100
changeset 322330 6a5f30460690b5089f648e8a38eed27d52a8d597
parent 322329 69bc807d8b8a36604f313626071fa4d624e8605d
child 322331 ca3090cb8e36dd77612ffdc84cd8953897ff72fd
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [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 6) - Shrink XPTTypeDescriptor. r=khuey. With careful layout we can reduce sizeof(XPTTypeDescriptor) from 4 to 3, which also reduces sizeof(XPTParamDescriptor) from 6 to 4. This reduces "xpti-working-set" by 16 KiB. The union-of-structs also improves readability by making it clearer exactly which fields are used for which types.
xpcom/reflect/xptinfo/xptiInterfaceInfo.cpp
xpcom/typelib/xpt/xpt_struct.cpp
xpcom/typelib/xpt/xpt_struct.h
--- a/xpcom/reflect/xptinfo/xptiInterfaceInfo.cpp
+++ b/xpcom/reflect/xptinfo/xptiInterfaceInfo.cpp
@@ -338,25 +338,25 @@ xptiInterfaceEntry::GetInterfaceIndexFor
     {
         NS_ERROR("bad param");
         return NS_ERROR_INVALID_ARG;
     }
 
     const XPTTypeDescriptor *td = &param->type;
 
     while (XPT_TDP_TAG(td->prefix) == TD_ARRAY) {
-        td = &mDescriptor->additional_types[td->type.additional_type];
+        td = &mDescriptor->additional_types[td->u.array.additional_type];
     }
 
     if(XPT_TDP_TAG(td->prefix) != TD_INTERFACE_TYPE) {
         NS_ERROR("not an interface");
         return NS_ERROR_INVALID_ARG;
     }
 
-    *interfaceIndex = td->type.iface;
+    *interfaceIndex = (td->u.iface.iface_hi8 << 8) | td->u.iface.iface_lo8;
     return NS_OK;
 }
 
 nsresult 
 xptiInterfaceEntry::GetEntryForParam(uint16_t methodIndex, 
                                      const nsXPTParamInfo * param,
                                      xptiInterfaceEntry** entry)
 {
@@ -477,17 +477,17 @@ xptiInterfaceEntry::GetTypeInArray(const
     const XPTTypeDescriptor *additional_types =
                 mDescriptor->additional_types;
 
     for (uint16_t i = 0; i < dimension; i++) {
         if(XPT_TDP_TAG(td->prefix) != TD_ARRAY) {
             NS_ERROR("bad dimension");
             return NS_ERROR_INVALID_ARG;
         }
-        td = &additional_types[td->type.additional_type];
+        td = &additional_types[td->u.array.additional_type];
     }
 
     *type = td;
     return NS_OK;
 }
 
 nsresult
 xptiInterfaceEntry::GetTypeForParam(uint16_t methodIndex,
@@ -551,25 +551,27 @@ xptiInterfaceEntry::GetSizeIsArgNumberFo
             return rv;
     }
     else
         td = &param->type;
 
     // verify that this is a type that has size_is
     switch (XPT_TDP_TAG(td->prefix)) {
       case TD_ARRAY:
+        *argnum = td->u.array.argnum;
+        break;
       case TD_PSTRING_SIZE_IS:
       case TD_PWSTRING_SIZE_IS:
+        *argnum = td->u.pstring_is.argnum;
         break;
       default:
         NS_ERROR("not a size_is");
         return NS_ERROR_INVALID_ARG;
     }
 
-    *argnum = td->argnum;
     return NS_OK;
 }
 
 nsresult
 xptiInterfaceEntry::GetInterfaceIsArgNumberForParam(uint16_t methodIndex,
                                                     const nsXPTParamInfo* param,
                                                     uint8_t* argnum)
 {
@@ -585,26 +587,25 @@ xptiInterfaceEntry::GetInterfaceIsArgNum
     {
         NS_ERROR("bad index");
         return NS_ERROR_INVALID_ARG;
     }
 
     const XPTTypeDescriptor *td = &param->type;
 
     while (XPT_TDP_TAG(td->prefix) == TD_ARRAY) {
-        td = &mDescriptor->
-                                additional_types[td->type.additional_type];
+        td = &mDescriptor->additional_types[td->u.array.additional_type];
     }
 
     if(XPT_TDP_TAG(td->prefix) != TD_INTERFACE_IS_TYPE) {
         NS_ERROR("not an iid_is");
         return NS_ERROR_INVALID_ARG;
     }
 
-    *argnum = td->argnum;
+    *argnum = td->u.interface_is.argnum;
     return NS_OK;
 }
 
 nsresult
 xptiInterfaceEntry::IsIID(const nsIID * iid, bool *_retval)
 {
     // It is not necessary to Resolve because this info is read from manifest.
     *_retval = mIID.Equals(*iid);
--- a/xpcom/typelib/xpt/xpt_struct.cpp
+++ b/xpcom/typelib/xpt/xpt_struct.cpp
@@ -351,45 +351,50 @@ DoTypeDescriptor(XPTArena *arena, XPTCur
                  XPTInterfaceDescriptor *id)
 {
     if (!DoTypeDescriptorPrefix(arena, cursor, &td->prefix)) {
         return PR_FALSE;
     }
     
     switch (XPT_TDP_TAG(td->prefix)) {
       case TD_INTERFACE_TYPE:
-        if (!XPT_Do16(cursor, &td->type.iface))
+        uint16_t iface;
+        if (!XPT_Do16(cursor, &iface))
             return PR_FALSE;
+        td->u.iface.iface_hi8 = (iface >> 8) & 0xff;
+        td->u.iface.iface_lo8 = iface & 0xff;
         break;
       case TD_INTERFACE_IS_TYPE:
-        if (!XPT_Do8(cursor, &td->argnum))
+        if (!XPT_Do8(cursor, &td->u.interface_is.argnum))
             return PR_FALSE;
         break;
       case TD_ARRAY: {
         // argnum2 appears in the on-disk format but it isn't used.
         uint8_t argnum2 = 0;
-        if (!XPT_Do8(cursor, &td->argnum) ||
+        if (!XPT_Do8(cursor, &td->u.array.argnum) ||
             !XPT_Do8(cursor, &argnum2))
             return PR_FALSE;
 
         if (!XPT_InterfaceDescriptorAddTypes(arena, id, 1))
             return PR_FALSE;
-        td->type.additional_type = id->num_additional_types - 1;
+        if ((id->num_additional_types - 1) > 255)
+            return PR_FALSE;
+        td->u.array.additional_type = id->num_additional_types - 1;
 
         if (!DoTypeDescriptor(arena, cursor, 
-                              &id->additional_types[td->type.additional_type], 
+                              &id->additional_types[td->u.array.additional_type],
                               id))
             return PR_FALSE;
         break;
       }
       case TD_PSTRING_SIZE_IS:
       case TD_PWSTRING_SIZE_IS: {
         // argnum2 appears in the on-disk format but it isn't used.
         uint8_t argnum2 = 0;
-        if (!XPT_Do8(cursor, &td->argnum) ||
+        if (!XPT_Do8(cursor, &td->u.pstring_is.argnum) ||
             !XPT_Do8(cursor, &argnum2))
             return PR_FALSE;
         break;
       }
       default:
         /* nothing special */
         break;
     }
--- a/xpcom/typelib/xpt/xpt_struct.h
+++ b/xpcom/typelib/xpt/xpt_struct.h
@@ -212,27 +212,48 @@ enum XPTTypeDescriptorTags {
     TD_UTF8STRING        = 23,
     TD_CSTRING           = 24,
     TD_ASTRING           = 25,
     TD_JSVAL             = 26
 };
 
 struct XPTTypeDescriptor {
     XPTTypeDescriptorPrefix prefix;
-    uint8_t argnum;               // used for TD_ARRAY, TD_PSTRING_SIZE_IS,
-                                  // TD_PWSTRING_SIZE_IS, TD_INTERFACE_IS_TYPE
+
+    // The memory layout here doesn't exactly match (for the appropriate types)
+    // the on-disk format. This is to save memory.
+    union {
+        // Used for TD_INTERFACE_IS_TYPE.
+        struct {
+            uint8_t argnum;
+        } interface_is;
+
+        // Used for TD_PSTRING_SIZE_IS, TD_PWSTRING_SIZE_IS.
+        struct {
+            uint8_t argnum;
+            //uint8_t argnum2;          // Present on disk, omitted here.
+        } pstring_is;
 
-    // This field exists (for the appropriate types) in the on-disk format. But
-    // it isn't used so we don't allocated space for it in memory.
-    //uint8_t argnum2;
+        // Used for TD_ARRAY.
+        struct {
+            uint8_t argnum;
+            //uint8_t argnum2;          // Present on disk, omitted here.
+            uint8_t additional_type;    // uint16_t on disk, uint8_t here;
+                                        // in practice it never exceeds 20.
+        } array;
 
-    union {
-        uint16_t iface;           // used for TD_INTERFACE_TYPE
-        uint16_t additional_type; // used for TD_ARRAY
-    } type;
+        // Used for TD_INTERFACE_TYPE.
+        struct {
+            // We store the 16-bit iface value as two 8-bit values in order to
+            // avoid 16-bit alignment requirements for XPTTypeDescriptor, which
+            // reduces its size and also the size of XPTParamDescriptor.
+            uint8_t iface_hi8;
+            uint8_t iface_lo8;
+        } iface;
+    } u;
 };
 
 /*
  * A ConstDescriptor is a variable-size record that records the name and 
  * value of a scoped interface constant. 
  *
  * The types of the method parameter are restricted to the following subset 
  * of TypeDescriptors: