From 3fdc7d6dd13b510de09cf29ffd3fe36adf89d541 Mon Sep 17 00:00:00 2001 From: "commit-bot@chromium.org" Date: Wed, 28 May 2014 19:40:21 +0000 Subject: Spin off just SkLazyFnPtr from 305513002. The memory barrier in SkOnce is a perf regression for sk_mem{set,cpy} in SkUtils on ARM. We can do a lot better for function pointers. BUG=skia: R=bungeman@google.com, mtklein@google.com Author: mtklein@chromium.org Review URL: https://codereview.chromium.org/305753002 git-svn-id: http://skia.googlecode.com/svn/trunk@14929 2bbb7eff-a529-9590-31e7-b0007b416f81 --- src/core/SkLazyFnPtr.h | 58 ++++++++++++++++++++++++++++++++++++++++++++++ src/core/SkThread.h | 6 +++++ src/core/SkUtils.cpp | 50 +++++++++++++-------------------------- src/ports/SkAtomics_sync.h | 6 +++++ src/ports/SkAtomics_win.h | 4 ++++ 5 files changed, 90 insertions(+), 34 deletions(-) create mode 100644 src/core/SkLazyFnPtr.h diff --git a/src/core/SkLazyFnPtr.h b/src/core/SkLazyFnPtr.h new file mode 100644 index 0000000000..cb7832d4c5 --- /dev/null +++ b/src/core/SkLazyFnPtr.h @@ -0,0 +1,58 @@ +#ifndef SkLazyFnPtr_DEFINED +#define SkLazyFnPtr_DEFINED + +/** Declare a lazily-chosen static function pointer of type F. + * + * Example usage: + * + * typedef int (*FooImpl)(int, int); + * + * static FooImpl choose_foo() { return ... }; + * + * int Foo(int a, int b) { + * SK_DECLARE_STATIC_LAZY_FN_PTR(FooImpl, choice); + * return choice.get(choose_foo)(a, b); + * } + * + * You can think of SK_DECLARE_STATIC_LAZY_FN_PTR as a cheaper specialization of SkOnce. + * There is no mutex, and in the fast path, no memory barriers are issued. + * + * This must be used in a global or function scope, not as a class member. + */ +#define SK_DECLARE_STATIC_LAZY_FN_PTR(F, name) static Private::SkLazyFnPtr name = { NULL } + + +// Everything below here is private implementation details. Don't touch, don't even look. + +#include "SkDynamicAnnotations.h" +#include "SkThread.h" + +namespace Private { + +// This has no constructor and is link-time initialized, so its members must be public. +template +struct SkLazyFnPtr { + F get(F (*choose)()) { + // First, try reading to see if it's already set. + F fn = (F)SK_ANNOTATE_UNPROTECTED_READ(fPtr); + if (fn != NULL) { + return fn; + } + + // We think it's not already set. + fn = choose(); + + // No particular memory barriers needed; we're not guarding anything but the pointer itself. + F prev = (F)sk_atomic_cas(&fPtr, NULL, (void*)fn); + + // If prev != NULL, someone snuck in and set fPtr concurrently. + // If prev == NULL, we did write fn to fPtr. + return prev != NULL ? prev : fn; + } + + void* fPtr; +}; + +} // namespace Private + +#endif//SkLazyFnPtr_DEFINED diff --git a/src/core/SkThread.h b/src/core/SkThread.h index c8cd4e9112..ad7d6b009e 100644 --- a/src/core/SkThread.h +++ b/src/core/SkThread.h @@ -33,6 +33,12 @@ static int32_t sk_atomic_dec(int32_t* addr); */ static bool sk_atomic_cas(int32_t* addr, int32_t before, int32_t after); +/** Atomic compare and set, for pointers. + * If *addr == before, set *addr to after. Always returns previous value of *addr. + * This must act as a compiler barrier. + */ +static void* sk_atomic_cas(void** addr, void* before, void* after); + /** If sk_atomic_dec does not act as an acquire (L/SL) barrier, * this must act as an acquire (L/SL) memory barrier and as a compiler barrier. */ diff --git a/src/core/SkUtils.cpp b/src/core/SkUtils.cpp index ca18e0cb2d..591a198c65 100644 --- a/src/core/SkUtils.cpp +++ b/src/core/SkUtils.cpp @@ -8,7 +8,7 @@ #include "SkUtils.h" -#include "SkOnce.h" +#include "SkLazyFnPtr.h" #if 0 #define assign_16_longs(dst, value) \ @@ -113,52 +113,34 @@ static void sk_memcpy32_portable(uint32_t dst[], const uint32_t src[], int count memcpy(dst, src, count * sizeof(uint32_t)); } -static void choose_memset16(SkMemset16Proc* proc) { - *proc = SkMemset16GetPlatformProc(); - if (NULL == *proc) { - *proc = &sk_memset16_portable; - } +static SkMemset16Proc choose_memset16() { + SkMemset16Proc proc = SkMemset16GetPlatformProc(); + return proc ? proc : sk_memset16_portable; } void sk_memset16(uint16_t dst[], uint16_t value, int count) { - SK_DECLARE_STATIC_ONCE(once); - static SkMemset16Proc proc = NULL; - SkOnce(&once, choose_memset16, &proc); - SkASSERT(proc != NULL); - - return proc(dst, value, count); + SK_DECLARE_STATIC_LAZY_FN_PTR(SkMemset16Proc, choice); + return choice.get(choose_memset16)(dst, value, count); } -static void choose_memset32(SkMemset32Proc* proc) { - *proc = SkMemset32GetPlatformProc(); - if (NULL == *proc) { - *proc = &sk_memset32_portable; - } +static SkMemset32Proc choose_memset32() { + SkMemset32Proc proc = SkMemset32GetPlatformProc(); + return proc ? proc : sk_memset32_portable; } void sk_memset32(uint32_t dst[], uint32_t value, int count) { - SK_DECLARE_STATIC_ONCE(once); - static SkMemset32Proc proc = NULL; - SkOnce(&once, choose_memset32, &proc); - SkASSERT(proc != NULL); - - return proc(dst, value, count); + SK_DECLARE_STATIC_LAZY_FN_PTR(SkMemset32Proc, choice); + return choice.get(choose_memset32)(dst, value, count); } -static void choose_memcpy32(SkMemcpy32Proc* proc) { - *proc = SkMemcpy32GetPlatformProc(); - if (NULL == *proc) { - *proc = &sk_memcpy32_portable; - } +static SkMemcpy32Proc choose_memcpy32() { + SkMemcpy32Proc proc = SkMemcpy32GetPlatformProc(); + return proc ? proc : sk_memcpy32_portable; } void sk_memcpy32(uint32_t dst[], const uint32_t src[], int count) { - SK_DECLARE_STATIC_ONCE(once); - static SkMemcpy32Proc proc = NULL; - SkOnce(&once, choose_memcpy32, &proc); - SkASSERT(proc != NULL); - - return proc(dst, src, count); + SK_DECLARE_STATIC_LAZY_FN_PTR(SkMemcpy32Proc, choice); + return choice.get(choose_memcpy32)(dst, src, count); } /////////////////////////////////////////////////////////////////////////////// diff --git a/src/ports/SkAtomics_sync.h b/src/ports/SkAtomics_sync.h index 45ba63f305..b0d17527f0 100644 --- a/src/ports/SkAtomics_sync.h +++ b/src/ports/SkAtomics_sync.h @@ -32,6 +32,12 @@ static inline __attribute__((always_inline)) bool sk_atomic_cas(int32_t* addr, return __sync_bool_compare_and_swap(addr, before, after); } +static inline __attribute__((always_inline)) void* sk_atomic_cas(void** addr, + void* before, + void* after) { + return __sync_val_compare_and_swap(addr, before, after); +} + static inline __attribute__((always_inline)) void sk_membar_acquire__after_atomic_conditional_inc() { } #endif diff --git a/src/ports/SkAtomics_win.h b/src/ports/SkAtomics_win.h index 7454d66055..f1b9ec2a62 100644 --- a/src/ports/SkAtomics_win.h +++ b/src/ports/SkAtomics_win.h @@ -40,6 +40,10 @@ static inline bool sk_atomic_cas(int32_t* addr, int32_t before, int32_t after) { return _InterlockedCompareExchange(reinterpret_cast(addr), after, before) == before; } +static inline void* sk_atomic_cas(void** addr, void* before, void* after) { + return InterlockedCompareExchange(reinterpret_cast(addr), after, before); +} + static inline void sk_membar_acquire__after_atomic_conditional_inc() { } #endif -- cgit v1.2.3