Remove WTF::SpinLock
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Aug 2015 02:34:02 +0000 (02:34 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Aug 2015 02:34:02 +0000 (02:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148208

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

Remove the one remaining use of SpinLock.

* API/JSValue.mm:
(handerForStructTag):

Source/WTF:

Remove the SpinLock.h file and remove references to the SpinLock class. Put the old SpinLock
algorithm in LockSpeedTest.cpp - which isn't compiled as part of a WTF or WebKit build - just
so we can still benchmark our locking algorithms against a spinlock baseline.

* WTF.vcxproj/WTF.vcxproj:
* WTF.xcodeproj/project.pbxproj:
* benchmarks/LockSpeedTest.cpp:
* wtf/CMakeLists.txt:
* wtf/Lock.h:
* wtf/SpinLock.h: Removed.
* wtf/WordLock.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@188677 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/API/JSValue.mm
Source/JavaScriptCore/ChangeLog
Source/WTF/ChangeLog
Source/WTF/WTF.vcxproj/WTF.vcxproj
Source/WTF/WTF.xcodeproj/project.pbxproj
Source/WTF/benchmarks/LockSpeedTest.cpp
Source/WTF/wtf/CMakeLists.txt
Source/WTF/wtf/Lock.h
Source/WTF/wtf/SpinLock.h [deleted file]
Source/WTF/wtf/WordLock.h

index 11be6b6..bab2862 100644 (file)
@@ -41,8 +41,8 @@
 #import "StrongInlines.h"
 #import <wtf/HashMap.h>
 #import <wtf/HashSet.h>
+#import <wtf/Lock.h>
 #import <wtf/ObjcRuntimeExtras.h>
-#import <wtf/SpinLock.h>
 #import <wtf/Vector.h>
 #import <wtf/text/WTFString.h>
 #import <wtf/text/StringHash.h>
@@ -1114,8 +1114,8 @@ static StructHandlers* createStructHandlerMap()
 
 static StructTagHandler* handerForStructTag(const char* encodedType)
 {
-    static StaticSpinLock handerForStructTagLock;
-    SpinLockHolder lockHolder(&handerForStructTagLock);
+    static StaticLock handerForStructTagLock;
+    LockHolder lockHolder(&handerForStructTagLock);
 
     static StructHandlers* structHandlers = createStructHandlerMap();
 
index 7b8313b..538dc37 100644 (file)
@@ -1,3 +1,15 @@
+2015-08-19  Filip Pizlo  <fpizlo@apple.com>
+
+        Remove WTF::SpinLock
+        https://bugs.webkit.org/show_bug.cgi?id=148208
+
+        Reviewed by Geoffrey Garen.
+
+        Remove the one remaining use of SpinLock.
+
+        * API/JSValue.mm:
+        (handerForStructTag):
+
 2015-08-19  Geoffrey Garen  <ggaren@apple.com>
 
         clearCode() should clear code
index 61c2feb..a833efa 100644 (file)
@@ -1,3 +1,22 @@
+2015-08-19  Filip Pizlo  <fpizlo@apple.com>
+
+        Remove WTF::SpinLock
+        https://bugs.webkit.org/show_bug.cgi?id=148208
+
+        Reviewed by Geoffrey Garen.
+
+        Remove the SpinLock.h file and remove references to the SpinLock class. Put the old SpinLock
+        algorithm in LockSpeedTest.cpp - which isn't compiled as part of a WTF or WebKit build - just
+        so we can still benchmark our locking algorithms against a spinlock baseline.
+
+        * WTF.vcxproj/WTF.vcxproj:
+        * WTF.xcodeproj/project.pbxproj:
+        * benchmarks/LockSpeedTest.cpp:
+        * wtf/CMakeLists.txt:
+        * wtf/Lock.h:
+        * wtf/SpinLock.h: Removed.
+        * wtf/WordLock.h:
+
 2015-08-19  Alex Christensen  <achristensen@webkit.org>
 
         CMake Windows build should not include files directly from other Source directories
index e1ce44d..d116025 100644 (file)
     <ClInclude Include="..\wtf\SHA1.h" />
     <ClInclude Include="..\wtf\SinglyLinkedList.h" />
     <ClInclude Include="..\wtf\SixCharacterHash.h" />
-    <ClInclude Include="..\wtf\SpinLock.h" />
     <ClInclude Include="..\wtf\StackBounds.h" />
     <ClInclude Include="..\wtf\StaticConstructors.h" />
     <ClInclude Include="..\wtf\StdLibExtras.h" />
index fe9c564..21d1a0f 100644 (file)
                A8A473C9151A825B004123FF /* Functional.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A472A7151A825A004123FF /* Functional.h */; };
                A8A473CA151A825B004123FF /* GetPtr.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A472A8151A825A004123FF /* GetPtr.h */; };
                A8A473D3151A825B004123FF /* HashCountedSet.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A472B3151A825A004123FF /* HashCountedSet.h */; };
