Bug 681161 - Shrink nsCSSCompressedDataBlock on 64-bit platforms. r=dbaron.
authorNicholas Nethercote <nnethercote@mozilla.com>
Thu, 25 Aug 2011 23:05:55 -0700
changeset 75913 c70c539f2e93
parent 75912 171e1d0b8cb3
child 75914 e5adec25155d
push id1499
push usernnethercote@mozilla.com
push dateFri, 26 Aug 2011 06:06:14 +0000
treeherdermozilla-inbound@c70c539f2e93 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs681161
milestone9.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 681161 - Shrink nsCSSCompressedDataBlock on 64-bit platforms. r=dbaron.
layout/style/nsCSSDataBlock.cpp
layout/style/nsCSSDataBlock.h
--- a/layout/style/nsCSSDataBlock.cpp
+++ b/layout/style/nsCSSDataBlock.cpp
@@ -43,27 +43,16 @@
 #include "nsCSSDataBlock.h"
 #include "mozilla/css/Declaration.h"
 #include "nsRuleData.h"
 #include "nsStyleSet.h"
 #include "nsStyleContext.h"
 
 namespace css = mozilla::css;
 
-/*
- * nsCSSCompressedDataBlock holds property-value pairs corresponding
- * to CSS declaration blocks.  Each pair is stored in a CDBValueStorage
- * object; these objects form an array at the end of the data block.
- */
-
-struct CDBValueStorage {
-    nsCSSProperty property;
-    nsCSSValue value;
-};
-
 enum {
     CDBValueStorage_advance = sizeof(CDBValueStorage)
 };
 
 /*
  * Define a bunch of utility functions for getting the property or any
  * of the value types when the cursor is at the beginning of the storage
  * for the property-value pair.  The versions taking a non-const cursor
@@ -282,17 +271,17 @@ nsCSSCompressedDataBlock::Clone() const
         const nsCSSValue* val = ValueAtCursor(cursor);
         nsCSSValue *result_val = ValueAtCursor(result_cursor);
         new (result_val) nsCSSValue(*val);
         cursor += CDBValueStorage_advance;
         result_cursor +=  CDBValueStorage_advance;
     }
     NS_ABORT_IF_FALSE(cursor == cursor_end, "inconsistent data");
 
-    result->mBlockEnd = result_cursor;
+    result->SetBlockEnd(result_cursor);
     result->mStyleBits = mStyleBits;
     NS_ABORT_IF_FALSE(result->DataSize() == DataSize(), "wrong size");
 
     return result.forget();
 }
 
 nsCSSCompressedDataBlock::~nsCSSCompressedDataBlock()
 {
@@ -309,17 +298,17 @@ nsCSSCompressedDataBlock::~nsCSSCompress
     }
     NS_ABORT_IF_FALSE(cursor == cursor_end, "inconsistent data");
 }
 
 /* static */ nsCSSCompressedDataBlock*
 nsCSSCompressedDataBlock::CreateEmptyBlock()
 {
     nsCSSCompressedDataBlock *result = new(0) nsCSSCompressedDataBlock();
-    result->mBlockEnd = result->Block();
+    result->SetBlockEnd(result->Block());
     return result;
 }
 
 /*****************************************************************************/
 
 nsCSSExpandedDataBlock::nsCSSExpandedDataBlock()
 {
     AssertInitialState();
@@ -358,17 +347,17 @@ nsCSSExpandedDataBlock::DoExpand(nsCSSCo
         dest->~nsCSSValue();
 #endif
         memcpy(dest, val, sizeof(nsCSSValue));
         cursor += CDBValueStorage_advance;
     }
     NS_ABORT_IF_FALSE(cursor == cursor_end, "inconsistent data");
 
     // Don't destroy remnants of what we just copied
-    aBlock->mBlockEnd = aBlock->Block();
+    aBlock->SetBlockEnd(aBlock->Block());
     delete aBlock;
 }
 
 void
 nsCSSExpandedDataBlock::Expand(nsCSSCompressedDataBlock *aNormalBlock,
                                nsCSSCompressedDataBlock *aImportantBlock)
 {
     NS_ABORT_IF_FALSE(aNormalBlock, "unexpected null block");
@@ -452,22 +441,22 @@ nsCSSExpandedDataBlock::Compress(nsCSSCo
             memcpy(&storage->value, val, sizeof(nsCSSValue));
             new (val) nsCSSValue();
             cursor += CDBValueStorage_advance;
             result->mStyleBits |=
                 nsCachedStyleData::GetBitForSID(nsCSSProps::kSIDTable[iProp]);
         }
     }
 
