Turning on DUMP_PROPERTYMAP_STATS causes a build failure
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Jun 2014 20:21:34 +0000 (20:21 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Jun 2014 20:21:34 +0000 (20:21 +0000)
https://bugs.webkit.org/show_bug.cgi?id=133673

Reviewed by Andreas Kling.

Source/JavaScriptCore:
Rewrote the property map statistics code because the old code wasn't building,
and it was also mixing numbers for lookups and insertions/removals.

New logging code records the number of calls to PropertyTable::find (finds) and
PropertyTable::get/PropertyTable::findWithString separately so that we can quantify
the number of probing during updates and lookups.

* jsc.cpp:
* runtime/PropertyMapHashTable.h:
(JSC::PropertyTable::find):
(JSC::PropertyTable::get):
(JSC::PropertyTable::findWithString):
(JSC::PropertyTable::add):
(JSC::PropertyTable::remove):
(JSC::PropertyTable::reinsert):
(JSC::PropertyTable::rehash):
* runtime/Structure.cpp:
(JSC::PropertyMapStatisticsExitLogger::PropertyMapStatisticsExitLogger):
(JSC::PropertyMapStatisticsExitLogger::~PropertyMapStatisticsExitLogger):

Source/WTF:
Added DEFINE_GLOBAL_FOR_LOGGING to allow running a destructor in logging code
that needs to be enabled in release builds (e.g. for JavaScriptCore).

* wtf/StdLibExtras.h:

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/jsc.cpp
Source/JavaScriptCore/runtime/PropertyMapHashTable.h
Source/JavaScriptCore/runtime/Structure.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/StdLibExtras.h

index b4d245bdc645db4319b90d132c17f635f16221cd..bd886271456384f1e2f748dd97cb5d36e347784c 100644 (file)
@@ -1,3 +1,30 @@
+2014-06-11  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Turning on DUMP_PROPERTYMAP_STATS causes a build failure
+        https://bugs.webkit.org/show_bug.cgi?id=133673
+
+        Reviewed by Andreas Kling.
+
+        Rewrote the property map statistics code because the old code wasn't building,
+        and it was also mixing numbers for lookups and insertions/removals.
+
+        New logging code records the number of calls to PropertyTable::find (finds) and
+        PropertyTable::get/PropertyTable::findWithString separately so that we can quantify
+        the number of probing during updates and lookups.
+
+        * jsc.cpp:
+        * runtime/PropertyMapHashTable.h:
+        (JSC::PropertyTable::find):
+        (JSC::PropertyTable::get):
+        (JSC::PropertyTable::findWithString):
+        (JSC::PropertyTable::add):
+        (JSC::PropertyTable::remove):
+        (JSC::PropertyTable::reinsert):
+        (JSC::PropertyTable::rehash):
+        * runtime/Structure.cpp:
+        (JSC::PropertyMapStatisticsExitLogger::PropertyMapStatisticsExitLogger):
+        (JSC::PropertyMapStatisticsExitLogger::~PropertyMapStatisticsExitLogger):
+
 2014-06-11  Andreas Kling  <akling@apple.com>
 
         Always inline JSValue::get() and Structure::get().
index aed7af1877f112b93c4569954502a7c665430550..8f4b6d4846ecabb24614998caac03e3d183554bd 100644 (file)
@@ -40,6 +40,7 @@
 #include "ProfilerDatabase.h"
 #include "SamplingTool.h"
 #include "StackVisitor.h"
+#include "StructureInlines.h"
 #include "StructureRareDataInlines.h"
 #include "TestRunnerUtils.h"
 #include <math.h>
index 2abc5ef65cfaf3e2601e511e1fdba8a5a873a55d..d36915cc30545ecc9aca4bf682b40fcf64da4dfc 100644 (file)
@@ -21,6 +21,7 @@
 #ifndef PropertyMapHashTable_h
 #define PropertyMapHashTable_h
 
+#include "JSExportMacros.h"
 #include "PropertyOffset.h"
 #include "Structure.h"
 #include "WriteBarrier.h"
 #include <wtf/text/StringImpl.h>
 
 
-#ifndef NDEBUG
-#define DUMP_PROPERTYMAP_STATS 0
-#else
 #define DUMP_PROPERTYMAP_STATS 0
-#endif
+#define DUMP_PROPERTYMAP_COLLISIONS 0
 
-#if DUMP_PROPERTYMAP_STATS
+#define PROPERTY_MAP_DELETED_ENTRY_KEY ((StringImpl*)1)
 
-extern int numProbes;
-extern int numCollisions;
-extern int numRehashes;
-extern int numRemoves;
+namespace JSC {
 
-#endif
+#if DUMP_PROPERTYMAP_STATS
 
-#define PROPERTY_MAP_DELETED_ENTRY_KEY ((StringImpl*)1)
+struct PropertyMapHashTableStats {
+    std::atomic<unsigned> numFinds;
+    std::atomic<unsigned> numCollisions;
+    std::atomic<unsigned> numLookups;
+    std::atomic<unsigned> numLookupProbing;
+    std::atomic<unsigned> numAdds;
+    std::atomic<unsigned> numRemoves;
+    std::atomic<unsigned> numRehashes;
+    std::atomic<unsigned> numReinserts;
+};
 
-namespace JSC {
+JS_EXPORTDATA extern PropertyMapHashTableStats* propertyMapHashTableStats;
+
+#endif
 
 inline bool isPowerOf2(unsigned v)
 {
@@ -289,7 +295,7 @@ inline PropertyTable::find_iterator PropertyTable::find(const KeyType& key)
     unsigned step = 0;
 
 #if DUMP_PROPERTYMAP_STATS
-    ++numProbes;
+    ++propertyMapHashTableStats->numFinds;
 #endif
 
     while (true) {
@@ -299,17 +305,19 @@ inline PropertyTable::find_iterator PropertyTable::find(const KeyType& key)
         if (key == table()[entryIndex - 1].key)
             return std::make_pair(&table()[entryIndex - 1], hash & m_indexMask);
 
-#if DUMP_PROPERTYMAP_STATS
-        ++numCollisions;
-#endif
-
         if (!step)
             step = WTF::doubleHash(key->existingHash()) | 1;
-        hash += step;
 
 #if DUMP_PROPERTYMAP_STATS
-        ++numRehashes;
+        ++propertyMapHashTableStats->numCollisions;
+#endif
+
+#if DUMP_PROPERTYMAP_COLLISIONS
+        dataLog("PropertyTable collision for ", key, " (", hash, ") with step ", step, "\n");
+        dataLog("Collided with ", table()[entryIndex - 1].key, "(", table()[entryIndex - 1].key->existingHash(), ")\n");
 #endif
+
+        hash += step;
     }
 }
 
@@ -325,7 +333,7 @@ inline PropertyTable::ValueType* PropertyTable::get(const KeyType& key)
     unsigned step = 0;
 
 #if DUMP_PROPERTYMAP_STATS
-    ++numProbes;
+    ++propertyMapHashTableStats->numLookups;
 #endif
 
     while (true) {
@@ -336,16 +344,12 @@ inline PropertyTable::ValueType* PropertyTable::get(const KeyType& key)
             return &table()[entryIndex - 1];
 
 #if DUMP_PROPERTYMAP_STATS
-        ++numCollisions;
+        ++propertyMapHashTableStats->numLookupProbing;
 #endif
 
         if (!step)
             step = WTF::doubleHash(key->existingHash()) | 1;
         hash += step;
-
-#if DUMP_PROPERTYMAP_STATS
-        ++numRehashes;
-#endif
     }
 }
 
@@ -357,7 +361,7 @@ inline PropertyTable::find_iterator PropertyTable::findWithString(const KeyType&
     unsigned step = 0;
 
 #if DUMP_PROPERTYMAP_STATS
-    ++numProbes;
+    ++propertyMapHashTableStats->numLookups;
 #endif
 
     while (true) {
@@ -369,16 +373,12 @@ inline PropertyTable::find_iterator PropertyTable::findWithString(const KeyType&
             return std::make_pair(&table()[entryIndex - 1], hash & m_indexMask);
 
 #if DUMP_PROPERTYMAP_STATS
-        ++numCollisions;
+        ++propertyMapHashTableStats->numLookupProbing;
 #endif
 
         if (!step)
             step = WTF::doubleHash(key->existingHash()) | 1;
         hash += step;
-
-#if DUMP_PROPERTYMAP_STATS
-        ++numRehashes;
-#endif
     }
 }
 
@@ -391,6 +391,10 @@ inline std::pair<PropertyTable::find_iterator, bool> PropertyTable::add(const Va
         return std::make_pair(iter, false);
     }
 
+#if DUMP_PROPERTYMAP_STATS
+    ++propertyMapHashTableStats->numAdds;
+#endif
+
     // Ref the key
     entry.key->ref();
 
@@ -424,7 +428,7 @@ inline void PropertyTable::remove(const find_iterator& iter)
         return;
 
 #if DUMP_PROPERTYMAP_STATS
-    ++numRemoves;
+    ++propertyMapHashTableStats->numRemoves;
 #endif
 
     // Replace this one element with the deleted sentinel. Also clear out
@@ -517,6 +521,10 @@ inline size_t PropertyTable::sizeInMemory()
 
 inline void PropertyTable::reinsert(const ValueType& entry)
 {
+#if DUMP_PROPERTYMAP_STATS
+    ++propertyMapHashTableStats->numReinserts;
+#endif
+
     // Used to insert a value known not to be in the table, and where
     // we know capacity to be available.
     ASSERT(canInsert());
@@ -532,6 +540,10 @@ inline void PropertyTable::reinsert(const ValueType& entry)
 
 inline void PropertyTable::rehash(unsigned newCapacity)
 {
+#if DUMP_PROPERTYMAP_STATS
+    ++propertyMapHashTableStats->numRehashes;
+#endif
+
     unsigned* oldEntryIndices = m_index;
     iterator iter = this->begin();
     iterator end = this->end();
index 3c38f5418cdda9fccfe837b4e9407f9581c65c8a..82b3852bb6e1d04a51c7e8ed2474046bd66cd3d8 100644 (file)
@@ -37,6 +37,7 @@
 #include "StructureChain.h"
 #include "StructureRareDataInlines.h"
 #include <wtf/CommaPrinter.h>
+#include <wtf/ProcessID.h>
 #include <wtf/RefCountedLeakCounter.h>
 #include <wtf/RefPtr.h>
 #include <wtf/Threading.h>
 using namespace std;
 using namespace WTF;
 
-#if DUMP_PROPERTYMAP_STATS
-
-int numProbes;
-int numCollisions;
-int numRehashes;
-int numRemoves;
-
-#endif
-
 namespace JSC {
 
 #if DUMP_STRUCTURE_ID_STATISTICS
@@ -816,19 +808,33 @@ void Structure::cloneRareDataFrom(VM& vm, const Structure* other)
 
 #if DUMP_PROPERTYMAP_STATS
 
+PropertyMapHashTableStats* propertyMapHashTableStats = 0;
+
 struct PropertyMapStatisticsExitLogger {
+    PropertyMapStatisticsExitLogger();
     ~PropertyMapStatisticsExitLogger();
 };
 
-static PropertyMapStatisticsExitLogger logger;
+DEFINE_GLOBAL_FOR_LOGGING(PropertyMapStatisticsExitLogger, logger, );
+
+PropertyMapStatisticsExitLogger::PropertyMapStatisticsExitLogger()
+{
+    propertyMapHashTableStats = adoptPtr(new PropertyMapHashTableStats()).leakPtr();
+}
 
 PropertyMapStatisticsExitLogger::~PropertyMapStatisticsExitLogger()
 {
-    dataLogF("\nJSC::PropertyMap statistics\n\n");
-    dataLogF("%d probes\n", numProbes);
-    dataLogF("%d collisions (%.1f%%)\n", numCollisions, 100.0 * numCollisions / numProbes);
-    dataLogF("%d rehashes\n", numRehashes);
-    dataLogF("%d removes\n", numRemoves);
+    unsigned finds = propertyMapHashTableStats->numFinds;
+    unsigned collisions = propertyMapHashTableStats->numCollisions;
+    dataLogF("\nJSC::PropertyMap statistics for process %d\n\n", getCurrentProcessID());
+    dataLogF("%d finds\n", finds);
+    dataLogF("%d collisions (%.1f%%)\n", collisions, 100.0 * collisions / finds);
+    dataLogF("%d lookups\n", propertyMapHashTableStats->numLookups.load());
+    dataLogF("%d lookup probings\n", propertyMapHashTableStats->numLookupProbing.load());
+    dataLogF("%d adds\n", propertyMapHashTableStats->numAdds.load());
+    dataLogF("%d removes\n", propertyMapHashTableStats->numRemoves.load());
+    dataLogF("%d rehashes\n", propertyMapHashTableStats->numRehashes.load());
+    dataLogF("%d reinserts\n", propertyMapHashTableStats->numReinserts.load());
 }
 
 #endif
index 6d3ee4042c96bf9aedb87f68815428912f3b058d..087452d50b754599682dfbf859058ca7f8104caa 100644 (file)
@@ -1,3 +1,15 @@
+2014-06-11  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Turning on DUMP_PROPERTYMAP_STATS causes a build failure
+        https://bugs.webkit.org/show_bug.cgi?id=133673
+
+        Reviewed by Andreas Kling.
+
+        Added DEFINE_GLOBAL_FOR_LOGGING to allow running a destructor in logging code
+        that needs to be enabled in release builds (e.g. for JavaScriptCore).
+
+        * wtf/StdLibExtras.h:
+
 2014-06-09  Benjamin Poulain  <bpoulain@apple.com>
 
         Improve CSSPrimitiveValue::customCSSText for ARMv7
index 815a4bb27ae20c25b9bf6cb074f552ce74563bdf..891fa617c6072eef9a2b68eb001a764930f30799 100644 (file)
 // Use this macro to declare and define a debug-only global variable that may have a
 // non-trivial constructor and destructor. When building with clang, this will suppress
 // warnings about global constructors and exit-time destructors.
-#ifndef NDEBUG
-#if COMPILER(CLANG)
-#define DEFINE_DEBUG_ONLY_GLOBAL(type, name, arguments) \
+#define DEFINE_GLOBAL_FOR_LOGGING(type, name, arguments) \
     _Pragma("clang diagnostic push") \
     _Pragma("clang diagnostic ignored \"-Wglobal-constructors\"") \
     _Pragma("clang diagnostic ignored \"-Wexit-time-destructors\"") \
     static type name arguments; \
     _Pragma("clang diagnostic pop")
+
+#ifndef NDEBUG
+#if COMPILER(CLANG)
+#define DEFINE_DEBUG_ONLY_GLOBAL(type, name, arguments) DEFINE_GLOBAL_FOR_LOGGING(type, name, arguments)
 #else
 #define DEFINE_DEBUG_ONLY_GLOBAL(type, name, arguments) \
     static type name arguments;