-               A8A4742D151A825B004123FF /* Hasher.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A47314151A825B004123FF /* Hasher.h */; };
                A8A473D4151A825B004123FF /* HashFunctions.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A472B4151A825A004123FF /* HashFunctions.h */; };
                A8A473D5151A825B004123FF /* HashIterators.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A472B5151A825A004123FF /* HashIterators.h */; };
                A8A473D6151A825B004123FF /* HashMap.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A472B6151A825A004123FF /* HashMap.h */; };
                A8A47429151A825B004123FF /* StaticConstructors.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A47310151A825B004123FF /* StaticConstructors.h */; };
                A8A4742A151A825B004123FF /* StdLibExtras.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A47311151A825B004123FF /* StdLibExtras.h */; };
                A8A4742C151A825B004123FF /* StringExtras.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A47313151A825B004123FF /* StringExtras.h */; };
+               A8A4742D151A825B004123FF /* Hasher.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A47314151A825B004123FF /* Hasher.h */; };
                A8A47433151A825B004123FF /* TemporaryChange.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A4731A151A825B004123FF /* TemporaryChange.h */; };
                A8A47434151A825B004123FF /* ASCIIFastPath.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A4731C151A825B004123FF /* ASCIIFastPath.h */; };
                A8A47435151A825B004123FF /* AtomicString.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A8A4731D151A825B004123FF /* AtomicString.cpp */; };
                E4A0AD3D1A96253C00536DF6 /* WorkQueueCocoa.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E4A0AD3C1A96253C00536DF6 /* WorkQueueCocoa.cpp */; };
                EB95E1F0161A72410089A2F5 /* ByteOrder.h in Headers */ = {isa = PBXBuildFile; fileRef = EB95E1EF161A72410089A2F5 /* ByteOrder.h */; };
                FE8225311B2A1E5B00BA68FD /* NakedPtr.h in Headers */ = {isa = PBXBuildFile; fileRef = FE8225301B2A1E5B00BA68FD /* NakedPtr.h */; };
