Modernize HashTable threading code
https://bugs.webkit.org/show_bug.cgi?id=127621
Reviewed by Darin Adler.
Source/WebCore:
Explicitly include headers that used to be brought in by HashTable.h
* platform/DragData.h:
Change a Windows-specific typedef to avoid having to include WindDef.h from a header.
* platform/audio/AudioSession.h:
* platform/network/cf/SocketStreamHandle.h:
Source/WebKit/win:
Explicitly include headers that used to be brought in by HashTable.h
* WebLocalizableStrings.cpp:
Source/WebKit2:
Explicitly include headers that used to be brought in by HashTable.h
* Shared/BlockingResponseMap.h:
Source/WTF:
Use std::mutex and std::atomic instead of WTF threading primitives.
* wtf/DynamicAnnotations.h:
Include Platform.h here since this file relies on USE macros.
* wtf/HashTable.cpp:
(WTF::HashTableStats::recordCollisionAtCount):
Change this to take an unsigned.
(WTF::HashTableStats::dumpStats):
* wtf/HashTable.h:
(WTF::KeyTraits>::HashTable):
(WTF::KeyTraits>::remove):
(WTF::KeyTraits>::invalidateIterators):
Use a single probe counter.
(WTF::addIterator):
(WTF::removeIterator):
Tools:
Explicitly include headers that used to be brought in by HashTable.h
* DumpRenderTree/JavaScriptThreading.cpp:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@162774 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
index e3c1ad1..27a8104 100644
--- a/Source/WTF/ChangeLog
+++ b/Source/WTF/ChangeLog
@@ -1,3 +1,29 @@
+2014-01-25 Anders Carlsson <andersca@apple.com>
+
+ Modernize HashTable threading code
+ https://bugs.webkit.org/show_bug.cgi?id=127621
+
+ Reviewed by Darin Adler.
+
+ Use std::mutex and std::atomic instead of WTF threading primitives.
+
+ * wtf/DynamicAnnotations.h:
+ Include Platform.h here since this file relies on USE macros.
+
+ * wtf/HashTable.cpp:
+ (WTF::HashTableStats::recordCollisionAtCount):
+ Change this to take an unsigned.
+
+ (WTF::HashTableStats::dumpStats):
+ * wtf/HashTable.h:
+ (WTF::KeyTraits>::HashTable):
+ (WTF::KeyTraits>::remove):
+ (WTF::KeyTraits>::invalidateIterators):
+ Use a single probe counter.
+
+ (WTF::addIterator):
+ (WTF::removeIterator):
+
2014-01-25 Darin Adler <darin@apple.com>
Restore alphabetical order in Compiler.h
diff --git a/Source/WTF/wtf/DynamicAnnotations.h b/Source/WTF/wtf/DynamicAnnotations.h
index 38acce3..2d5a5a9 100644
--- a/Source/WTF/wtf/DynamicAnnotations.h
+++ b/Source/WTF/wtf/DynamicAnnotations.h
@@ -27,6 +27,8 @@
#ifndef WTF_DynamicAnnotations_h
#define WTF_DynamicAnnotations_h
+#include <wtf/Platform.h>
+
/* This file defines dynamic annotations for use with dynamic analysis
* tool such as ThreadSanitizer, Valgrind, etc.
*
diff --git a/Source/WTF/wtf/HashTable.cpp b/Source/WTF/wtf/HashTable.cpp
index a578f52..458dd53 100644
--- a/Source/WTF/wtf/HashTable.cpp
+++ b/Source/WTF/wtf/HashTable.cpp
@@ -27,13 +27,14 @@
#if DUMP_HASHTABLE_STATS
-int HashTableStats::numAccesses;
-int HashTableStats::numCollisions;
-int HashTableStats::collisionGraph[4096];
-int HashTableStats::maxCollisions;
-int HashTableStats::numRehashes;
-int HashTableStats::numRemoves;
-int HashTableStats::numReinserts;
+std::atomic<unsigned> HashTableStats::numAccesses;
+std::atomic<unsigned> HashTableStats::numRehashes;
+std::atomic<unsigned> HashTableStats::numRemoves;
+std::atomic<unsigned> HashTableStats::numReinserts;
+
+unsigned HashTableStats::numCollisions;
+unsigned HashTableStats::collisionGraph[4096];
+unsigned HashTableStats::maxCollisions;
static std::mutex& hashTableStatsMutex()
{
@@ -46,7 +47,7 @@
return *mutex;
}
-void HashTableStats::recordCollisionAtCount(int count)
+void HashTableStats::recordCollisionAtCount(unsigned count)
{
std::lock_guard<std::mutex> lock(hashTableStatsMutex());
@@ -61,14 +62,14 @@
std::lock_guard<std::mutex> lock(hashTableStatsMutex());
dataLogF("\nWTF::HashTable statistics\n\n");
- dataLogF("%d accesses\n", numAccesses);
+ dataLogF("%u accesses\n", numAccesses.load());
dataLogF("%d total collisions, average %.2f probes per access\n", numCollisions, 1.0 * (numAccesses + numCollisions) / numAccesses);
dataLogF("longest collision chain: %d\n", maxCollisions);
- for (int i = 1; i <= maxCollisions; i++) {
- dataLogF(" %d lookups with exactly %d collisions (%.2f%% , %.2f%% with this many or more)\n", collisionGraph[i], i, 100.0 * (collisionGraph[i] - collisionGraph[i+1]) / numAccesses, 100.0 * collisionGraph[i] / numAccesses);
+ for (unsigned i = 1; i <= maxCollisions; i++) {
+ dataLogF(" %u lookups with exactly %u collisions (%.2f%% , %.2f%% with this many or more)\n", collisionGraph[i], i, 100.0 * (collisionGraph[i] - collisionGraph[i+1]) / numAccesses, 100.0 * collisionGraph[i] / numAccesses);
}
- dataLogF("%d rehashes\n", numRehashes);
- dataLogF("%d reinserts\n", numReinserts);
+ dataLogF("%d rehashes\n", numRehashes.load());
+ dataLogF("%d reinserts\n", numReinserts.load());
}
#endif
diff --git a/Source/WTF/wtf/HashTable.h b/Source/WTF/wtf/HashTable.h
index 7aa1915..8519ae3 100644
--- a/Source/WTF/wtf/HashTable.h
+++ b/Source/WTF/wtf/HashTable.h
@@ -22,6 +22,8 @@
#ifndef WTF_HashTable_h
#define WTF_HashTable_h
+#include <atomic>
+#include <mutex>
#include <string.h>
#include <type_traits>
#include <utility>
@@ -29,15 +31,8 @@
#include <wtf/FastMalloc.h>
#include <wtf/HashTraits.h>
#include <wtf/StdLibExtras.h>
-#include <wtf/Threading.h>
#include <wtf/ValueCheck.h>
-#ifndef NDEBUG
-// Required for CHECK_HASHTABLE_ITERATORS.
-#include <wtf/OwnPtr.h>
-#include <wtf/PassOwnPtr.h>
-#endif
-
#define DUMP_HASHTABLE_STATS 0
#define DUMP_HASHTABLE_STATS_PER_TABLE 0
@@ -62,17 +57,17 @@
struct HashTableStats {
// The following variables are all atomically incremented when modified.
- WTF_EXPORTDATA static int numAccesses;
- WTF_EXPORTDATA static int numRehashes;
- WTF_EXPORTDATA static int numRemoves;
- WTF_EXPORTDATA static int numReinserts;
+ WTF_EXPORTDATA static std::atomic<unsigned> numAccesses;
+ WTF_EXPORTDATA static std::atomic<unsigned> numRehashes;
+ WTF_EXPORTDATA static std::atomic<unsigned> numRemoves;
+ WTF_EXPORTDATA static std::atomic<unsigned> numReinserts;
// The following variables are only modified in the recordCollisionAtCount method within a mutex.
- WTF_EXPORTDATA static int maxCollisions;
- WTF_EXPORTDATA static int numCollisions;
- WTF_EXPORTDATA static int collisionGraph[4096];
+ WTF_EXPORTDATA static unsigned maxCollisions;
+ WTF_EXPORTDATA static unsigned numCollisions;
+ WTF_EXPORTDATA static unsigned collisionGraph[4096];
- WTF_EXPORT_PRIVATE static void recordCollisionAtCount(int count);
+ WTF_EXPORT_PRIVATE static void recordCollisionAtCount(unsigned count);
WTF_EXPORT_PRIVATE static void dumpStats();
};
@@ -483,12 +478,12 @@
// All access to m_iterators should be guarded with m_mutex.
mutable const_iterator* m_iterators;
// Use OwnPtr so HashTable can still be memmove'd or memcpy'ed.
- mutable std::unique_ptr<Mutex> m_mutex;
+ mutable std::unique_ptr<std::mutex> m_mutex;
#endif
#if DUMP_HASHTABLE_STATS_PER_TABLE
public:
- mutable OwnPtr<Stats> m_stats;
+ mutable std::unique_ptr<Stats> m_stats;
#endif
};
@@ -539,7 +534,7 @@
, m_deletedCount(0)
#if CHECK_HASHTABLE_ITERATORS
, m_iterators(0)
- , m_mutex(std::make_unique<Mutex>())
+ , m_mutex(std::make_unique<std::mutex>())
#endif
#if DUMP_HASHTABLE_STATS_PER_TABLE
, m_stats(std::make_unique<Stats>())
@@ -599,13 +594,12 @@
return 0;
#if DUMP_HASHTABLE_STATS
- atomicIncrement(&HashTableStats::numAccesses);
- int probeCount = 0;
+ ++HashTableStats::numAccesses;
+ unsigned probeCount = 0;
#endif
#if DUMP_HASHTABLE_STATS_PER_TABLE
++m_stats->numAccesses;
- int perTableProbeCount = 0;
#endif
while (1) {
@@ -631,8 +625,7 @@
#endif
#if DUMP_HASHTABLE_STATS_PER_TABLE
- ++perTableProbeCount;
- m_stats->recordCollisionAtCount(perTableProbeCount);
+ m_stats->recordCollisionAtCount(probeCount);
#endif
if (k == 0)
@@ -655,13 +648,12 @@
int i = h & sizeMask;
#if DUMP_HASHTABLE_STATS
- atomicIncrement(&HashTableStats::numAccesses);
+ ++HashTableStats::numAccesses;
int probeCount = 0;
#endif
#if DUMP_HASHTABLE_STATS_PER_TABLE
++m_stats->numAccesses;
- int perTableProbeCount = 0;
#endif
ValueType* deletedEntry = 0;
@@ -694,8 +686,7 @@
#endif
#if DUMP_HASHTABLE_STATS_PER_TABLE
- ++perTableProbeCount;
- m_stats->recordCollisionAtCount(perTableProbeCount);
+ m_stats->recordCollisionAtCount(probeCount);
#endif
if (k == 0)
@@ -718,13 +709,12 @@
int i = h & sizeMask;
#if DUMP_HASHTABLE_STATS
- atomicIncrement(&HashTableStats::numAccesses);
- int probeCount = 0;
+ ++HashTableStats::numAccesses;
+ unsigned probeCount = 0;
#endif
#if DUMP_HASHTABLE_STATS_PER_TABLE
++m_stats->numAccesses;
- int perTableProbeCount = 0;
#endif
ValueType* deletedEntry = 0;
@@ -757,8 +747,7 @@
#endif
#if DUMP_HASHTABLE_STATS_PER_TABLE
- ++perTableProbeCount;
- m_stats->recordCollisionAtCount(perTableProbeCount);
+ m_stats->recordCollisionAtCount(probeCount);
#endif
if (k == 0)
@@ -814,13 +803,12 @@
int i = h & sizeMask;
#if DUMP_HASHTABLE_STATS
- atomicIncrement(&HashTableStats::numAccesses);
- int probeCount = 0;
+ ++HashTableStats::numAccesses;
+ unsigned probeCount = 0;
#endif
#if DUMP_HASHTABLE_STATS_PER_TABLE
++m_stats->numAccesses;
- int perTableProbeCount = 0;
#endif
ValueType* deletedEntry = 0;
@@ -853,8 +841,7 @@
#endif
#if DUMP_HASHTABLE_STATS_PER_TABLE
- ++perTableProbeCount;
- m_stats->recordCollisionAtCount(perTableProbeCount);
+ m_stats->recordCollisionAtCount(probeCount);
#endif
if (k == 0)
@@ -924,7 +911,7 @@
ASSERT(!lookupForWriting(Extractor::extract(entry)).second);
ASSERT(!isDeletedBucket(*(lookupForWriting(Extractor::extract(entry)).first)));
#if DUMP_HASHTABLE_STATS
- atomicIncrement(&HashTableStats::numReinserts);
+ ++HashTableStats::numReinserts;
#endif
#if DUMP_HASHTABLE_STATS_PER_TABLE
++m_stats->numReinserts;
@@ -994,7 +981,7 @@
void HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::remove(ValueType* pos)
{
#if DUMP_HASHTABLE_STATS
- atomicIncrement(&HashTableStats::numRemoves);
+ ++HashTableStats::numRemoves;
#endif
#if DUMP_HASHTABLE_STATS_PER_TABLE
++m_stats->numRemoves;
@@ -1090,7 +1077,7 @@
#if DUMP_HASHTABLE_STATS
if (oldTableSize != 0)
- atomicIncrement(&HashTableStats::numRehashes);
+ ++HashTableStats::numRehashes;
#endif
#if DUMP_HASHTABLE_STATS_PER_TABLE
@@ -1147,7 +1134,7 @@
, m_deletedCount(0)
#if CHECK_HASHTABLE_ITERATORS
, m_iterators(0)
- , m_mutex(std::make_unique<Mutex>())
+ , m_mutex(std::make_unique<std::mutex>())
#endif
#if DUMP_HASHTABLE_STATS_PER_TABLE
, m_stats(std::make_unique<Stats>(*other.m_stats))
@@ -1252,7 +1239,7 @@
template<typename Key, typename Value, typename Extractor, typename HashFunctions, typename Traits, typename KeyTraits>
void HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::invalidateIterators()
{
- MutexLocker lock(*m_mutex);
+ std::lock_guard<std::mutex> lock(*m_mutex);
const_iterator* next;
for (const_iterator* p = m_iterators; p; p = next) {
next = p->m_next;
@@ -1274,7 +1261,7 @@
if (!table) {
it->m_next = 0;
} else {
- MutexLocker lock(*table->m_mutex);
+ std::lock_guard<std::mutex> lock(*table->m_mutex);
ASSERT(table->m_iterators != it);
it->m_next = table->m_iterators;
table->m_iterators = it;
@@ -1296,7 +1283,7 @@
ASSERT(!it->m_next);
ASSERT(!it->m_previous);
} else {
- MutexLocker lock(*it->m_table->m_mutex);
+ std::lock_guard<std::mutex> lock(*it->m_table->m_mutex);
if (it->m_next) {
ASSERT(it->m_next->m_previous == it);
it->m_next->m_previous = it->m_previous;
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 9c59178..ffc15d4 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,18 @@
+2014-01-25 Anders Carlsson <andersca@apple.com>
+
+ Modernize HashTable threading code
+ https://bugs.webkit.org/show_bug.cgi?id=127621
+
+ Reviewed by Darin Adler.
+
+ Explicitly include headers that used to be brought in by HashTable.h
+
+ * platform/DragData.h:
+ Change a Windows-specific typedef to avoid having to include WindDef.h from a header.
+
+ * platform/audio/AudioSession.h:
+ * platform/network/cf/SocketStreamHandle.h:
+
2014-01-25 Zan Dobersek <zdobersek@igalia.com>
Move CSSGroupingRule, CSSStyleSheet, ElementRuleCollector to std::unique_ptr
diff --git a/Source/WebCore/platform/DragData.h b/Source/WebCore/platform/DragData.h
index 8fc0163..ae91079 100644
--- a/Source/WebCore/platform/DragData.h
+++ b/Source/WebCore/platform/DragData.h
@@ -74,7 +74,7 @@
};
#if PLATFORM(WIN)
-typedef HashMap<UINT, Vector<String>> DragDataMap;
+typedef HashMap<unsigned, Vector<String>> DragDataMap;
#endif
class DragData {
diff --git a/Source/WebCore/platform/audio/AudioSession.h b/Source/WebCore/platform/audio/AudioSession.h
index eb12773..4fff5a6 100644
--- a/Source/WebCore/platform/audio/AudioSession.h
+++ b/Source/WebCore/platform/audio/AudioSession.h
@@ -30,6 +30,7 @@
#include <memory>
#include <wtf/HashSet.h>
+#include <wtf/Noncopyable.h>
namespace WebCore {
diff --git a/Source/WebCore/platform/network/cf/SocketStreamHandle.h b/Source/WebCore/platform/network/cf/SocketStreamHandle.h
index 8c1a4e1..5855aad 100644
--- a/Source/WebCore/platform/network/cf/SocketStreamHandle.h
+++ b/Source/WebCore/platform/network/cf/SocketStreamHandle.h
@@ -35,6 +35,7 @@
#include "AuthenticationClient.h"
#include "SocketStreamHandleBase.h"
#include <wtf/RetainPtr.h>
+#include <wtf/ThreadSafeRefCounted.h>
typedef struct __CFHTTPMessage* CFHTTPMessageRef;
diff --git a/Source/WebCore/platform/win/WindowMessageBroadcaster.h b/Source/WebCore/platform/win/WindowMessageBroadcaster.h
index d36c233..bc8f828 100644
--- a/Source/WebCore/platform/win/WindowMessageBroadcaster.h
+++ b/Source/WebCore/platform/win/WindowMessageBroadcaster.h
@@ -31,6 +31,7 @@
#include <wtf/HashMap.h>
#include <wtf/HashSet.h>
+#include <wtf/Noncopyable.h>
namespace WebCore {
diff --git a/Source/WebCore/rendering/svg/SVGResourcesCycleSolver.h b/Source/WebCore/rendering/svg/SVGResourcesCycleSolver.h
index 5bac21e..fd95a74 100644
--- a/Source/WebCore/rendering/svg/SVGResourcesCycleSolver.h
+++ b/Source/WebCore/rendering/svg/SVGResourcesCycleSolver.h
@@ -22,6 +22,7 @@
#if ENABLE(SVG)
#include <wtf/HashSet.h>
+#include <wtf/Noncopyable.h>
namespace WebCore {
diff --git a/Source/WebKit/win/ChangeLog b/Source/WebKit/win/ChangeLog
index bc8db44..c955f48 100644
--- a/Source/WebKit/win/ChangeLog
+++ b/Source/WebKit/win/ChangeLog
@@ -1,3 +1,14 @@
+2014-01-25 Anders Carlsson <andersca@apple.com>
+
+ Modernize HashTable threading code
+ https://bugs.webkit.org/show_bug.cgi?id=127621
+
+ Reviewed by Darin Adler.
+
+ Explicitly include headers that used to be brought in by HashTable.h
+
+ * WebLocalizableStrings.cpp:
+
2014-01-24 Anders Carlsson <andersca@apple.com>
Remove back/forward list related functions from Page
diff --git a/Source/WebKit/win/WebLocalizableStrings.cpp b/Source/WebKit/win/WebLocalizableStrings.cpp
index 442a801..957b3ad 100644
--- a/Source/WebKit/win/WebLocalizableStrings.cpp
+++ b/Source/WebKit/win/WebLocalizableStrings.cpp
@@ -36,6 +36,7 @@
#include <wtf/HashMap.h>
#include <wtf/RetainPtr.h>
#include <wtf/StdLibExtras.h>
+#include <wtf/ThreadingPrimitives.h>
#include <CoreFoundation/CoreFoundation.h>
class LocalizedString;
diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
index 04f3c9f..d1db3bc 100644
--- a/Source/WebKit2/ChangeLog
+++ b/Source/WebKit2/ChangeLog
@@ -1,3 +1,14 @@
+2014-01-25 Anders Carlsson <andersca@apple.com>
+
+ Modernize HashTable threading code
+ https://bugs.webkit.org/show_bug.cgi?id=127621
+
+ Reviewed by Darin Adler.
+
+ Explicitly include headers that used to be brought in by HashTable.h
+
+ * Shared/BlockingResponseMap.h:
+
2014-01-25 Zan Dobersek <zdobersek@igalia.com>
[GTK] Remove null check for WebPopupMenuProxy::Client in WebPopupMenuProxyGtk::showPopupMenu
diff --git a/Source/WebKit2/Shared/BlockingResponseMap.h b/Source/WebKit2/Shared/BlockingResponseMap.h
index 11ace75..f4529af 100644
--- a/Source/WebKit2/Shared/BlockingResponseMap.h
+++ b/Source/WebKit2/Shared/BlockingResponseMap.h
@@ -28,6 +28,7 @@
#include <condition_variable>
#include <wtf/HashMap.h>
+#include <wtf/Noncopyable.h>
template<typename T>
class BlockingResponseMap {
diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 927dde5..2146152 100644
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,14 @@
+2014-01-25 Anders Carlsson <andersca@apple.com>
+
+ Modernize HashTable threading code
+ https://bugs.webkit.org/show_bug.cgi?id=127621
+
+ Reviewed by Darin Adler.
+
+ Explicitly include headers that used to be brought in by HashTable.h
+
+ * DumpRenderTree/JavaScriptThreading.cpp:
+
2014-01-24 Eric Carlson <eric.carlson@apple.com>
Unreviewed. Add Philip Jägenstedt to watch and contributor lists.
diff --git a/Tools/DumpRenderTree/JavaScriptThreading.cpp b/Tools/DumpRenderTree/JavaScriptThreading.cpp
index 66de156..5d673b8 100644
--- a/Tools/DumpRenderTree/JavaScriptThreading.cpp
+++ b/Tools/DumpRenderTree/JavaScriptThreading.cpp
@@ -36,6 +36,8 @@
#include <stdlib.h>
#include <wtf/Assertions.h>
#include <wtf/HashSet.h>
+#include <wtf/Threading.h>
+#include <wtf/ThreadingPrimitives.h>
#include <wtf/Vector.h>
static const size_t javaScriptThreadsCount = 4;
