SkLiteDL: remove freelisting, add reset() and SKLITEDL_PAGE knob.

We think Android can cache these better than a global freelist allows.
This removes the freelisting but adds reset() to allow reuse.

I took the opportunity to abstract 4096 as a define SKLITEDL_PAGE.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2248693004

Review-Url: https://codereview.chromium.org/2248693004

5 files changed, 22 insertions, 94 deletions

diff --git a/include/core/SkMath.h b/include/core/SkMath.h
index 8276c5d04f..6e252306df 100644
--- a/include/core/SkMath.h
+++ b/include/core/SkMath.h
@@ -90,7 +90,7 @@ static inline int SkClampMax(int value, int max) {
  *  Returns true if value is a power of 2. Does not explicitly check for
  *  value <= 0.
  */
-template <typename T> inline bool SkIsPow2(T value) {
+template <typename T> constexpr inline bool SkIsPow2(T value) {
     return (value & (value - 1)) == 0;
 }
 
diff --git a/src/core/SkGraphics.cpp b/src/core/SkGraphics.cpp
index 397d03bb81..01b1432ef0 100644
--- a/src/core/SkGraphics.cpp
+++ b/src/core/SkGraphics.cpp
@@ -14,7 +14,6 @@
 #include "SkGeometry.h"
 #include "SkGlyphCache.h"
 #include "SkImageFilter.h"
-#include "SkLiteDL.h"
 #include "SkMath.h"
 #include "SkMatrix.h"
 #include "SkOpts.h"
@@ -62,7 +61,6 @@ void SkGraphics::PurgeAllCaches() {
     SkGraphics::PurgeFontCache();
     SkGraphics::PurgeResourceCache();
     SkImageFilter::PurgeCache();
-    SkLiteDL::PurgeFreelist();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/src/core/SkLiteDL.cpp b/src/core/SkLiteDL.cpp
index b8938ba771..2c0b3a57ec 100644
--- a/src/core/SkLiteDL.cpp
+++ b/src/core/SkLiteDL.cpp
@@ -9,12 +9,17 @@
 #include "SkData.h"
 #include "SkImageFilter.h"
 #include "SkLiteDL.h"
+#include "SkMath.h"
 #include "SkPicture.h"
 #include "SkMutex.h"
 #include "SkRSXform.h"
 #include "SkSpinlock.h"
 #include "SkTextBlob.h"
 
+#ifndef SKLITEDL_PAGE
+    #define SKLITEDL_PAGE 4096
+#endif
+
 // A stand-in for an optional SkRect which was not set, e.g. bounds for a saveLayer().
 static const SkRect kUnset = { SK_ScalarInfinity, 0,0,0};
 static const SkRect* maybe_unset(const SkRect& r) {
@@ -527,7 +532,9 @@ void* SkLiteDL::push(size_t pod, Args&&... args) {
     size_t skip = SkAlignPtr(sizeof(T) + pod);
     SkASSERT(skip < (1<<24));
     if (fUsed + skip > fReserved) {
-        fReserved = (fUsed + skip + 4096) & ~4095;  // Next greater multiple of 4096.
+        static_assert(SkIsPow2(SKLITEDL_PAGE), "This math needs updating for non-pow2.");
+        // Next greater multiple of SKLITEDL_PAGE.
+        fReserved = (fUsed + skip + SKLITEDL_PAGE) & ~(SKLITEDL_PAGE-1);
         fBytes.realloc(fReserved);
     }
     SkASSERT(fUsed + skip <= fReserved);
@@ -768,60 +775,21 @@ bool SkLiteDL::empty() const {
     return fUsed == 0;
 }
 
-#if !defined(SK_LITEDL_USES)
-    #define  SK_LITEDL_USES 16
-#endif
+SkLiteDL:: SkLiteDL(SkRect bounds) : fUsed(0), fReserved(0), fBounds(bounds) {}
 
-SkLiteDL:: SkLiteDL() : fUsed(0), fReserved(0), fUsesRemaining(SK_LITEDL_USES) {}
-SkLiteDL::~SkLiteDL() {}
-
-// If you're tempted to make this lock free, please don't forget about ABA.
-static SkSpinlock gFreeStackLock;
-static SkLiteDL*  gFreeStack = nullptr;
+SkLiteDL::~SkLiteDL() {
+    this->reset(SkRect::MakeEmpty());
+}
 
 sk_sp<SkLiteDL> SkLiteDL::New(SkRect bounds) {
-    sk_sp<SkLiteDL> dl;
-    {
-        SkAutoMutexAcquire lock(gFreeStackLock);
-        if (gFreeStack) {
-            dl.reset(gFreeStack);  // Adopts the ref the stack's been holding.
-            gFreeStack = gFreeStack->fNext;
-        }
-    }
-
-    if (!dl) {
-        dl.reset(new SkLiteDL);
-    }
-
-    dl->fBounds = bounds;
-    return dl;
+    return sk_sp<SkLiteDL>(new SkLiteDL(bounds));
 }
 
-void SkLiteDL::internal_dispose() const {
-    // Whether we delete this or leave it on the free stack,
-    // we want its refcnt at 1.
-    this->internal_dispose_restore_refcnt_to_1();
-
-    auto self = const_cast<SkLiteDL*>(this);
-    self->map(dtor_fns);
-
-    if (--self->fUsesRemaining > 0) {
-        self->fUsed = 0;
-
-        SkAutoMutexAcquire lock(gFreeStackLock);
-        self->fNext = gFreeStack;
-        gFreeStack = self;
-        return;
-    }
-
-    delete this;
-}
+void SkLiteDL::reset(SkRect bounds) {
+    SkASSERT(this->unique());
+    this->map(dtor_fns);
 
-void SkLiteDL::PurgeFreelist() {
-    SkAutoMutexAcquire lock(gFreeStackLock);
-    while (gFreeStack) {
-        SkLiteDL* top = gFreeStack;
-        gFreeStack = gFreeStack->fNext;
-        delete top;   // Calling unref() here would just put it back on the list!
-    }
+    // Leave fBytes and fReserved alone.
+    fUsed   = 0;
+    fBounds = bounds;
 }
diff --git a/src/core/SkLiteDL.h b/src/core/SkLiteDL.h
index aa4dea480a..ac1eb9f782 100644
--- a/src/core/SkLiteDL.h
+++ b/src/core/SkLiteDL.h
@@ -20,8 +20,7 @@ class GrContext;
 class SkLiteDL final : public SkDrawable {
 public:
     static sk_sp<SkLiteDL> New(SkRect);
-
-    static void PurgeFreelist();
+    void reset(SkRect);
 
     void optimizeFor(GrContext*);
     void makeThreadsafe();
@@ -81,11 +80,9 @@ public:
                    SkXfermode::Mode, const SkRect*, const SkPaint*);
 
 private:
-    SkLiteDL();
+    SkLiteDL(SkRect);
     ~SkLiteDL();
 
-    void internal_dispose() const override;
-
     SkRect   onGetBounds() override;
     void onDraw(SkCanvas*) override;
 
@@ -99,10 +96,6 @@ private:
     size_t                 fUsed;
     size_t                 fReserved;
     SkRect                 fBounds;
-
-    // Only used for freelisting.
-    SkLiteDL* fNext;
-    int       fUsesRemaining;
 };
 
 #endif//SkLiteDL_DEFINED
diff --git a/tests/SkLiteDLTest.cpp b/tests/SkLiteDLTest.cpp
index cb6ba98708..70a9077f59 100644
--- a/tests/SkLiteDLTest.cpp
+++ b/tests/SkLiteDLTest.cpp
@@ -9,37 +9,6 @@
 #include "SkLiteDL.h"
 #include "SkLiteRecorder.h"
 
-#if 0   // This test doesn't make sense when run in a threaded environment.  It tests global state.
-DEF_TEST(SkLiteDL_freelisting, r) {
-    // TODO: byte and count limit tests
-    sk_sp<SkLiteDL> sp1 = SkLiteDL::New({1,1,10,10}),
-                    sp2 = SkLiteDL::New({2,2,20,20});
-
-    SkLiteDL* p1 = sp1.get();
-    SkLiteDL* p2 = sp2.get();
-    REPORTER_ASSERT(r, p1 != p2);
-    REPORTER_ASSERT(r, p1->getBounds().left() == 1);
-    REPORTER_ASSERT(r, p2->getBounds().left() == 2);
-
-    sp2.reset();
-
-    sk_sp<SkLiteDL> sp3 = SkLiteDL::New({3,3,30,30});
-    SkLiteDL* p3 = sp3.get();
-    REPORTER_ASSERT(r, p1 != p3);
-    REPORTER_ASSERT(r, p2 == p3);
-    REPORTER_ASSERT(r, p1->getBounds().left() == 1);
-    REPORTER_ASSERT(r, p3->getBounds().left() == 3);
-
-    sp3.reset();
-    sp1.reset();
-
-    sk_sp<SkLiteDL> sp4 = SkLiteDL::New({4,4,40,40});
-    SkLiteDL* p4 = sp4.get();
-    REPORTER_ASSERT(r, p4 == p1);  // Checks that we operate in stack order.  Nice, not essential.
-    REPORTER_ASSERT(r, p4->getBounds().left() == 4);
-}
-#endif
-
 DEF_TEST(SkLiteDL_basics, r) {
     sk_sp<SkLiteDL> p { SkLiteDL::New({2,2,3,3}) };