-               FE91E8811AB2A0200099895F /* SpinLock.h in Headers */ = {isa = PBXBuildFile; fileRef = FE91E8801AB2A0200099895F /* SpinLock.h */; };
                FEDACD3D1630F83F00C69634 /* StackStats.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FEDACD3B1630F83F00C69634 /* StackStats.cpp */; };
                FEDACD3E1630F83F00C69634 /* StackStats.h in Headers */ = {isa = PBXBuildFile; fileRef = FEDACD3C1630F83F00C69634 /* StackStats.h */; };
 /* End PBXBuildFile section */
                A8A472A7151A825A004123FF /* Functional.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Functional.h; sourceTree = "<group>"; };
                A8A472A8151A825A004123FF /* GetPtr.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = GetPtr.h; sourceTree = "<group>"; };
                A8A472B3151A825A004123FF /* HashCountedSet.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = HashCountedSet.h; sourceTree = "<group>"; };
-               A8A47314151A825B004123FF /* Hasher.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Hasher.h; sourceTree = "<group>"; };
                A8A472B4151A825A004123FF /* HashFunctions.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = HashFunctions.h; sourceTree = "<group>"; };
                A8A472B5151A825A004123FF /* HashIterators.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = HashIterators.h; sourceTree = "<group>"; };
                A8A472B6151A825A004123FF /* HashMap.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = HashMap.h; sourceTree = "<group>"; };
                A8A47310151A825B004123FF /* StaticConstructors.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = StaticConstructors.h; sourceTree = "<group>"; };
                A8A47311151A825B004123FF /* StdLibExtras.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = StdLibExtras.h; sourceTree = "<group>"; };
                A8A47313151A825B004123FF /* StringExtras.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = StringExtras.h; sourceTree = "<group>"; };
+               A8A47314151A825B004123FF /* Hasher.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Hasher.h; sourceTree = "<group>"; };
                A8A4731A151A825B004123FF /* TemporaryChange.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TemporaryChange.h; sourceTree = "<group>"; };
                A8A4731C151A825B004123FF /* ASCIIFastPath.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASCIIFastPath.h; sourceTree = "<group>"; };
                A8A4731D151A825B004123FF /* AtomicString.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = AtomicString.cpp; sourceTree = "<group>"; };
                E4A0AD3C1A96253C00536DF6 /* WorkQueueCocoa.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WorkQueueCocoa.cpp; sourceTree = "<group>"; };
                EB95E1EF161A72410089A2F5 /* ByteOrder.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ByteOrder.h; sourceTree = "<group>"; };
                FE8225301B2A1E5B00BA68FD /* NakedPtr.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = NakedPtr.h; sourceTree = "<group>"; };
-               FE91E8801AB2A0200099895F /* SpinLock.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SpinLock.h; sourceTree = "<group>"; };
                FEDACD3B1630F83F00C69634 /* StackStats.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = StackStats.cpp; sourceTree = "<group>"; };
                FEDACD3C1630F83F00C69634 /* StackStats.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = StackStats.h; sourceTree = "<group>"; };
 /* End PBXFileReference section */
                                A748745017A0BDAE00FA04CB /* SixCharacterHash.h */,
                                A8A4730C151A825B004123FF /* SizeLimits.cpp */,
                                A8A4730D151A825B004123FF /* Spectrum.h */,
-                               FE91E8801AB2A0200099895F /* SpinLock.h */,
                                A8A4730E151A825B004123FF /* StackBounds.cpp */,
                                A8A4730F151A825B004123FF /* StackBounds.h */,
                                FEDACD3B1630F83F00C69634 /* StackStats.cpp */,
                                A8A4743D151A825B004123FF /* StringBuilder.h in Headers */,
                                70ECA60E1B02426800449739 /* SymbolImpl.h in Headers */,
                                A8A4743E151A825B004123FF /* StringConcatenate.h in Headers */,
-                               FE91E8811AB2A0200099895F /* SpinLock.h in Headers */,
                                A8A4742C151A825B004123FF /* StringExtras.h in Headers */,
                                A8A4743F151A825B004123FF /* StringHash.h in Headers */,
                                FE8225311B2A1E5B00BA68FD /* NakedPtr.h in Headers */,
index 8cd4bd3..19bacea 100644 (file)
@@ -31,7 +31,6 @@
 #include <unistd.h>
 #include <wtf/CurrentTime.h>
 #include <wtf/Lock.h>
