Revert of Use BBH reserve hook to preallocate space for tiles. (patchset #2 id:80001...
authormtklein <mtklein@google.com>
Thu, 9 Oct 2014 20:55:20 +0000 (13:55 -0700)
committerCommit bot <commit-bot@chromium.org>
Thu, 9 Oct 2014 20:55:20 +0000 (13:55 -0700)
Reason for revert:
failed assertion "fXTiles * fYTiles != 0"

Original issue's description:
> Use BBH reserve hook to preallocate space for tiles.
>
> Before getting too far into changing how SkTileGrid stores its tiles, I figured I'd
> better see how much I can tweak out the existing format.  Cleverly, that way
> any improvements I make by changing the format will look that much less
> impressive.
>
> This CL looks like it will be a 5-15% win in time spent recording, with no effect
> on playback.
>
> This CL also shrinks the tiles to fit exactly when we're done inserting,
> using newly added SkTDArray::shrinkToFit().  It's quite cheap to run (maybe
> taking back 1-2% from those 5-15% wins), and means we'll lug around about 15%
> fewer bytes in the tile grids.  Note though this strategy temporarily uses up to
> 30% more memory while building the tile grid.  For our largest SKPs, that's
> maybe 75-100K extra.
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/52455cbc02d7f480d988ae7cdacc11ad69078c2c

TBR=reed@google.com,robertphillips@google.com,mtklein@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review URL: https://codereview.chromium.org/642933002

include/core/SkTDArray.h
src/core/SkTileGrid.cpp
src/core/SkTileGrid.h

index 8e8b492..92f297c 100644 (file)
@@ -347,11 +347,6 @@ public:
     }
 #endif
 
-    void shrinkToFit() {
-        fReserve = fCount;
-        fArray = (T*)sk_realloc_throw(fArray, fReserve * sizeof(T));
-    }
-
 private:
 #ifdef SK_DEBUG
     enum {
index 2a3eac9..8b96b73 100644 (file)
@@ -17,37 +17,12 @@ SkTileGrid::SkTileGrid(int xTiles, int yTiles, const SkTileGridFactory::TileGrid
     , fOffset(SkPoint::Make(info.fOffset.fX, info.fOffset.fY))
     , fGridBounds(SkRect::MakeWH(xTiles * info.fTileInterval.width(),
                                  yTiles * info.fTileInterval.height()))
-    , fTiles(SkNEW_ARRAY(SkTDArray<unsigned>, xTiles * yTiles)) {
-    SkASSERT(fXTiles * fYTiles != 0);
-}
+    , fTiles(SkNEW_ARRAY(SkTDArray<unsigned>, xTiles * yTiles)) {}
 
 SkTileGrid::~SkTileGrid() {
     SkDELETE_ARRAY(fTiles);
 }
 
-void SkTileGrid::reserve(unsigned opCount) {
-    // If we assume every op we're about to try to insert() falls within our grid bounds,
-    // then every op has to hit at least one tile.  In fact, a quick scan over our small
-    // SKP set shows that in the average SKP, each op hits two 256x256 tiles.
-
-    // If we take those observations and further assume the ops are distributed evenly
-    // across the picture, we get this guess for number of ops per tile:
-    const int opsPerTileGuess = (2 * opCount) / (fXTiles * fYTiles);
-
-    for (SkTDArray<unsigned>* tile = fTiles; tile != fTiles + (fXTiles * fYTiles); tile++) {
-        tile->setReserve(opsPerTileGuess);
-    }
-
-    // In practice, this heuristic means we'll temporarily allocate about 30% more bytes
-    // than if we made no setReserve() calls, but time spent in insert() drops by about 50%.
-}
-
-void SkTileGrid::flushDeferredInserts() {
-    for (SkTDArray<unsigned>* tile = fTiles; tile != fTiles + (fXTiles * fYTiles); tile++) {
-        tile->shrinkToFit();
-    }
-}
-
 // Adjustments to user-provided bounds common to both insert() and search().
 // Call this after making insert- or search- specific adjustments.
 void SkTileGrid::commonAdjust(SkRect* rect) const {
index fd7584f..e8a0d96 100644 (file)
@@ -39,9 +39,6 @@ public:
     // For testing.
     int tileCount(int x, int y) { return fTiles[y * fXTiles + x].count(); }
 
-    virtual void reserve(unsigned opCount) SK_OVERRIDE;
-    virtual void flushDeferredInserts() SK_OVERRIDE;
-
 private:
     void commonAdjust(SkRect*) const;
     void userToGrid(const SkRect&, SkIRect* grid) const;