From 104bf697fbd2a0b90b5f01344ee01c8caa1745d0 Mon Sep 17 00:00:00 2001 From: csilvers Date: Mon, 18 May 2009 22:50:20 +0000 Subject: [PATCH] Use the google spinlock code instead of the built-in windows code. The main benefit for perftools is that google spinlocks allow for link-time (static) initialization, which we had to simulate before, yielding bugs and worse performance. git-svn-id: http://gperftools.googlecode.com/svn/trunk@73 6b5cf1ce-ec42-a296-1ba9-69fdba395a50 --- src/base/atomicops-internals-x86-msvc.h | 6 +- src/base/spinlock.cc | 6 ++ src/base/spinlock.h | 12 +++- src/windows/config.h | 15 ---- src/windows/patch_functions.cc | 1 + src/windows/port.cc | 1 + src/windows/port.h | 39 ++++++++-- .../addressmap_unittest.vcproj | 66 +++++++++++++++++ .../libtcmalloc_minimal.vcproj | 26 +++++++ .../low_level_alloc_unittest.vcproj | 49 +++++++++++++ .../packed-cache_test.vcproj | 72 +++++++++++++++++++ vsprojects/tmu-static/tmu-static.vcproj | 28 ++++++++ 12 files changed, 295 insertions(+), 26 deletions(-) diff --git a/src/base/atomicops-internals-x86-msvc.h b/src/base/atomicops-internals-x86-msvc.h index a2c685a..d50894c 100644 --- a/src/base/atomicops-internals-x86-msvc.h +++ b/src/base/atomicops-internals-x86-msvc.h @@ -37,6 +37,9 @@ #ifndef BASE_ATOMICOPS_INTERNALS_X86_MSVC_H_ #define BASE_ATOMICOPS_INTERNALS_X86_MSVC_H_ + +#include +#include #include "base/basictypes.h" // For COMPILE_ASSERT typedef int32 Atomic32; @@ -257,9 +260,6 @@ inline Atomic64 Release_Load(volatile const Atomic64* ptr) { // TBD(vchen): The GNU assembly below must be converted to MSVC inline // assembly. -#include -#include - inline void NotImplementedFatalError(const char *function_name) { fprintf(stderr, "64-bit %s() not implemented on this platform\n", function_name); diff --git a/src/base/spinlock.cc b/src/base/spinlock.cc index 71ab646..cd8de8e 100644 --- a/src/base/spinlock.cc +++ b/src/base/spinlock.cc @@ -33,7 +33,9 @@ #include #include /* For nanosleep() */ +#ifdef HAVE_SCHED_H #include /* For sched_yield() */ +#endif #ifdef HAVE_UNISTD_H #include /* For nanosleep() on Windows, read() */ #endif @@ -78,9 +80,13 @@ void SpinLock::SlowLock() { c--; } +#ifdef HAVE_SCHED_H if (lockword_ == 1) { sched_yield(); // Spinning failed. Let's try to be gentle. } +#else + sleep(0); // best we can do? Useful on windows at least. +#endif while (Acquire_CompareAndSwap(&lockword_, 0, 1) != 0) { // This code was adapted from the ptmalloc2 implementation of diff --git a/src/base/spinlock.h b/src/base/spinlock.h index 92b3287..6761bc0 100644 --- a/src/base/spinlock.h +++ b/src/base/spinlock.h @@ -67,7 +67,9 @@ class LOCKABLE SpinLock { } // Acquire this SpinLock. - inline void Lock() EXCLUSIVE_LOCK_FUNCTION() { + // TODO(csilvers): uncomment the annotation when we figure out how to + // support this macro with 0 args (see thread_annotations.h) + inline void Lock() /*EXCLUSIVE_LOCK_FUNCTION()*/ { if (Acquire_CompareAndSwap(&lockword_, 0, 1) != 0) { SlowLock(); } @@ -87,7 +89,9 @@ class LOCKABLE SpinLock { } // Release this SpinLock, which must be held by the calling thread. - inline void Unlock() UNLOCK_FUNCTION() { + // TODO(csilvers): uncomment the annotation when we figure out how to + // support this macro with 0 args (see thread_annotations.h) + inline void Unlock() /*UNLOCK_FUNCTION()*/ { // This is defined in mutex.cc. extern void SubmitSpinLockProfileData(const void *, int64); @@ -144,7 +148,9 @@ class SCOPED_LOCKABLE SpinLockHolder { : lock_(l) { l->Lock(); } - inline ~SpinLockHolder() UNLOCK_FUNCTION() { lock_->Unlock(); } + // TODO(csilvers): uncomment the annotation when we figure out how to + // support this macro with 0 args (see thread_annotations.h) + inline ~SpinLockHolder() /*UNLOCK_FUNCTION()*/ { lock_->Unlock(); } }; // Catch bug where variable name is omitted, e.g. SpinLockHolder (&lock); #define SpinLockHolder(x) COMPILE_ASSERT(0, spin_lock_decl_missing_var_name) diff --git a/src/windows/config.h b/src/windows/config.h index 2811296..bb7e3d4 100644 --- a/src/windows/config.h +++ b/src/windows/config.h @@ -19,15 +19,6 @@ */ #undef WIN32_OVERRIDE_ALLOCATORS -/* the location of */ -#define HASH_MAP_H - -/* the namespace of hash_map/hash_set */ -#define HASH_NAMESPACE stdext - -/* the location of */ -#define HASH_SET_H - /* Define to 1 if your libc has a snprintf implementation */ #undef HAVE_SNPRINTF @@ -85,12 +76,6 @@ /* Define to 1 if you have the header file. */ #undef HAVE_GRP_H -/* define if the compiler has hash_map */ -#define HAVE_HASH_MAP 1 - -/* define if the compiler has hash_set */ -#define HAVE_HASH_SET 1 - /* Define to 1 if you have the header file. */ #undef HAVE_INTTYPES_H diff --git a/src/windows/patch_functions.cc b/src/windows/patch_functions.cc index a282eb5..2cfb0c4 100644 --- a/src/windows/patch_functions.cc +++ b/src/windows/patch_functions.cc @@ -76,6 +76,7 @@ #include // for _msize and _expand #include // for CreateToolhelp32Snapshot() #include +#include "base/spinlock.h" #include "google/malloc_hook.h" #include "malloc_hook-inl.h" #include "preamble_patcher.h" diff --git a/src/windows/port.cc b/src/windows/port.cc index 5a5ad3b..03ccd48 100644 --- a/src/windows/port.cc +++ b/src/windows/port.cc @@ -42,6 +42,7 @@ #include #include "port.h" #include "base/logging.h" +#include "base/spinlock.h" #include "system-alloc.h" // ----------------------------------------------------------------------- diff --git a/src/windows/port.h b/src/windows/port.h index 9020dce..592f2ae 100644 --- a/src/windows/port.h +++ b/src/windows/port.h @@ -111,7 +111,7 @@ inline void* perftools_pthread_getspecific(DWORD key) { } #define perftools_pthread_setspecific(key, val) \ TlsSetValue((key), (val)) -// NOTE: this is Win98 and later. For Win95 we could use a CRITICAL_SECTION... +// NOTE: this is Win2K and later. For Win98 we could use a CRITICAL_SECTION... #define perftools_pthread_once(once, init) do { \ if (InterlockedCompareExchange(once, 1, 0) == 0) (init)(); \ } while (0) @@ -122,11 +122,16 @@ inline void* perftools_pthread_getspecific(DWORD key) { // things we need to do before main()! So this kind of TLS is safe for us. #define __thread __declspec(thread) +// This code is obsolete, but I keep it around in case we are ever in +// an environment where we can't or don't want to use google spinlocks +// (from base/spinlock.{h,cc}). In that case, uncommenting this out, +// and removing spinlock.cc from the build, should be enough to revert +// back to using native spinlocks. +#if 0 // Windows uses a spinlock internally for its mutexes, making our life easy! // However, the Windows spinlock must always be initialized, making life hard, // since we want LINKER_INITIALIZED. We work around this by having the // linker initialize a bool to 0, and check that before accessing the mutex. -// TODO(csilvers): figure out a faster way. // This replaces spinlock.{h,cc}, and all the stuff it depends on (atomicops) #ifdef __cplusplus class SpinLock { @@ -134,7 +139,10 @@ class SpinLock { SpinLock() : initialize_token_(PTHREAD_ONCE_INIT) {} // Used for global SpinLock vars (see base/spinlock.h for more details). enum StaticInitializer { LINKER_INITIALIZED }; - explicit SpinLock(StaticInitializer) : initialize_token_(PTHREAD_ONCE_INIT) {} + explicit SpinLock(StaticInitializer) : initialize_token_(PTHREAD_ONCE_INIT) { + perftools_pthread_once(&initialize_token_, InitializeMutex); + } + // It's important SpinLock not have a destructor: otherwise we run // into problems when the main thread has exited, but other threads // are still running and try to access a main-thread spinlock. This @@ -144,6 +152,15 @@ class SpinLock { // perfectly fine. But be aware of this for the future! void Lock() { + // You'd thionk this would be unnecessary, since we call + // InitializeMutex() in our constructor. But sometimes Lock() can + // be called before our constructor is! This can only happen in + // global constructors, when this is a global. If we live in + // bar.cc, and some global constructor in foo.cc calls a routine + // in bar.cc that calls this->Lock(), then Lock() may well run + // before our global constructor does. To protect against that, + // we do this check. For SpinLock objects created after main() + // has started, this pthread_once call will always be a noop. perftools_pthread_once(&initialize_token_, InitializeMutex); EnterCriticalSection(&mutex_); } @@ -172,7 +189,12 @@ class SpinLockHolder { // Acquires a spinlock for as long as the scope lasts inline explicit SpinLockHolder(SpinLock* l) : lock_(l) { l->Lock(); } inline ~SpinLockHolder() { lock_->Unlock(); } }; -#endif +#endif // #ifdef __cplusplus + +// This keeps us from using base/spinlock.h's implementation of SpinLock. +#define BASE_SPINLOCK_H_ 1 + +#endif // #if 0 // This replaces testutil.{h,cc} extern PERFTOOLS_DLL_DECL void RunInThread(void (*fn)()); @@ -264,6 +286,14 @@ extern PERFTOOLS_DLL_DECL int getpagesize(); // in port.cc #define random rand #define sleep(t) Sleep(t * 1000) +struct timespec { + int tv_sec; + int tv_nsec; +}; + +#define nanosleep(tm_ptr, ignored) \ + Sleep((tm_ptr)->tv_sec * 1000 + (tm_ptr)->tv_nsec / 1000000) + #ifndef __MINGW32__ #define strtoq _strtoi64 #define strtouq _strtoui64 @@ -284,7 +314,6 @@ extern PERFTOOLS_DLL_DECL void PatchWindowsFunctions(); // windows/port.h defines compatibility APIs for several .h files, which // we therefore shouldn't be #including directly. This hack keeps us from // doing so. TODO(csilvers): do something more principled. -#define BASE_SPINLOCK_H_ 1 #define GOOGLE_MAYBE_THREADS_H_ 1 diff --git a/vsprojects/addressmap_unittest/addressmap_unittest.vcproj b/vsprojects/addressmap_unittest/addressmap_unittest.vcproj index 7a4c242..10930b0 100755 --- a/vsprojects/addressmap_unittest/addressmap_unittest.vcproj +++ b/vsprojects/addressmap_unittest/addressmap_unittest.vcproj @@ -129,6 +129,40 @@ RuntimeLibrary="2"/> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/vsprojects/libtcmalloc_minimal/libtcmalloc_minimal.vcproj b/vsprojects/libtcmalloc_minimal/libtcmalloc_minimal.vcproj index f6af7da..5f2fecc 100755 --- a/vsprojects/libtcmalloc_minimal/libtcmalloc_minimal.vcproj +++ b/vsprojects/libtcmalloc_minimal/libtcmalloc_minimal.vcproj @@ -539,6 +539,23 @@ RuntimeLibrary="2"/> + + + + + + + + + + + + + + diff --git a/vsprojects/low_level_alloc_unittest/low_level_alloc_unittest.vcproj b/vsprojects/low_level_alloc_unittest/low_level_alloc_unittest.vcproj index f51b9f0..165b9c6 100755 --- a/vsprojects/low_level_alloc_unittest/low_level_alloc_unittest.vcproj +++ b/vsprojects/low_level_alloc_unittest/low_level_alloc_unittest.vcproj @@ -129,6 +129,23 @@ RuntimeLibrary="2"/> + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/vsprojects/packed-cache_test/packed-cache_test.vcproj b/vsprojects/packed-cache_test/packed-cache_test.vcproj index d6b22bf..e0b242b 100755 --- a/vsprojects/packed-cache_test/packed-cache_test.vcproj +++ b/vsprojects/packed-cache_test/packed-cache_test.vcproj @@ -129,6 +129,57 @@ RuntimeLibrary="2"/> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/vsprojects/tmu-static/tmu-static.vcproj b/vsprojects/tmu-static/tmu-static.vcproj index 5ff9419..66e7bce 100755 --- a/vsprojects/tmu-static/tmu-static.vcproj +++ b/vsprojects/tmu-static/tmu-static.vcproj @@ -583,6 +583,25 @@ RuntimeLibrary="2"/> + + + + + + + + + + + + + +