-    result_normal->mBlockEnd = cursor_normal;
+    result_normal->SetBlockEnd(cursor_normal);
     NS_ABORT_IF_FALSE(result_normal->DataSize() == ptrdiff_t(size.normal),
                       "size miscalculation");
 
     if (result_important) {
-        result_important->mBlockEnd = cursor_important;
+        result_important->SetBlockEnd(cursor_important);
         NS_ABORT_IF_FALSE(result_important->DataSize() ==
                           ptrdiff_t(size.important),
                           "size miscalculation");
     }
 
     ClearSets();
     AssertInitialState();
     *aNormalBlock = result_normal.forget();
--- a/layout/style/nsCSSDataBlock.h
+++ b/layout/style/nsCSSDataBlock.h
@@ -50,16 +50,26 @@ struct nsRuleData;
 class nsCSSExpandedDataBlock;
 
 namespace mozilla {
 namespace css {
 class Declaration;
 }
 }
 
+/*
+ * nsCSSCompressedDataBlock holds property-value pairs corresponding
+ * to CSS declaration blocks.  Each pair is stored in a CDBValueStorage
+ * object; these objects form an array at the end of the data block.
+ */
+struct CDBValueStorage {
+    nsCSSProperty property;
+    nsCSSValue value;
+};
+
 /**
  * An |nsCSSCompressedDataBlock| holds a usually-immutable chunk of
  * property-value data for a CSS declaration block (which we misname a
  * |css::Declaration|).  Mutation is accomplished through
  * |nsCSSExpandedDataBlock| or in some cases via direct slot access.
  */
 class nsCSSCompressedDataBlock {
 private:
@@ -107,42 +117,55 @@ public:
     nsCSSCompressedDataBlock* Clone() const;
 
     /**
      * Create a new nsCSSCompressedDataBlock holding no declarations.
      */
     static nsCSSCompressedDataBlock* CreateEmptyBlock();
 
 private:
-    PRInt32 mStyleBits; // the structs for which we have data, according to
-                        // |nsCachedStyleData::GetBitForSID|.
-
-    enum { block_chars = 4 }; // put 4 chars in the definition of the class
-                              // to ensure size not inflated by alignment
-
     void* operator new(size_t aBaseSize, size_t aDataSize) {
-        // subtract off the extra size to store |mBlock_|
-        return ::operator new(aBaseSize + aDataSize -
-                              sizeof(char) * block_chars);
+        NS_ABORT_IF_FALSE(aBaseSize == sizeof(nsCSSCompressedDataBlock),
+                          "unexpected size for nsCSSCompressedDataBlock");
+        return ::operator new(aBaseSize + aDataSize);
     }
 
     /**
      * Delete all the data stored in this block, and the block itself.
      */
     void Destroy();
 
-    char* mBlockEnd; // the byte after the last valid byte
-    char mBlock_[block_chars]; // must be the last member!
+    PRInt32 mStyleBits; // the structs for which we have data, according to
+                        // |nsCachedStyleData::GetBitForSID|.
+    PRUint32 mDataSize;
+    // CDBValueStorage elements are stored after these fields.  Space for them
+    // is allocated in |operator new| above.  The static assertions following
+    // this class make sure that the CDBValueStorage elements are aligned
+    // appropriately.
+    
+    char* Block() { return (char*)this + sizeof(*this); }
+    char* BlockEnd() { return Block() + mDataSize; }
+    const char* Block() const { return (char*)this + sizeof(*this); }
+    const char* BlockEnd() const { return Block() + mDataSize; }
+    void SetBlockEnd(char *blockEnd) { 
+        /*
+         * Note:  if we ever change nsCSSDeclaration to store the declarations
+         * in order and also store repeated declarations of the same property,
+         * then we need to worry about checking for integer overflow here.
+         */
+        NS_ABORT_IF_FALSE(size_t(blockEnd - Block()) <= size_t(PR_UINT32_MAX),
+                          "overflow of mDataSize");
+        mDataSize = PRUint32(blockEnd - Block());
+    }
+    ptrdiff_t DataSize() const { return mDataSize; }
+};
 
-    char* Block() { return mBlock_; }
-    char* BlockEnd() { return mBlockEnd; }
-    const char* Block() const { return mBlock_; }
-    const char* BlockEnd() const { return mBlockEnd; }
-    ptrdiff_t DataSize() const { return BlockEnd() - Block(); }
-};
+/* Make sure the CDBValueStorage elements are aligned appropriately. */
+PR_STATIC_ASSERT(sizeof(nsCSSCompressedDataBlock) == 8);
+PR_STATIC_ASSERT(NS_ALIGNMENT_OF(CDBValueStorage) <= 8); 
 
 class nsCSSExpandedDataBlock {
     friend class nsCSSCompressedDataBlock;
 
 public:
     nsCSSExpandedDataBlock();
     ~nsCSSExpandedDataBlock();