bug 703100 - pt 5 - optimize allocation of gfxTextRun objects to avoid separate allocation for CompressedGlyph records. r=roc
authorJonathan Kew <jfkthame@gmail.com>
Thu, 05 Jan 2012 11:54:45 +0000
changeset 85105 daadf6827faaf55146463b7fdf68956fa1b78887
parent 85104 433b37e097c724cd18e2e5e16b243d0c6a37cb9a
child 85106 7b612dae71ec81d28e265f4414d43e80242077f5
push id805
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 18:17:35 +0000
treeherdermozilla-aurora@6fb3bf232436 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs703100
milestone12.0a1
bug 703100 - pt 5 - optimize allocation of gfxTextRun objects to avoid separate allocation for CompressedGlyph records. r=roc
gfx/thebes/gfxFont.cpp
gfx/thebes/gfxFont.h
layout/generic/nsTextRunTransformations.cpp
layout/generic/nsTextRunTransformations.h
--- a/gfx/thebes/gfxFont.cpp
+++ b/gfx/thebes/gfxFont.cpp
@@ -3910,68 +3910,68 @@ AccountStorageForTextRun(gfxTextRun *aTe
     // Ignores detailed glyphs... we don't know when those have been constructed
     // Also ignores gfxSkipChars dynamic storage (which won't be anything
     // for preformatted text)
     // Also ignores GlyphRun array, again because it hasn't been constructed
     // by the time this gets called. If there's only one glyphrun that's stored
     // directly in the textrun anyway so no additional overhead.
     PRUint32 length = aTextRun->GetLength();
     PRInt32 bytes = length * sizeof(gfxTextRun::CompressedGlyph);
-    if (aTextRun->GetFlags() & gfxTextRunFactory::TEXT_IS_PERSISTENT) {
-      bytes += length * ((aTextRun->GetFlags() & gfxTextRunFactory::TEXT_IS_8BIT) ? 1 : 2);
-      bytes += sizeof(gfxTextRun::CompressedGlyph) - 1;
-      bytes &= ~(sizeof(gfxTextRun::CompressedGlyph) - 1);
-    }
     bytes += sizeof(gfxTextRun);
     gTextRunStorage += bytes*aSign;
     gTextRunStorageHighWaterMark = NS_MAX(gTextRunStorageHighWaterMark, gTextRunStorage);
 }
 #endif
 
 // Helper for textRun creation to preallocate storage for glyph records;
 // this function returns a pointer to the newly-allocated glyph storage.
 // Returns nsnull if allocation fails.