-#include <wtf/SpinLock.h>
 #include <wtf/StdLibExtras.h>
 #include <wtf/Threading.h>
 #include <wtf/ThreadingPrimitives.h>
 
 namespace {
 
+// This is the old WTF::SpinLock class, included here so that we can still compare our new locks to a
+// spinlock baseline.
+class SpinLock {
+public:
+    SpinLock()
+    {
+        m_lock.store(0, std::memory_order_relaxed);
+    }
+
+    void lock()
+    {
+        while (!m_lock.compareExchangeWeak(0, 1, std::memory_order_acquire))
+            std::this_thread::yield();
+    }
+
+    void unlock()
+    {
+        m_lock.store(0, std::memory_order_release);
+    }
+
+    bool isLocked() const
+    {
+        return m_lock.load(std::memory_order_acquire);
+    }
+
+private:
+    Atomic<unsigned> m_lock;
+};
+
 unsigned numThreadGroups;
 unsigned numThreadsPerGroup;
 unsigned workPerCriticalSection;
index 23be066..6dd7bb7 100644 (file)
@@ -84,7 +84,6 @@ set(WTF_HEADERS
     SHA1.h
     SaturatedArithmetic.h
     SegmentedVector.h
-    SpinLock.h
     StackBounds.h
     StackStats.h
     StaticConstructors.h
index 7f86c4e..8395653 100644 (file)
@@ -38,10 +38,10 @@ struct LockInspector;
 namespace WTF {
 
 // This is a fully adaptive mutex that only requires 1 byte of storage. It has fast paths that are
-// competetive to SpinLock (uncontended locking is inlined and is just a CAS, microcontention is
-// handled by spinning and yielding), and a slow path that is competetive to Mutex (if a lock cannot
-// be acquired in a short period of time, the thread is put to sleep until the lock is available
-// again). It uses less memory than either SpinLock or Mutex.
+// competetive to a spinlock (uncontended locking is inlined and is just a CAS, microcontention is
+// handled by spinning and yielding), and a slow path that is competetive to std::mutex (if a lock
+// cannot be acquired in a short period of time, the thread is put to sleep until the lock is available
+// again). It uses less memory than a std::mutex.
 
 // This is a struct without a constructor or destructor so that it can be statically initialized.
 // Use Lock in instance variables.
diff --git a/Source/WTF/wtf/SpinLock.h b/Source/WTF/wtf/SpinLock.h
deleted file mode 100644 (file)
index 58604b0..0000000
+++ /dev/null
@@ -1,87 +0,0 @@
-/*
- * Copyright (C) 2015 Apple Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
- * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
- * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
- * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
- * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef SpinLock_h
-#define SpinLock_h
-
-#include <thread>
-#include <wtf/Atomics.h>
-#include <wtf/Locker.h>
-
-namespace WTF {
-
-// SpinLock is a very simple lock implementation that has extremely fast lock/unlock for very small
-// uncontended critical sections. However, it will exhibit bad performance degradation when the lock
-// becomes contended: the thread trying to acquire the lock will simply waste CPU cycles.
-//
-// For most (all?) locking use cases, it's better to use Lock (see wtf/Lock.h). That uses only a bit
-// more memory (8 bytes instead of 4 on 64-bit), and is only a bit slower in the uncontended case
-// (Lock needs CAS to unlock, while SpinLock doesn't), but will burn a lot less CPU time - for 10
-// threads acquiring a 50 microsecond critical section, Lock will use up to 100x less kernel CPU time
-// than SpinLock.
-
-// SpinLockBase is a struct without an explicitly defined constructors so that
-// it can be initialized at compile time. See StaticSpinLock below.
-struct SpinLockBase {
-
-    void lock()
-    {
-        while (!m_lock.compareExchangeWeak(0, 1, std::memory_order_acquire))
-            std::this_thread::yield();
-    }
-
-    void unlock()
-    {
-        m_lock.store(0, std::memory_order_release);
-    }
-
-    bool isLocked() const
-    {
-        return m_lock.load(std::memory_order_acquire);
-    }
-    
-    Atomic<unsigned> m_lock;
-};
-
-// SpinLock is for use as instance variables in structs and classes, not as
-// statics and globals.
-struct SpinLock : public SpinLockBase {
-    SpinLock()
-    {
-        m_lock.store(0, std::memory_order_relaxed);
-    }
-};
-
-// StaticSpinLock is for use as statics and globals, not as instance variables.
-typedef SpinLockBase StaticSpinLock;
-typedef Locker<SpinLockBase> SpinLockHolder;
-
-} // namespace WTF
-
-using WTF::StaticSpinLock;
-using WTF::SpinLock;
-using WTF::SpinLockHolder;
-
-#endif // SpinLock_h
index ea399e7..c7e208b 100644 (file)
@@ -38,8 +38,8 @@ struct LockInspector;
 namespace WTF {
 
 // A WordLock is a fully adaptive mutex that uses sizeof(void*) storage. It has a fast path that is
-// similar to SpinLock, and a slow path that is similar to Mutex. In most cases, you should use Lock
-// instead. WordLock sits lower in the stack and is used to implement Lock, so Lock is the main
+// similar to a spinlock, and a slow path that is similar to std::mutex. In most cases, you should use
+// Lock instead. WordLock sits lower in the stack and is used to implement Lock, so Lock is the main
 // client of WordLock.
 
 class WordLock {