Bug 681161 - Shrink nsCSSCompressedDataBlock on 64-bit platforms. r=dbaron.
authorNicholas Nethercote <nnethercote@mozilla.com>
Thu, 25 Aug 2011 23:05:55 -0700
changeset 75936 c70c539f2e937ebe5bb82550cfba6766cdf5313d
parent 75935 171e1d0b8cb32c65987c5d7b5149d3c9b18a7543
child 75937 e5adec25155d6c697473762d464b03aa4879561e
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewersdbaron
bugs681161
milestone9.0a1
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();