-gfxTextRun::CompressedGlyph *
-gfxTextRun::AllocateStorage(PRUint32 aLength)
+void *
+gfxTextRun::AllocateStorageForTextRun(size_t aSize, PRUint32 aLength)
 {
-    // allocate the storage we need, returning nsnull on failure rather than
-    // throwing an exception (because web content can create huge runs)
-    CompressedGlyph *storage = new (std::nothrow) CompressedGlyph[aLength];
+    // Allocate the storage we need, returning nsnull on failure rather than
+    // throwing an exception (because web content can create huge runs).
+    void *storage = moz_malloc(aSize + aLength * sizeof(CompressedGlyph));
     if (!storage) {
-        NS_WARNING("failed to allocate glyph storage for text run!");
+        NS_WARNING("failed to allocate storage for text run!");
         return nsnull;
     }
 
+    // Initialize the glyph storage (beyond aSize) to zero
+    memset(reinterpret_cast<char*>(storage) + aSize, 0,
+           aLength * sizeof(CompressedGlyph));
+
     return storage;
 }
 
 gfxTextRun *
 gfxTextRun::Create(const gfxTextRunFactory::Parameters *aParams, const void *aText,
                    PRUint32 aLength, gfxFontGroup *aFontGroup, PRUint32 aFlags)
 {
-    CompressedGlyph *glyphStorage = AllocateStorage(aLength);
-    if (!glyphStorage) {
+    void *storage = AllocateStorageForTextRun(sizeof(gfxTextRun), aLength);
+    if (!storage) {
         return nsnull;
     }
 
-    return new gfxTextRun(aParams, aText, aLength, aFontGroup, aFlags, glyphStorage);
+    return new (storage) gfxTextRun(aParams, aText, aLength, aFontGroup, aFlags);
 }
 
 gfxTextRun::gfxTextRun(const gfxTextRunFactory::Parameters *aParams, const void *aText,
-                       PRUint32 aLength, gfxFontGroup *aFontGroup, PRUint32 aFlags,
-                       CompressedGlyph *aGlyphStorage)
-  : mCharacterGlyphs(aGlyphStorage),
-    mUserData(aParams->mUserData),
+                       PRUint32 aLength, gfxFontGroup *aFontGroup, PRUint32 aFlags)
+  : mUserData(aParams->mUserData),
     mFontGroup(aFontGroup),
     mAppUnitsPerDevUnit(aParams->mAppUnitsPerDevUnit),
     mFlags(aFlags), mCharacterCount(aLength)
 {
     NS_ASSERTION(mAppUnitsPerDevUnit != 0, "Invalid app unit scale");
     MOZ_COUNT_CTOR(gfxTextRun);
     NS_ADDREF(mFontGroup);
+
+    mCharacterGlyphs = reinterpret_cast<CompressedGlyph*>(this + 1);
+
     if (aParams->mSkipChars) {
         mSkipChars.TakeFrom(aParams->mSkipChars);
     }
 
 #ifdef DEBUG_TEXT_RUN_STORAGE_METRICS
     AccountStorageForTextRun(this, 1);
 #endif
 
@@ -3983,44 +3983,39 @@ gfxTextRun::~gfxTextRun()
 #ifdef DEBUG_TEXT_RUN_STORAGE_METRICS
     AccountStorageForTextRun(this, -1);
 #endif
 #ifdef DEBUG
     // Make it easy to detect a dead text run
     mFlags = 0xFFFFFFFF;
 #endif
 
-    // this will also delete the text, if it is owned by the run,
-    // because we merge the storage allocations
-    delete [] mCharacterGlyphs;
-
     NS_RELEASE(mFontGroup);
     MOZ_COUNT_DTOR(gfxTextRun);
 }
 
 bool
 gfxTextRun::SetPotentialLineBreaks(PRUint32 aStart, PRUint32 aLength,
                                    PRUint8 *aBreakBefore,
                                    gfxContext *aRefContext)
 {
     NS_ASSERTION(aStart + aLength <= mCharacterCount, "Overflow");
 
-    if (!mCharacterGlyphs)
-        return true;
     PRUint32 changed = 0;
     PRUint32 i;
+    CompressedGlyph *charGlyphs = mCharacterGlyphs + aStart;
     for (i = 0; i < aLength; ++i) {
         PRUint8 canBreak = aBreakBefore[i];
-        if (canBreak && !mCharacterGlyphs[aStart + i].IsClusterStart()) {
+        if (canBreak && !charGlyphs[i].IsClusterStart()) {
             // This can happen ... there is no guarantee that our linebreaking rules
             // align with the platform's idea of what constitutes a cluster.
             NS_WARNING("Break suggested inside cluster!");
             canBreak = CompressedGlyph::FLAG_BREAK_TYPE_NONE;
         }
-        changed |= mCharacterGlyphs[aStart + i].SetCanBreakBefore(canBreak);
+        changed |= charGlyphs[i].SetCanBreakBefore(canBreak);
     }
     return changed != 0;
 }
 
 gfxTextRun::LigatureData
 gfxTextRun::ComputeLigatureData(PRUint32 aPartStart, PRUint32 aPartEnd,
                                 PropertyProvider *aProvider)
 {
@@ -4850,19 +4845,20 @@ gfxTextRun::SanitizeGlyphRuns()
     if (mGlyphRuns.Length() <= 1)
         return;
 
     // If any glyph run starts with ligature-continuation characters, we need to advance it
     // to the first "real" character to avoid drawing partial ligature glyphs from wrong font
     // (seen with U+FEFF in reftest 474417-1, as Core Text eliminates the glyph, which makes
     // it appear as if a ligature has been formed)
     PRInt32 i, lastRunIndex = mGlyphRuns.Length() - 1;
+    const CompressedGlyph *charGlyphs = mCharacterGlyphs;
     for (i = lastRunIndex; i >= 0; --i) {
         GlyphRun& run = mGlyphRuns[i];
-        while (mCharacterGlyphs[run.mCharacterOffset].IsLigatureContinuation() &&
+        while (charGlyphs[run.mCharacterOffset].IsLigatureContinuation() &&
                run.mCharacterOffset < mCharacterCount) {
             run.mCharacterOffset++;
         }
         // if the run has become empty, eliminate it
         if ((i < lastRunIndex &&
              run.mCharacterOffset >= mGlyphRuns[i+1].mCharacterOffset) ||
             (i == lastRunIndex && run.mCharacterOffset == mCharacterCount)) {
             mGlyphRuns.RemoveElementAt(i);
@@ -4884,20 +4880,16 @@ gfxTextRun::CountMissingGlyphs()
     return count;
 }
 
 gfxTextRun::DetailedGlyph *
 gfxTextRun::AllocateDetailedGlyphs(PRUint32 aIndex, PRUint32 aCount)
 {
     NS_ASSERTION(aIndex < mCharacterCount, "Index out of range");
 
-    if (!mCharacterGlyphs) {
-        return nsnull;
-    }
-
     if (!mDetailedGlyphs) {
         mDetailedGlyphs = new DetailedGlyphStore();
     }
 
     DetailedGlyph *details = mDetailedGlyphs->Allocate(aIndex, aCount);
     if (!details) {
         mCharacterGlyphs[aIndex].SetMissing(0);
         return nsnull;
@@ -4988,36 +4980,38 @@ gfxTextRun::CopyGlyphDataFrom(gfxTextRun
     NS_ASSERTION(aDest + aLength <= GetLength(),
                  "Destination substring out of range");
 
     if (aSource->mSkipDrawing) {
         mSkipDrawing = true;
     }
 
     // Copy base glyph data, and DetailedGlyph data where present
+    const CompressedGlyph *srcGlyphs = aSource->mCharacterGlyphs + aStart;
+    CompressedGlyph *dstGlyphs = mCharacterGlyphs + aDest;
     for (PRUint32 i = 0; i < aLength; ++i) {
-        CompressedGlyph g = aSource->mCharacterGlyphs[i + aStart];
-        g.SetCanBreakBefore(mCharacterGlyphs[i + aDest].CanBreakBefore());
+        CompressedGlyph g = srcGlyphs[i];
+        g.SetCanBreakBefore(dstGlyphs[i].CanBreakBefore());
         if (!g.IsSimpleGlyph()) {
             PRUint32 count = g.GetGlyphCount();
             if (count > 0) {
                 DetailedGlyph *dst = AllocateDetailedGlyphs(i + aDest, count);
                 if (dst) {
                     DetailedGlyph *src = aSource->GetDetailedGlyphs(i + aStart);
                     if (src) {
                         ::memcpy(dst, src, count * sizeof(DetailedGlyph));
                     } else {
                         g.SetMissing(0);
                     }
                 } else {
                     g.SetMissing(0);
                 }
             }
         }
-        mCharacterGlyphs[i + aDest] = g;
+        dstGlyphs[i] = g;
     }
 
     // Copy glyph runs
     GlyphRunIterator iter(aSource, aStart, aLength);
 #ifdef DEBUG
     gfxFont *lastFont = nsnull;
 #endif
     while (iter.NextRun()) {
@@ -5214,33 +5208,31 @@ gfxTextRun::ClusterIterator::ClusterAdva
     return mTextRun->GetAdvanceWidth(mCurrentChar, ClusterLength(), aProvider);
 }
 
 size_t
 gfxTextRun::SizeOfExcludingThis(nsMallocSizeOfFun aMallocSizeOf)
 {
     // The second arg is how much gfxTextRun::AllocateStorage would have
     // allocated.
-    size_t total =
-        aMallocSizeOf(mCharacterGlyphs,
-                      sizeof(CompressedGlyph) * mCharacterCount);
+    size_t total = mGlyphRuns.SizeOfExcludingThis(aMallocSizeOf);
 
     if (mDetailedGlyphs) {
         total += mDetailedGlyphs->SizeOfIncludingThis(aMallocSizeOf);
     }
 
-    total += mGlyphRuns.SizeOfExcludingThis(aMallocSizeOf);
-
     return total;
 }
 
 size_t
 gfxTextRun::SizeOfIncludingThis(nsMallocSizeOfFun aMallocSizeOf)
 {
-    return aMallocSizeOf(this, sizeof(gfxTextRun)) +
+    // The second arg is how much gfxTextRun::AllocateStorageForTextRun would
+    // have allocated, given the character count of this run.
+    return aMallocSizeOf(this, sizeof(gfxTextRun) + sizeof(CompressedGlyph) * GetLength()) +
            SizeOfExcludingThis(aMallocSizeOf);
 }
 
 
 #ifdef DEBUG
 void
 gfxTextRun::Dump(FILE* aOutput) {
     if (!aOutput) {
--- a/gfx/thebes/gfxFont.h
+++ b/gfx/thebes/gfxFont.h
@@ -1915,19 +1915,18 @@ public:
         mAgeCounter = 0;
     }
     PRUint32 IncrementAge() {
         return ++mAgeCounter;
     }
 
     void SetSimpleGlyph(PRUint32 aCharIndex, CompressedGlyph aGlyph) {
         NS_ASSERTION(aGlyph.IsSimpleGlyph(), "Should be a simple glyph here");
-        if (mCharacterGlyphs) {
-            mCharacterGlyphs[aCharIndex] = aGlyph;
-        }
+        NS_ASSERTION(mCharacterGlyphs, "mCharacterGlyphs pointer is null!");
+        mCharacterGlyphs[aCharIndex] = aGlyph;
     }
 
     void SetGlyphs(PRUint32 aCharIndex, CompressedGlyph aGlyph,
                    const DetailedGlyph *aGlyphs);
 
     void SetMissingGlyph(PRUint32 aIndex, PRUint32 aChar, gfxFont *aFont);
 
     void SetIsSpace(PRUint32 aIndex) {
@@ -2172,16 +2171,22 @@ private:
 class THEBES_API gfxTextRun {
 public:
     // we use the same glyph storage as gfxShapedWord, to facilitate copying
     // glyph data from shaped words into text runs as needed
     typedef gfxShapedWord::CompressedGlyph    CompressedGlyph;
     typedef gfxShapedWord::DetailedGlyph      DetailedGlyph;
     typedef gfxShapedWord::DetailedGlyphStore DetailedGlyphStore;
 
+    // Override operator delete to properly free the object that was
+    // allocated via moz_malloc.
+    void operator delete(void* p) {
+        moz_free(p);
+    }
+
     virtual ~gfxTextRun();
 
     typedef gfxFont::RunMetrics Metrics;
 
     // Public textrun API for general use
 
     bool IsClusterStart(PRUint32 aPos) {
         NS_ASSERTION(aPos < mCharacterCount, "aPos out of range");
@@ -2564,27 +2569,25 @@ public:
     nsresult AddGlyphRun(gfxFont *aFont, PRUint8 aMatchType,
                          PRUint32 aStartCharIndex, bool aForceNewRun);
     void ResetGlyphRuns() { mGlyphRuns.Clear(); }
     void SortGlyphRuns();
     void SanitizeGlyphRuns();
 
     // Call the following glyph-setters during initialization or during reshaping
     // only. It is OK to overwrite existing data for a character.
+    void SetSimpleGlyph(PRUint32 aCharIndex, CompressedGlyph aGlyph) {
+        NS_ASSERTION(aGlyph.IsSimpleGlyph(), "Should be a simple glyph here");
+        mCharacterGlyphs[aCharIndex] = aGlyph;
+    }
     /**
      * Set the glyph data for a character. aGlyphs may be null if aGlyph is a
      * simple glyph or has no associated glyphs. If non-null the data is copied,
      * the caller retains ownership.
      */
-    void SetSimpleGlyph(PRUint32 aCharIndex, CompressedGlyph aGlyph) {
-        NS_ASSERTION(aGlyph.IsSimpleGlyph(), "Should be a simple glyph here");
-        if (mCharacterGlyphs) {
-            mCharacterGlyphs[aCharIndex] = aGlyph;
-        }
-    }
     void SetGlyphs(PRUint32 aCharIndex, CompressedGlyph aGlyph,
                    const DetailedGlyph *aGlyphs);
     void SetMissingGlyph(PRUint32 aCharIndex, PRUint32 aUnicodeChar);
     void SetSpaceGlyph(gfxFont *aFont, gfxContext *aContext, PRUint32 aCharIndex);
 
     // Set the glyph data for the given character index to the font's
     // space glyph, IF this can be done as a "simple" glyph record
     // (not requiring a DetailedGlyph entry). This avoids the need to call
@@ -2713,30 +2716,36 @@ public:
     }
 
 #ifdef DEBUG
     void Dump(FILE* aOutput);
 #endif
 
 protected:
     /**
-     * Initializes the textrun to blank.
-     * @param aGlyphStorage preallocated array of CompressedGlyph[aLength]
-     * for the textrun to use; if aText is not persistent, then it has also
-     * been appended to this array, so it must NOT be freed separately.
+     * Create a textrun, and set its mCharacterGlyphs to point immediately
+     * after the base object; this is ONLY used in conjunction with placement
+     * new, after allocating a block large enough for the glyph records to
+     * follow the base textrun object.
      */
     gfxTextRun(const gfxTextRunFactory::Parameters *aParams, const void *aText,
-               PRUint32 aLength, gfxFontGroup *aFontGroup, PRUint32 aFlags,
-               CompressedGlyph *aGlyphStorage);
+               PRUint32 aLength, gfxFontGroup *aFontGroup, PRUint32 aFlags);
 
     /**
      * Helper for the Create() factory method to allocate the required
-     * glyph storage.
+     * glyph storage for a textrun object with the basic size aSize,
+     * plus room for aLength glyph records.
      */
-    static CompressedGlyph* AllocateStorage(PRUint32 aLength);
+    static void* AllocateStorageForTextRun(size_t aSize, PRUint32 aLength);
+
+    // All our glyph data is in logical order, not visual.
+    // Space for mCharacterGlyphs is allocated fused with the textrun object,
+    // and then the constructor sets the pointer to the beginning of this
+    // storage area. Thus, this pointer must NOT be freed!
+    CompressedGlyph  *mCharacterGlyphs;
 
 private:
     // **** general helpers **** 
 
     // Allocate aCount DetailedGlyphs for the given index
     DetailedGlyph *AllocateDetailedGlyphs(PRUint32 aCharIndex, PRUint32 aCount);
 
     // Get the total advance for a range of glyphs.
@@ -2784,23 +2793,16 @@ private:
                                  Metrics *aMetrics);
 
     // **** drawing helper ****
     void DrawGlyphs(gfxFont *aFont, gfxContext *aContext, bool aDrawToPath,
                     gfxPoint *aPt, PRUint32 aStart, PRUint32 aEnd,
                     PropertyProvider *aProvider,
                     PRUint32 aSpacingStart, PRUint32 aSpacingEnd);
 
-    // All our glyph data is in logical order, not visual.
-    // mCharacterGlyphs is allocated by the factory that creates the textrun,
-    // to avoid the possibility of failure during the constructor;
-    // however, ownership passes to the textrun during construction and so
-    // it must be deleted in the destructor.
-    CompressedGlyph*                mCharacterGlyphs;
-
     nsAutoPtr<DetailedGlyphStore>   mDetailedGlyphs;
 
     // XXX this should be changed to a GlyphRun plus a maybe-null GlyphRun*,
     // for smaller size especially in the super-common one-glyphrun case
     nsAutoTArray<GlyphRun,1>        mGlyphRuns;
 
     void             *mUserData;
     gfxFontGroup     *mFontGroup; // addrefed
--- a/layout/generic/nsTextRunTransformations.cpp
+++ b/layout/generic/nsTextRunTransformations.cpp
@@ -54,24 +54,24 @@ nsTransformedTextRun::Create(const gfxTe
                              gfxFontGroup* aFontGroup,
                              const PRUnichar* aString, PRUint32 aLength,
                              const PRUint32 aFlags, nsStyleContext** aStyles,
                              bool aOwnsFactory)
 {
   NS_ASSERTION(!(aFlags & gfxTextRunFactory::TEXT_IS_8BIT),
                "didn't expect text to be marked as 8-bit here");
 
-  CompressedGlyph *glyphStorage = AllocateStorage(aLength);
-  if (!glyphStorage) {
+  void *storage = AllocateStorageForTextRun(sizeof(nsTransformedTextRun), aLength);
+  if (!storage) {
     return nsnull;
   }
 
-  return new nsTransformedTextRun(aParams, aFactory, aFontGroup,
-                                  aString, aLength,
-                                  aFlags, aStyles, aOwnsFactory, glyphStorage);
+  return new (storage) nsTransformedTextRun(aParams, aFactory, aFontGroup,
+                                            aString, aLength,
+                                            aFlags, aStyles, aOwnsFactory);
 }
 
 void
 nsTransformedTextRun::SetCapitalization(PRUint32 aStart, PRUint32 aLength,
                                         bool* aCapitalization,
                                         gfxContext* aRefContext)
 {
   if (mCapitalize.IsEmpty()) {
@@ -109,17 +109,18 @@ nsTransformedTextRun::SizeOfExcludingThi
     total += aMallocSizeOf(mFactory, 0);
   }
   return total;
 }
 
 size_t
 nsTransformedTextRun::SizeOfIncludingThis(nsMallocSizeOfFun aMallocSizeOf)
 {
-  return aMallocSizeOf(this, sizeof(nsTransformedTextRun)) +
+  return aMallocSizeOf(this, sizeof(nsTransformedTextRun) +
+                             GetLength() * sizeof(CompressedGlyph)) +
          SizeOfExcludingThis(aMallocSizeOf);
 }
 
 nsTransformedTextRun*
 nsTransformingTextRunFactory::MakeTextRun(const PRUnichar* aString, PRUint32 aLength,
                                           const gfxTextRunFactory::Parameters* aParams,
                                           gfxFontGroup* aFontGroup, PRUint32 aFlags,
                                           nsStyleContext** aStyles, bool aOwnsFactory)
--- a/layout/generic/nsTextRunTransformations.h
+++ b/layout/generic/nsTextRunTransformations.h
@@ -144,22 +144,23 @@ public:
   bool                                mNeedsRebuild;
 
 private:
   nsTransformedTextRun(const gfxTextRunFactory::Parameters* aParams,
                        nsTransformingTextRunFactory* aFactory,
                        gfxFontGroup* aFontGroup,
                        const PRUnichar* aString, PRUint32 aLength,
                        const PRUint32 aFlags, nsStyleContext** aStyles,
-                       bool aOwnsFactory,
-                       CompressedGlyph *aGlyphStorage)
-    : gfxTextRun(aParams, aString, aLength, aFontGroup, aFlags, aGlyphStorage),
+                       bool aOwnsFactory)
+    : gfxTextRun(aParams, aString, aLength, aFontGroup, aFlags),
       mFactory(aFactory), mString(aString, aLength),
       mOwnsFactory(aOwnsFactory), mNeedsRebuild(true)
   {
+    mCharacterGlyphs = reinterpret_cast<CompressedGlyph*>(this + 1);
+
     PRUint32 i;
     for (i = 0; i < aLength; ++i) {
       mStyles.AppendElement(aStyles[i]);
     }
   }
 };
 
 #endif /*NSTEXTRUNTRANSFORMATIONS_H_*/