From 78358bf624c7e7c09ffccf638c50870808d884d6 Mon Sep 17 00:00:00 2001 From: mtklein Date: Mon, 2 Jun 2014 08:44:27 -0700 Subject: [PATCH] Port most uses of SkOnce to SkLazyPtr. BUG=skia: Committed: http://code.google.com/p/skia/source/detail?r=15006 Committed: http://code.google.com/p/skia/source/detail?r=15014 R=reed@google.com, mtklein@google.com Author: mtklein@chromium.org Review URL: https://codereview.chromium.org/304383005 --- include/core/SkPathRef.h | 2 +- include/core/SkTypeface.h | 3 +- include/ports/SkFontMgr.h | 2 +- include/ports/SkFontMgr_indirect.h | 1 - include/ports/SkRemotableFontMgr.h | 2 +- src/core/SkFontHost.cpp | 17 ++++-------- src/core/SkGlyphCache.cpp | 18 ++++++------ src/core/SkLazyPtr.h | 2 -- src/core/SkMatrix.cpp | 40 +++++++++++++++------------ src/core/SkMessageBus.h | 21 ++++++-------- src/core/SkPathRef.cpp | 16 +++++------ src/core/SkTypeface.cpp | 44 ++++++++++++++---------------- src/fonts/SkRemotableFontMgr.cpp | 16 ++++------- src/lazy/SkDiscardableMemoryPool.cpp | 23 ++++++---------- src/ports/SkFontConfigInterface_direct.cpp | 15 +++++----- 15 files changed, 98 insertions(+), 124 deletions(-) diff --git a/include/core/SkPathRef.h b/include/core/SkPathRef.h index 8802714..47a69b7 100644 --- a/include/core/SkPathRef.h +++ b/include/core/SkPathRef.h @@ -418,7 +418,7 @@ private: /** * Called the first time someone calls CreateEmpty to actually create the singleton. */ - static void CreateEmptyImpl(int/*unused*/); + static SkPathRef* CreateEmptyImpl(); void setIsOval(bool isOval) { fIsOval = isOval; } diff --git a/include/core/SkTypeface.h b/include/core/SkTypeface.h index 0be97eb..0b1c1f2 100644 --- a/include/core/SkTypeface.h +++ b/include/core/SkTypeface.h @@ -339,7 +339,8 @@ private: uint32_t glyphIDsCount = 0) const; private: - static void create_default_typeface(Style style); + static SkTypeface* CreateDefault(int style); // SkLazyPtr requires an int, not a Style. + static void DeleteDefault(SkTypeface*); SkFontID fUniqueID; Style fStyle; diff --git a/include/ports/SkFontMgr.h b/include/ports/SkFontMgr.h index a2fad9a..bb8c7b7 100644 --- a/include/ports/SkFontMgr.h +++ b/include/ports/SkFontMgr.h @@ -131,7 +131,7 @@ protected: unsigned styleBits) const = 0; private: static SkFontMgr* Factory(); // implemented by porting layer - friend void set_up_default(SkFontMgr** singleton); + static SkFontMgr* CreateDefault(); typedef SkRefCnt INHERITED; }; diff --git a/include/ports/SkFontMgr_indirect.h b/include/ports/SkFontMgr_indirect.h index 6d497ee..b9ce344 100644 --- a/include/ports/SkFontMgr_indirect.h +++ b/include/ports/SkFontMgr_indirect.h @@ -11,7 +11,6 @@ #include "SkDataTable.h" #include "SkFontMgr.h" #include "SkFontStyle.h" -#include "SkOnce.h" #include "SkRemotableFontMgr.h" #include "SkTArray.h" #include "SkTypeface.h" diff --git a/include/ports/SkRemotableFontMgr.h b/include/ports/SkRemotableFontMgr.h index 68b4462..bd99497 100644 --- a/include/ports/SkRemotableFontMgr.h +++ b/include/ports/SkRemotableFontMgr.h @@ -46,7 +46,7 @@ public: private: SkRemotableFontIdentitySet() : fCount(0), fData() { } - static void NewEmptyImpl(int); + static SkRemotableFontIdentitySet* NewEmptyImpl(); int fCount; SkAutoTMalloc fData; diff --git a/src/core/SkFontHost.cpp b/src/core/SkFontHost.cpp index 9e7eeb1..a16a8c4 100644 --- a/src/core/SkFontHost.cpp +++ b/src/core/SkFontHost.cpp @@ -6,7 +6,7 @@ */ #include "SkFontLCDConfig.h" -#include "SkOnce.h" +#include "SkLazyPtr.h" static SkFontLCDConfig::LCDOrientation gLCDOrientation = SkFontLCDConfig::kHorizontal_LCDOrientation; static SkFontLCDConfig::LCDOrder gLCDOrder = SkFontLCDConfig::kRGB_LCDOrder; @@ -198,19 +198,14 @@ SkTypeface* SkFontMgr::legacyCreateTypeface(const char familyName[], return this->onLegacyCreateTypeface(familyName, styleBits); } -void set_up_default(SkFontMgr** singleton) { - *singleton = SkFontMgr::Factory(); - // we never want to return NULL - if (NULL == *singleton) { - *singleton = SkNEW(SkEmptyFontMgr); - } +SkFontMgr* SkFontMgr::CreateDefault() { + SkFontMgr* fm = SkFontMgr::Factory(); + return fm ? fm : SkNEW(SkEmptyFontMgr); } SkFontMgr* SkFontMgr::RefDefault() { - static SkFontMgr* gFM = NULL; - SK_DECLARE_STATIC_ONCE(once); - SkOnce(&once, set_up_default, &gFM); - return SkRef(gFM); + SK_DECLARE_STATIC_LAZY_PTR(SkFontMgr, singleton, CreateDefault); + return SkRef(singleton.get()); } ////////////////////////////////////////////////////////////////////////// diff --git a/src/core/SkGlyphCache.cpp b/src/core/SkGlyphCache.cpp index 699801d..2ab721a 100755 --- a/src/core/SkGlyphCache.cpp +++ b/src/core/SkGlyphCache.cpp @@ -11,7 +11,7 @@ #include "SkGlyphCache_Globals.h" #include "SkDistanceFieldGen.h" #include "SkGraphics.h" -#include "SkOnce.h" +#include "SkLazyPtr.h" #include "SkPaint.h" #include "SkPath.h" #include "SkTemplates.h" @@ -21,18 +21,18 @@ //#define SPEW_PURGE_STATUS //#define RECORD_HASH_EFFICIENCY -static void create_globals(SkGlyphCache_Globals** globals) { - *globals = SkNEW_ARGS(SkGlyphCache_Globals, (SkGlyphCache_Globals::kYes_UseMutex)); +namespace { + +SkGlyphCache_Globals* create_globals() { + return SkNEW_ARGS(SkGlyphCache_Globals, (SkGlyphCache_Globals::kYes_UseMutex)); } +} // namespace + // Returns the shared globals static SkGlyphCache_Globals& getSharedGlobals() { - // we leak this, so we don't incur any shutdown cost of the destructor - static SkGlyphCache_Globals* gGlobals = NULL; - SK_DECLARE_STATIC_ONCE(once); - SkOnce(&once, create_globals, &gGlobals); - SkASSERT(NULL != gGlobals); - return *gGlobals; + SK_DECLARE_STATIC_LAZY_PTR(SkGlyphCache_Globals, globals, create_globals); + return *globals.get(); } // Returns the TLS globals (if set), or the shared globals diff --git a/src/core/SkLazyPtr.h b/src/core/SkLazyPtr.h index 71876e7..50d8328 100644 --- a/src/core/SkLazyPtr.h +++ b/src/core/SkLazyPtr.h @@ -64,7 +64,6 @@ // See FIXME below. class SkFontConfigInterface; -class SkTypeface; namespace Private { @@ -100,7 +99,6 @@ public: #ifdef SK_DEVELOPER // FIXME: We know we leak refs on some classes. For now, let them leak. void cleanup(SkFontConfigInterface*) {} - void cleanup(SkTypeface*) {} template void cleanup(U* ptr) { Destroy(ptr); } ~SkLazyPtr() { diff --git a/src/core/SkMatrix.cpp b/src/core/SkMatrix.cpp index 40f6e52..8778f78 100644 --- a/src/core/SkMatrix.cpp +++ b/src/core/SkMatrix.cpp @@ -7,7 +7,7 @@ #include "SkMatrix.h" #include "SkFloatBits.h" -#include "SkOnce.h" +#include "SkLazyPtr.h" #include "SkString.h" // In a few places, we performed the following @@ -1558,29 +1558,33 @@ bool SkMatrix::getMinMaxScales(SkScalar scaleFactors[2]) const { return get_scale_factor(this->getType(), fMat, scaleFactors); } -static void reset_identity_matrix(SkMatrix* identity) { - identity->reset(); +namespace { + +SkMatrix* create_identity() { + SkMatrix* m = SkNEW(SkMatrix); + m->reset(); + return m; +} + +SkMatrix* create_invalid() { + SkMatrix* m = SkNEW(SkMatrix); + m->setAll(SK_ScalarMax, SK_ScalarMax, SK_ScalarMax, + SK_ScalarMax, SK_ScalarMax, SK_ScalarMax, + SK_ScalarMax, SK_ScalarMax, SK_ScalarMax); + m->getType(); // Force the type to be computed. + return m; } +} // namespace + const SkMatrix& SkMatrix::I() { - // If you can use C++11 now, you might consider replacing this with a constexpr constructor. - static SkMatrix gIdentity; - SK_DECLARE_STATIC_ONCE(once); - SkOnce(&once, reset_identity_matrix, &gIdentity); - return gIdentity; + SK_DECLARE_STATIC_LAZY_PTR(SkMatrix, identity, create_identity); + return *identity.get(); } const SkMatrix& SkMatrix::InvalidMatrix() { - static SkMatrix gInvalid; - static bool gOnce; - if (!gOnce) { - gInvalid.setAll(SK_ScalarMax, SK_ScalarMax, SK_ScalarMax, - SK_ScalarMax, SK_ScalarMax, SK_ScalarMax, - SK_ScalarMax, SK_ScalarMax, SK_ScalarMax); - gInvalid.getType(); // force the type to be computed - gOnce = true; - } - return gInvalid; + SK_DECLARE_STATIC_LAZY_PTR(SkMatrix, invalid, create_invalid); + return *invalid.get(); } /////////////////////////////////////////////////////////////////////////////// diff --git a/src/core/SkMessageBus.h b/src/core/SkMessageBus.h index ddeac57..f36c42b 100644 --- a/src/core/SkMessageBus.h +++ b/src/core/SkMessageBus.h @@ -8,7 +8,7 @@ #ifndef SkMessageBus_DEFINED #define SkMessageBus_DEFINED -#include "SkOnce.h" +#include "SkLazyPtr.h" #include "SkTDArray.h" #include "SkThread.h" #include "SkTypes.h" @@ -38,7 +38,7 @@ public: private: SkMessageBus(); static SkMessageBus* Get(); - static void New(SkMessageBus**); + static SkMessageBus* New(); SkTDArray fInboxes; SkMutex fInboxesMutex; @@ -46,14 +46,11 @@ private: // This must go in a single .cpp file, not some .h, or we risk creating more than one global // SkMessageBus per type when using shared libraries. -#define DECLARE_SKMESSAGEBUS_MESSAGE(Message) \ - template <> \ - SkMessageBus* SkMessageBus::Get() { \ - static SkMessageBus* bus = NULL; \ - SK_DECLARE_STATIC_ONCE(once); \ - SkOnce(&once, &New, &bus); \ - SkASSERT(bus != NULL); \ - return bus; \ +#define DECLARE_SKMESSAGEBUS_MESSAGE(Message) \ + template <> \ + SkMessageBus* SkMessageBus::Get() { \ + SK_DECLARE_STATIC_LAZY_PTR(SkMessageBus, bus, New); \ + return bus.get(); \ } // ----------------------- Implementation of SkMessageBus::Inbox ----------------------- @@ -100,8 +97,8 @@ template SkMessageBus::SkMessageBus() {} template -/*static*/ void SkMessageBus::New(SkMessageBus** bus) { - *bus = new SkMessageBus(); +/*static*/ SkMessageBus* SkMessageBus::New() { + return SkNEW(SkMessageBus); } template diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp index 161eb80..de7a8f5 100644 --- a/src/core/SkPathRef.cpp +++ b/src/core/SkPathRef.cpp @@ -6,7 +6,7 @@ */ #include "SkBuffer.h" -#include "SkOnce.h" +#include "SkLazyPtr.h" #include "SkPath.h" #include "SkPathRef.h" @@ -28,18 +28,16 @@ SkPathRef::Editor::Editor(SkAutoTUnref* pathRef, } ////////////////////////////////////////////////////////////////////////////// -static SkPathRef* gEmptyPathRef = NULL; -static void cleanup_gEmptyPathRef() { gEmptyPathRef->unref(); } -void SkPathRef::CreateEmptyImpl(int) { - gEmptyPathRef = SkNEW(SkPathRef); - gEmptyPathRef->computeBounds(); // Preemptively avoid a race to clear fBoundsIsDirty. +SkPathRef* SkPathRef::CreateEmptyImpl() { + SkPathRef* p = SkNEW(SkPathRef); + p->computeBounds(); // Preemptively avoid a race to clear fBoundsIsDirty. + return p; } SkPathRef* SkPathRef::CreateEmpty() { - SK_DECLARE_STATIC_ONCE(once); - SkOnce(&once, SkPathRef::CreateEmptyImpl, 0, cleanup_gEmptyPathRef); - return SkRef(gEmptyPathRef); + SK_DECLARE_STATIC_LAZY_PTR(SkPathRef, empty, CreateEmptyImpl); + return SkRef(empty.get()); } void SkPathRef::CreateTransformedCopy(SkAutoTUnref* dst, diff --git a/src/core/SkTypeface.cpp b/src/core/SkTypeface.cpp index cd3953b..fd2803b 100644 --- a/src/core/SkTypeface.cpp +++ b/src/core/SkTypeface.cpp @@ -8,7 +8,7 @@ #include "SkAdvancedTypefaceMetrics.h" #include "SkFontDescriptor.h" #include "SkFontHost.h" -#include "SkOnce.h" +#include "SkLazyPtr.h" #include "SkStream.h" #include "SkTypeface.h" @@ -74,34 +74,30 @@ protected: } }; -static SkTypeface* gDefaultTypefaces[] = { NULL, NULL, NULL, NULL }; -static const size_t FONT_STYLE_COUNT = SK_ARRAY_COUNT(gDefaultTypefaces); -static SkOnceFlag gDefaultTypefaceOnce[FONT_STYLE_COUNT] = { - SK_ONCE_INIT, SK_ONCE_INIT, SK_ONCE_INIT, SK_ONCE_INIT -}; -template struct SkTIsPow2 { - static const bool value = (N & (N - 1)) == 0; -}; -SK_COMPILE_ASSERT(SkTIsPow2::value, FONT_STYLE_COUNT_not_power_of_2); +SkTypeface* SkTypeface::CreateDefault(int style) { + // If backed by fontconfig, it's not safe to call SkFontHost::CreateTypeface concurrently. + // To be safe, we serialize here with a mutex so only one call to + // CreateTypeface is happening at any given time. + // TODO(bungeman, mtklein): This is sad. Make our fontconfig code safe? + SK_DECLARE_STATIC_MUTEX(mutex); + SkAutoMutexAcquire lock(&mutex); -void SkTypeface::create_default_typeface(Style style) { - if (NULL == gDefaultTypefaces[style]) { - gDefaultTypefaces[style] = SkFontHost::CreateTypeface(NULL, NULL, style); - } - if (NULL == gDefaultTypefaces[style]) { - // FIXME: Use a singleton for SkEmptyTypeface. - gDefaultTypefaces[style] = SkEmptyTypeface::Create(); - } + SkTypeface* t = SkFontHost::CreateTypeface(NULL, NULL, (Style)style); + return t ? t : SkEmptyTypeface::Create(); } -SkTypeface* SkTypeface::GetDefaultTypeface(Style style) { - SkASSERT((size_t)style < FONT_STYLE_COUNT); +void SkTypeface::DeleteDefault(SkTypeface* t) { + // The SkTypeface returned by SkFontHost::CreateTypeface may _itself_ be a + // cleverly-shared singleton. This is less than ideal. This means we + // cannot just assert our ownership and SkDELETE(t) like we'd want to. + SkSafeUnref(t); +} - // mask off any other bits to avoid a crash in SK_RELEASE - style = (Style)(style & (FONT_STYLE_COUNT - 1)); +SkTypeface* SkTypeface::GetDefaultTypeface(Style style) { + SK_DECLARE_STATIC_LAZY_PTR_ARRAY(SkTypeface, defaults, 4, CreateDefault, DeleteDefault); - SkOnce(&gDefaultTypefaceOnce[style], SkTypeface::create_default_typeface, style); - return gDefaultTypefaces[style]; + SkASSERT((int)style < 4); + return defaults[style]; } SkTypeface* SkTypeface::RefDefault(Style style) { diff --git a/src/fonts/SkRemotableFontMgr.cpp b/src/fonts/SkRemotableFontMgr.cpp index 1139972..633e914 100644 --- a/src/fonts/SkRemotableFontMgr.cpp +++ b/src/fonts/SkRemotableFontMgr.cpp @@ -7,7 +7,7 @@ #include "SkRemotableFontMgr.h" -#include "SkOnce.h" +#include "SkLazyPtr.h" SkRemotableFontIdentitySet::SkRemotableFontIdentitySet(int count, SkFontIdentity** data) : fCount(count), fData(count) @@ -16,17 +16,11 @@ SkRemotableFontIdentitySet::SkRemotableFontIdentitySet(int count, SkFontIdentity *data = fData; } -static SkRemotableFontIdentitySet* gEmptyRemotableFontIdentitySet = NULL; -static void cleanup_gEmptyRemotableFontIdentitySet() { gEmptyRemotableFontIdentitySet->unref(); } - -void SkRemotableFontIdentitySet::NewEmptyImpl(int) { - gEmptyRemotableFontIdentitySet = new SkRemotableFontIdentitySet(); +SkRemotableFontIdentitySet* SkRemotableFontIdentitySet::NewEmptyImpl() { + return SkNEW(SkRemotableFontIdentitySet); } SkRemotableFontIdentitySet* SkRemotableFontIdentitySet::NewEmpty() { - SK_DECLARE_STATIC_ONCE(once); - SkOnce(&once, SkRemotableFontIdentitySet::NewEmptyImpl, 0, - cleanup_gEmptyRemotableFontIdentitySet); - gEmptyRemotableFontIdentitySet->ref(); - return gEmptyRemotableFontIdentitySet; + SK_DECLARE_STATIC_LAZY_PTR(SkRemotableFontIdentitySet, empty, NewEmptyImpl); + return SkRef(empty.get()); } diff --git a/src/lazy/SkDiscardableMemoryPool.cpp b/src/lazy/SkDiscardableMemoryPool.cpp index 4fb8ae4..1d63a0b 100644 --- a/src/lazy/SkDiscardableMemoryPool.cpp +++ b/src/lazy/SkDiscardableMemoryPool.cpp @@ -7,7 +7,7 @@ #include "SkDiscardableMemory.h" #include "SkDiscardableMemoryPool.h" -#include "SkOnce.h" +#include "SkLazyPtr.h" #include "SkTInternalLList.h" #include "SkThread.h" @@ -248,27 +248,20 @@ void DiscardableMemoryPool::dumpPool() { //////////////////////////////////////////////////////////////////////////////// SK_DECLARE_STATIC_MUTEX(gMutex); -SkDiscardableMemoryPool* gPool = NULL; -void create_global_pool(int) { - SkASSERT(NULL == gPool); - gPool = SkDiscardableMemoryPool::Create( - SK_DEFAULT_GLOBAL_DISCARDABLE_MEMORY_POOL_SIZE, &gMutex); -} -void cleanup_global_pool() { - gPool->unref(); +SkDiscardableMemoryPool* create_global_pool() { + return SkDiscardableMemoryPool::Create(SK_DEFAULT_GLOBAL_DISCARDABLE_MEMORY_POOL_SIZE, + &gMutex); } + } // namespace -SkDiscardableMemoryPool* SkDiscardableMemoryPool::Create( - size_t size, SkBaseMutex* mutex) { +SkDiscardableMemoryPool* SkDiscardableMemoryPool::Create(size_t size, SkBaseMutex* mutex) { return SkNEW_ARGS(DiscardableMemoryPool, (size, mutex)); } SkDiscardableMemoryPool* SkGetGlobalDiscardableMemoryPool() { - SK_DECLARE_STATIC_ONCE(create_pool_once); - SkOnce(&create_pool_once, create_global_pool, 0, &cleanup_global_pool); - SkASSERT(NULL != gPool); - return gPool; + SK_DECLARE_STATIC_LAZY_PTR(SkDiscardableMemoryPool, global, create_global_pool); + return global.get(); } // defined in SkImageGenerator.h diff --git a/src/ports/SkFontConfigInterface_direct.cpp b/src/ports/SkFontConfigInterface_direct.cpp index 80ee56e..bc3dede 100644 --- a/src/ports/SkFontConfigInterface_direct.cpp +++ b/src/ports/SkFontConfigInterface_direct.cpp @@ -15,7 +15,7 @@ #include "SkBuffer.h" #include "SkFontConfigInterface.h" -#include "SkOnce.h" +#include "SkLazyPtr.h" #include "SkStream.h" size_t SkFontConfigInterface::FontIdentity::writeToMemory(void* addr) const { @@ -124,14 +124,13 @@ private: SkMutex mutex_; }; -static void create_singleton_direct_interface(SkFontConfigInterface** singleton) { - *singleton = new SkFontConfigInterfaceDirect; -} +namespace { +SkFontConfigInterface* create_direct() { return SkNEW(SkFontConfigInterfaceDirect); } +} // namespace + SkFontConfigInterface* SkFontConfigInterface::GetSingletonDirectInterface() { - static SkFontConfigInterface* gDirect; - SK_DECLARE_STATIC_ONCE(once); - SkOnce(&once, create_singleton_direct_interface, &gDirect); - return gDirect; + SK_DECLARE_STATIC_LAZY_PTR(SkFontConfigInterface, direct, create_direct); + return direct.get(); } /////////////////////////////////////////////////////////////////////////////// -- 2.7.4