<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Dec 5, 2017 at 1:45 PM Lang Hames via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: lhames<br>
Date: Tue Dec 5 13:44:56 2017<br>
New Revision: 319839<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=319839&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=319839&view=rev</a><br>
Log:<br>
[Orc] Add a SymbolStringPool data structure for efficient storage and fast<br>
comparison of symbol names.<br>
<br>
SymbolStringPool is a thread-safe string pool that will be used in upcoming Orc<br>
APIs to facilitate efficient storage and fast comparison of symbol name strings.<br>
<br>
Added:<br>
llvm/trunk/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h<br>
llvm/trunk/unittests/ExecutionEngine/Orc/SymbolStringPoolTest.cpp<br>
Modified:<br>
llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt<br>
<br>
Added: llvm/trunk/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h?rev=319839&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h?rev=319839&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h (added)<br>
+++ llvm/trunk/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h Tue Dec 5 13:44:56 2017<br>
@@ -0,0 +1,133 @@<br>
+//===- SymbolStringPool.h - Multi-threaded pool for JIT symbols -*- C++ -*-===//<br>
+//<br>
+// The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+//<br>
+// Contains a multi-threaded string pool suitable for use with ORC.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#ifndef LLVM_EXECUTIONENGINE_ORC_SYMBOLSTRINGPOOL_H<br>
+#define LLVM_EXECUTIONENGINE_ORC_SYMBOLSTRINGPOOL_H<br>
+<br>
+#include "llvm/ADT/StringMap.h"<br>
+#include <atomic><br>
+#include <mutex><br>
+<br>
+namespace llvm {<br>
+namespace orc {<br>
+<br>
+class SymbolStringPtr;<br>
+<br>
+/// @brief String pool for symbol names used by the JIT.<br>
+class SymbolStringPool {<br>
+ friend class SymbolStringPtr;<br>
+public:<br>
+ /// @brief Create a symbol string pointer from the given string.<br>
+ SymbolStringPtr intern(StringRef S);<br>
+<br>
+ /// @brief Remove from the pool any entries that are no longer referenced.<br>
+ void clearDeadEntries();<br>
+<br>
+ /// @brief Returns true if the pool is empty.<br>
+ bool empty() const;<br>
+private:<br>
+ using RefCountType = std::atomic_uint64_t;<br>
+ using PoolMap = StringMap<RefCountType>;<br>
+ using PoolMapEntry = StringMapEntry<RefCountType>;<br>
+ mutable std::mutex PoolMutex;<br>
+ PoolMap Pool;<br>
+};<br>
+<br>
+/// @brief Pointer to a pooled string representing a symbol name.<br>
+class SymbolStringPtr {<br>
+ friend class SymbolStringPool;<br>
+public:<br>
+ SymbolStringPtr() = default;<br>
+ SymbolStringPtr(const SymbolStringPtr &Other)<br>
+ : S(Other.S) {<br>
+ if (S)<br>
+ ++S->getValue();<br>
+ }<br>
+<br>
+ SymbolStringPtr& operator=(const SymbolStringPtr &Other) {<br>
+ if (S)<br>
+ --S->getValue();<br>
+ S = Other.S;<br>
+ if (S)<br>
+ ++S->getValue();<br>
+ return *this;<br>
+ }<br>
+<br>
+ SymbolStringPtr(SymbolStringPtr &&Other) : S(nullptr) {<br>
+ std::swap(S, Other.S);<br>
+ }<br>
+<br>
+ SymbolStringPtr& operator=(SymbolStringPtr &&Other) {<br>
+ if (S)<br>
+ --S->getValue();<br>
+ S = nullptr;<br>
+ std::swap(S, Other.S);<br>
+ return *this;<br>
+ }<br>
+<br>
+ ~SymbolStringPtr() {<br>
+ if (S)<br>
+ --S->getValue();<br>
+ }<br>
+<br>
+ bool operator==(const SymbolStringPtr &Other) const {<br></blockquote><div><br>In general, any op overload that can be a non-member should be (though it can still be a friend and defined inline, so it doesn't change the code much) so that any conversions that apply to the RHS apply equally to the LHS.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ return S == Other.S;<br>
+ }<br>
+<br>
+ bool operator!=(const SymbolStringPtr &Other) const {<br>
+ return !(*this == Other);<br>
+ }<br>
+<br>
+private:<br>
+<br>
+ SymbolStringPtr(SymbolStringPool::PoolMapEntry *S)<br>
+ : S(S) {<br>
+ if (S)<br>
+ ++S->getValue();<br>
+ }<br>
+<br>
+ SymbolStringPool::PoolMapEntry *S = nullptr;<br>
+};<br>
+<br>
+SymbolStringPtr SymbolStringPool::intern(StringRef S) {<br>
+ std::lock_guard<std::mutex> Lock(PoolMutex);<br>
+ auto I = Pool.find(S);<br>
+ if (I != Pool.end())<br>
+ return SymbolStringPtr(&*I);<br>
+<br>
+ bool Added;<br>
+ std::tie(I, Added) = Pool.try_emplace(S, 0);<br></blockquote><div><br>This implementation involves two map lookups (one to find, another to emplace if the find fails) - if you call try_emplace first, and inspect its "Added" result to decide what to do (you might not have to do anything, actually), then you can use one lookup.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ assert(Added && "Insert should always succeed here");<br>
+ return SymbolStringPtr(&*I);<br>
+}<br>
+<br>
+void SymbolStringPool::clearDeadEntries() {<br>
+ std::lock_guard<std::mutex> Lock(PoolMutex);<br>
+ for (auto I = Pool.begin(), E = Pool.end(); I != E;) {<br>
+ auto Tmp = std::next(I);<br>
+ if (I->second == 0)<br>
+ Pool.erase(I);<br>
+ I = Tmp;<br></blockquote><div><br>This ^ might be better written as:<br><br>auto Tmp = I++;</div><div>if (Tmp->second == 0)<br> Pool.erase(Tmp);<br><br>So there's no need to "remember" the assignment to I at the end? (action at a distance, etc) - though I can appreciate having the "increment" action at the end of the loop as it's written here.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ }<br>
+}<br>
+<br>
+bool SymbolStringPool::empty() const {<br>
+ std::lock_guard<std::mutex> Lock(PoolMutex);<br>
+ return Pool.empty();<br>
+}<br>
+<br>
+} // end namespace orc<br>
+<br>
+} // end namespace llvm<br>
+<br>
+#endif // LLVM_EXECUTIONENGINE_ORC_SYMBOLSTRINGPOOL_H<br>
<br>
Modified: llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt?rev=319839&r1=319838&r2=319839&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt?rev=319839&r1=319838&r2=319839&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt (original)<br>
+++ llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt Tue Dec 5 13:44:56 2017<br>
@@ -21,6 +21,7 @@ add_llvm_unittest(OrcJITTests<br>
RemoteObjectLayerTest.cpp<br>
RPCUtilsTest.cpp<br>
RTDyldObjectLinkingLayerTest.cpp<br>
+ SymbolStringPoolTest.cpp<br>
)<br>
<br>
target_link_libraries(OrcJITTests ${LLVM_PTHREAD_LIB})<br>
<br>
Added: llvm/trunk/unittests/ExecutionEngine/Orc/SymbolStringPoolTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/SymbolStringPoolTest.cpp?rev=319839&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/SymbolStringPoolTest.cpp?rev=319839&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/ExecutionEngine/Orc/SymbolStringPoolTest.cpp (added)<br>
+++ llvm/trunk/unittests/ExecutionEngine/Orc/SymbolStringPoolTest.cpp Tue Dec 5 13:44:56 2017<br>
@@ -0,0 +1,43 @@<br>
+//===----- SymbolStringPoolTest.cpp - Unit tests for SymbolStringPool -----===//<br>
+//<br>
+// The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#include "llvm/ExecutionEngine/Orc/SymbolStringPool.h"<br>
+#include "gtest/gtest.h"<br>
+<br>
+using namespace llvm;<br>
+using namespace llvm::orc;<br>
+<br>
+namespace {<br>
+<br>
+TEST(SymbolStringPool, UniquingAndEquality) {<br>
+ SymbolStringPool SP;<br>
+ auto P1 = SP.intern("hello");<br>
+<br>
+ std::string S("hel");<br>
+ S += "lo";<br>
+ auto P2 = SP.intern(S);<br>
+<br>
+ auto P3 = SP.intern("goodbye");<br>
+<br>
+ EXPECT_EQ(P1, P2) << "Failed to unique entries";<br>
+ EXPECT_NE(P1, P3) << "Inequal pooled symbol strings comparing equal";<br>
+}<br>
+<br>
+TEST(SymbolStringPool, ClearDeadEntries) {<br>
+ SymbolStringPool SP;<br>
+ {<br>
+ auto P1 = SP.intern("s1");<br>
+ SP.clearDeadEntries();<br>
+ EXPECT_FALSE(SP.empty()) << "\"s1\" entry in pool should still be retained";<br>
+ }<br>
+ SP.clearDeadEntries();<br>
+ EXPECT_TRUE(SP.empty()) << "pool should be empty";<br>
+}<br>
+<br>
+}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>