diff options
author | commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> | 2014-05-28 19:58:14 +0000 |
---|---|---|
committer | commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> | 2014-05-28 19:58:14 +0000 |
commit | 05d1cc23ec8c7c4befa039330c48901f9ff1251c (patch) | |
tree | c181737a55a4c09cf64b09264d114e77ae1b939e | |
parent | 6640ee3378e3ecf23749faa3b513642ce188b055 (diff) |
Revert of Spin off just SkLazyFnPtr from 305513002. (https://codereview.chromium.org/305753002/)
Reason for revert:
depends on bad cl
Original issue's description:
> 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:
>
> Committed: http://code.google.com/p/skia/source/detail?r=14929
R=bungeman@google.com, mtklein@chromium.org
TBR=bungeman@google.com, mtklein@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=skia:
Author: mtklein@google.com
Review URL: https://codereview.chromium.org/300843012
git-svn-id: http://skia.googlecode.com/svn/trunk@14933 2bbb7eff-a529-9590-31e7-b0007b416f81
-rw-r--r-- | src/core/SkLazyFnPtr.h | 58 | ||||
-rw-r--r-- | src/core/SkThread.h | 6 | ||||
-rw-r--r-- | src/core/SkUtils.cpp | 50 | ||||
-rw-r--r-- | src/ports/SkAtomics_sync.h | 6 | ||||
-rw-r--r-- | src/ports/SkAtomics_win.h | 4 |
5 files changed, 34 insertions, 90 deletions
diff --git a/src/core/SkLazyFnPtr.h b/src/core/SkLazyFnPtr.h deleted file mode 100644 index cb7832d4c5..0000000000 --- a/src/core/SkLazyFnPtr.h +++ /dev/null @@ -1,58 +0,0 @@ -#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<F> 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 <typename F> -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 ad7d6b009e..c8cd4e9112 100644 --- a/src/core/SkThread.h +++ b/src/core/SkThread.h @@ -33,12 +33,6 @@ 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 591a198c65..ca18e0cb2d 100644 --- a/src/core/SkUtils.cpp +++ b/src/core/SkUtils.cpp @@ -8,7 +8,7 @@ #include "SkUtils.h" -#include "SkLazyFnPtr.h" +#include "SkOnce.h" #if 0 #define assign_16_longs(dst, value) \ @@ -113,34 +113,52 @@ static void sk_memcpy32_portable(uint32_t dst[], const uint32_t src[], int count memcpy(dst, src, count * sizeof(uint32_t)); } -static SkMemset16Proc choose_memset16() { - SkMemset16Proc proc = SkMemset16GetPlatformProc(); - return proc ? proc : sk_memset16_portable; +static void choose_memset16(SkMemset16Proc* proc) { + *proc = SkMemset16GetPlatformProc(); + if (NULL == *proc) { + *proc = &sk_memset16_portable; + } } void sk_memset16(uint16_t dst[], uint16_t value, int count) { - SK_DECLARE_STATIC_LAZY_FN_PTR(SkMemset16Proc, choice); - return choice.get(choose_memset16)(dst, value, count); + SK_DECLARE_STATIC_ONCE(once); + static SkMemset16Proc proc = NULL; + SkOnce(&once, choose_memset16, &proc); + SkASSERT(proc != NULL); + + return proc(dst, value, count); } -static SkMemset32Proc choose_memset32() { - SkMemset32Proc proc = SkMemset32GetPlatformProc(); - return proc ? proc : sk_memset32_portable; +static void choose_memset32(SkMemset32Proc* proc) { + *proc = SkMemset32GetPlatformProc(); + if (NULL == *proc) { + *proc = &sk_memset32_portable; + } } void sk_memset32(uint32_t dst[], uint32_t value, int count) { - SK_DECLARE_STATIC_LAZY_FN_PTR(SkMemset32Proc, choice); - return choice.get(choose_memset32)(dst, value, count); + SK_DECLARE_STATIC_ONCE(once); + static SkMemset32Proc proc = NULL; + SkOnce(&once, choose_memset32, &proc); + SkASSERT(proc != NULL); + + return proc(dst, value, count); } -static SkMemcpy32Proc choose_memcpy32() { - SkMemcpy32Proc proc = SkMemcpy32GetPlatformProc(); - return proc ? proc : sk_memcpy32_portable; +static void choose_memcpy32(SkMemcpy32Proc* proc) { + *proc = SkMemcpy32GetPlatformProc(); + if (NULL == *proc) { + *proc = &sk_memcpy32_portable; + } } void sk_memcpy32(uint32_t dst[], const uint32_t src[], int count) { - SK_DECLARE_STATIC_LAZY_FN_PTR(SkMemcpy32Proc, choice); - return choice.get(choose_memcpy32)(dst, src, count); + SK_DECLARE_STATIC_ONCE(once); + static SkMemcpy32Proc proc = NULL; + SkOnce(&once, choose_memcpy32, &proc); + SkASSERT(proc != NULL); + + return proc(dst, src, count); } /////////////////////////////////////////////////////////////////////////////// diff --git a/src/ports/SkAtomics_sync.h b/src/ports/SkAtomics_sync.h index b0d17527f0..45ba63f305 100644 --- a/src/ports/SkAtomics_sync.h +++ b/src/ports/SkAtomics_sync.h @@ -32,12 +32,6 @@ 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 f1b9ec2a62..7454d66055 100644 --- a/src/ports/SkAtomics_win.h +++ b/src/ports/SkAtomics_win.h @@ -40,10 +40,6 @@ static inline bool sk_atomic_cas(int32_t* addr, int32_t before, int32_t after) { return _InterlockedCompareExchange(reinterpret_cast<long*>(addr), after, before) == before; } -static inline void* sk_atomic_cas(void** addr, void* before, void* after) { - return InterlockedCompareExchange(reinterpret_cast<long*>(addr), after, before); -} - static inline void sk_membar_acquire__after_atomic_conditional_inc() { } #endif |