Allocate SkPictureFlat::fScratch lazily.

src/core/SkPictureFlat.h

 
 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 #ifndef SkPictureFlat_DEFINED
 #define SkPictureFlat_DEFINED
 
@@ skipping 358 matching lines @@
     template <typename T, typename Traits, int kScratchSizeGuess> friend class SkFlatDictionary;
 };
 
 template <typename T, typename Traits, int kScratchSizeGuess=0>
 class SkFlatDictionary {
     static const size_t kWriteBufferGrowthBytes = 1024;
 
 public:
     explicit SkFlatDictionary(SkFlatController* controller)
     : fController(SkRef(controller))
     , fScratchSize(kScratchSizeGuess)

[mtklein, 2014/01/09 14:38:58]

For consistency, let's make this fScratchSize(0) then set it to
kScratchSizeGuess just before fScratch = AllocScrach(fScratchSize) in
lazyBufferInit().  It feels spooky to have fScratchSize inconsistent when we
start.

[tomhudson, 2014/01/09 14:49:32]

Done.

-    , fScratch(AllocScratch(fScratchSize))
+    , fScratch(NULL)
     , fWriteBuffer(kWriteBufferGrowthBytes)
-    , fWriteBufferReady(false) {
+    , fBuffersReady(false) {

[mtklein, 2014/01/09 14:38:58]

Suggestion: fReady and lazyInit() ?

[tomhudson, 2014/01/09 14:49:32]

Done.

         this->reset();
     }
 
     /**
      * Clears the dictionary of all entries. However, it does NOT free the
      * memory that was allocated for each entry (that's owned by controller).
      */
     void reset() {
         fIndexedData.rewind();
         // TODO(mtklein): There's no reason to have the index start from 1.  Clean this up.
@@ skipping 115 matching lines @@
         SkASSERT(SkIsAlign4(flatDataSize));
         return sizeof(SkFlatData) + flatDataSize;
     }
 
     // Allocate a new scratch SkFlatData.  Must be sk_freed.
     static SkFlatData* AllocScratch(size_t scratchSize) {
         return (SkFlatData*) sk_malloc_throw(SizeWithPadding(scratchSize));
     }
 
     // We have to delay fWriteBuffer's initialization until its first use; fController might not
-    // be fully set up by the time we get it in the constructor.
-    void lazyWriteBufferInit() {
-        if (fWriteBufferReady) {
+    // be fully set up by the time we get it in the constructor. We also delay fScratch to

[mtklein, 2014/01/09 14:38:58]

delay -> delay allocating?

[tomhudson, 2014/01/09 14:49:32]

Done.

+    // avoid unnecessary heap allocations, since we're paying the price of the conditional
+    // anyway.
+    void lazyBufferInit() {
+        if (fBuffersReady) {
             return;
         }
+
+        fScratch = AllocScratch(fScratchSize);
+
         // Without a bitmap heap, we'll flatten bitmaps into paints.  That's never what you want.
         SkASSERT(fController->getBitmapHeap() != NULL);
         fWriteBuffer.setBitmapHeap(fController->getBitmapHeap());
         fWriteBuffer.setTypefaceRecorder(fController->getTypefaceSet());
         fWriteBuffer.setNamedFactoryRecorder(fController->getNamedFactorySet());
         fWriteBuffer.setFlags(fController->getWriteBufferFlags());
-        fWriteBufferReady = true;
+        fBuffersReady = true;
     }
 
     // As findAndReturnFlat, but returns a mutable pointer for internal use.
     SkFlatData* findAndReturnMutableFlat(const T& element) {
         // Only valid until the next call to resetScratch().
         const SkFlatData& scratch = this->resetScratch(element, fNextIndex);
 
         SkFlatData* candidate = fHash.find(scratch);
         if (candidate != NULL) return candidate;
 
         SkFlatData* detached = this->detachScratch();
         fHash.add(detached);
         *fIndexedData.insert(fNextIndex) = detached;
         fNextIndex++;
         return detached;
     }
 
     // This reference is valid only until the next call to resetScratch() or detachScratch().
     const SkFlatData& resetScratch(const T& element, int index) {
-        this->lazyWriteBufferInit();
+        this->lazyBufferInit();
 
         // Flatten element into fWriteBuffer (using fScratch as storage).
         fWriteBuffer.reset(fScratch->data(), fScratchSize);
         Traits::flatten(fWriteBuffer, element);
         const size_t bytesWritten = fWriteBuffer.bytesWritten();
 
         // If all the flattened bytes fit into fScratch, we can skip a call to writeToMemory.
         if (!fWriteBuffer.wroteOnlyToStorage()) {
             SkASSERT(bytesWritten > fScratchSize);
             // It didn't all fit.  Copy into a larger replacement SkFlatData.
@@ skipping 10 matching lines @@
         // The data is in fScratch now but we need to stamp its header.
         fScratch->stampHeader(index, bytesWritten);
         return *fScratch;
     }
 
     // This result is owned by fController and lives as long as it does (unless unalloc'd).
     SkFlatData* detachScratch() {
         // Allocate a new SkFlatData exactly big enough to hold our current scratch.
         // We use the controller for this allocation to extend the allocation's lifetime and allow
         // the controller to do whatever memory management it wants.
+        SkASSERT(fScratch);

[mtklein, 2014/01/09 14:38:58]

Suggestion: SkASSERT(fScratch != NULL); as a free reminder it's a pointer?

[tomhudson, 2014/01/09 14:49:32]

Done.

         const size_t paddedSize = SizeWithPadding(fScratch->flatSize());
         SkFlatData* detached = (SkFlatData*)fController->allocThrow(paddedSize);
 
         // Copy scratch into the new SkFlatData.
         memcpy(detached, fScratch, paddedSize);
 
         // We can now reuse fScratch, and detached will live until fController dies.
         return detached;
     }
 
     void unflatten(T* dst, const SkFlatData* element) const {
         element->unflatten<Traits>(dst,
                                    fController->getBitmapHeap(),
                                    fController->getTypefacePlayback());
     }
 
     // All SkFlatData* stored in fIndexedData and fHash are owned by the controller.
     SkAutoTUnref<SkFlatController> fController;
     size_t fScratchSize;  // How many bytes fScratch has allocated for data itself.
-    SkFlatData* fScratch;  // Owned, must be freed with sk_free.
+    SkFlatData* fScratch;  // Owned, lazily allocated, must be freed with sk_free.
     SkOrderedWriteBuffer fWriteBuffer;
-    bool fWriteBufferReady;
+    bool fBuffersReady;
 
     // We map between SkFlatData and a 1-based integer index.
     int fNextIndex;
 
     // For index -> SkFlatData.  fIndexedData[0] is always NULL.
     SkTDArray<const SkFlatData*> fIndexedData;
 
     // For SkFlatData -> cached SkFlatData, which has index().
     SkTDynamicHash<SkFlatData, SkFlatData,
                    SkFlatData::Identity, SkFlatData::Hash, SkFlatData::Equal> fHash;
@@ skipping 65 matching lines @@
     }
 
 private:
     SkChunkAlloc               fHeap;
     SkAutoTUnref<SkRefCntSet>  fTypefaceSet;
     void*                      fLastAllocated;
     mutable SkTypefacePlayback fTypefacePlayback;
 };
 
 #endif