[llvm] r319839 - [Orc] Add a SymbolStringPool data structure for efficient storage and fast
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 9 20:03:37 PST 2018
Thanks for the feedback Dave!
All good points. Suggestions applied in r322159.
Cheers,
Lang.
On Thu, Dec 14, 2017 at 1:21 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Tue, Dec 5, 2017 at 1:45 PM Lang Hames via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: lhames
>> Date: Tue Dec 5 13:44:56 2017
>> New Revision: 319839
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=319839&view=rev
>> Log:
>> [Orc] Add a SymbolStringPool data structure for efficient storage and fast
>> comparison of symbol names.
>>
>> SymbolStringPool is a thread-safe string pool that will be used in
>> upcoming Orc
>> APIs to facilitate efficient storage and fast comparison of symbol name
>> strings.
>>
>> Added:
>> llvm/trunk/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h
>> llvm/trunk/unittests/ExecutionEngine/Orc/SymbolStringPoolTest.cpp
>> Modified:
>> llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt
>>
>> Added: llvm/trunk/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/
>> llvm/ExecutionEngine/Orc/SymbolStringPool.h?rev=319839&view=auto
>> ============================================================
>> ==================
>> --- llvm/trunk/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h
>> (added)
>> +++ llvm/trunk/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h Tue
>> Dec 5 13:44:56 2017
>> @@ -0,0 +1,133 @@
>> +//===- SymbolStringPool.h - Multi-threaded pool for JIT symbols -*- C++
>> -*-===//
>> +//
>> +// The LLVM Compiler Infrastructure
>> +//
>> +// This file is distributed under the University of Illinois Open Source
>> +// License. See LICENSE.TXT for details.
>> +//
>> +//===------------------------------------------------------
>> ----------------===//
>> +//
>> +// Contains a multi-threaded string pool suitable for use with ORC.
>> +//
>> +//===------------------------------------------------------
>> ----------------===//
>> +
>> +#ifndef LLVM_EXECUTIONENGINE_ORC_SYMBOLSTRINGPOOL_H
>> +#define LLVM_EXECUTIONENGINE_ORC_SYMBOLSTRINGPOOL_H
>> +
>> +#include "llvm/ADT/StringMap.h"
>> +#include <atomic>
>> +#include <mutex>
>> +
>> +namespace llvm {
>> +namespace orc {
>> +
>> +class SymbolStringPtr;
>> +
>> +/// @brief String pool for symbol names used by the JIT.
>> +class SymbolStringPool {
>> + friend class SymbolStringPtr;
>> +public:
>> + /// @brief Create a symbol string pointer from the given string.
>> + SymbolStringPtr intern(StringRef S);
>> +
>> + /// @brief Remove from the pool any entries that are no longer
>> referenced.
>> + void clearDeadEntries();
>> +
>> + /// @brief Returns true if the pool is empty.
>> + bool empty() const;
>> +private:
>> + using RefCountType = std::atomic_uint64_t;
>> + using PoolMap = StringMap<RefCountType>;
>> + using PoolMapEntry = StringMapEntry<RefCountType>;
>> + mutable std::mutex PoolMutex;
>> + PoolMap Pool;
>> +};
>> +
>> +/// @brief Pointer to a pooled string representing a symbol name.
>> +class SymbolStringPtr {
>> + friend class SymbolStringPool;
>> +public:
>> + SymbolStringPtr() = default;
>> + SymbolStringPtr(const SymbolStringPtr &Other)
>> + : S(Other.S) {
>> + if (S)
>> + ++S->getValue();
>> + }
>> +
>> + SymbolStringPtr& operator=(const SymbolStringPtr &Other) {
>> + if (S)
>> + --S->getValue();
>> + S = Other.S;
>> + if (S)
>> + ++S->getValue();
>> + return *this;
>> + }
>> +
>> + SymbolStringPtr(SymbolStringPtr &&Other) : S(nullptr) {
>> + std::swap(S, Other.S);
>> + }
>> +
>> + SymbolStringPtr& operator=(SymbolStringPtr &&Other) {
>> + if (S)
>> + --S->getValue();
>> + S = nullptr;
>> + std::swap(S, Other.S);
>> + return *this;
>> + }
>> +
>> + ~SymbolStringPtr() {
>> + if (S)
>> + --S->getValue();
>> + }
>> +
>> + bool operator==(const SymbolStringPtr &Other) const {
>>
>
> 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.
>
>
>> + return S == Other.S;
>> + }
>> +
>> + bool operator!=(const SymbolStringPtr &Other) const {
>> + return !(*this == Other);
>> + }
>> +
>> +private:
>> +
>> + SymbolStringPtr(SymbolStringPool::PoolMapEntry *S)
>> + : S(S) {
>> + if (S)
>> + ++S->getValue();
>> + }
>> +
>> + SymbolStringPool::PoolMapEntry *S = nullptr;
>> +};
>> +
>> +SymbolStringPtr SymbolStringPool::intern(StringRef S) {
>> + std::lock_guard<std::mutex> Lock(PoolMutex);
>> + auto I = Pool.find(S);
>> + if (I != Pool.end())
>> + return SymbolStringPtr(&*I);
>> +
>> + bool Added;
>> + std::tie(I, Added) = Pool.try_emplace(S, 0);
>>
>
> 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.
>
>
>> + assert(Added && "Insert should always succeed here");
>> + return SymbolStringPtr(&*I);
>> +}
>> +
>> +void SymbolStringPool::clearDeadEntries() {
>> + std::lock_guard<std::mutex> Lock(PoolMutex);
>> + for (auto I = Pool.begin(), E = Pool.end(); I != E;) {
>> + auto Tmp = std::next(I);
>> + if (I->second == 0)
>> + Pool.erase(I);
>> + I = Tmp;
>>
>
> This ^ might be better written as:
>
> auto Tmp = I++;
> if (Tmp->second == 0)
> Pool.erase(Tmp);
>
> 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.
>
>
>> + }
>> +}
>> +
>> +bool SymbolStringPool::empty() const {
>> + std::lock_guard<std::mutex> Lock(PoolMutex);
>> + return Pool.empty();
>> +}
>> +
>> +} // end namespace orc
>> +
>> +} // end namespace llvm
>> +
>> +#endif // LLVM_EXECUTIONENGINE_ORC_SYMBOLSTRINGPOOL_H
>>
>> Modified: llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/
>> ExecutionEngine/Orc/CMakeLists.txt?rev=319839&r1=
>> 319838&r2=319839&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt (original)
>> +++ llvm/trunk/unittests/ExecutionEngine/Orc/CMakeLists.txt Tue Dec 5
>> 13:44:56 2017
>> @@ -21,6 +21,7 @@ add_llvm_unittest(OrcJITTests
>> RemoteObjectLayerTest.cpp
>> RPCUtilsTest.cpp
>> RTDyldObjectLinkingLayerTest.cpp
>> + SymbolStringPoolTest.cpp
>> )
>>
>> target_link_libraries(OrcJITTests ${LLVM_PTHREAD_LIB})
>>
>> Added: llvm/trunk/unittests/ExecutionEngine/Orc/SymbolStringPoolTest.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/
>> ExecutionEngine/Orc/SymbolStringPoolTest.cpp?rev=319839&view=auto
>> ============================================================
>> ==================
>> --- llvm/trunk/unittests/ExecutionEngine/Orc/SymbolStringPoolTest.cpp
>> (added)
>> +++ llvm/trunk/unittests/ExecutionEngine/Orc/SymbolStringPoolTest.cpp
>> Tue Dec 5 13:44:56 2017
>> @@ -0,0 +1,43 @@
>> +//===----- SymbolStringPoolTest.cpp - Unit tests for SymbolStringPool
>> -----===//
>> +//
>> +// The LLVM Compiler Infrastructure
>> +//
>> +// This file is distributed under the University of Illinois Open Source
>> +// License. See LICENSE.TXT for details.
>> +//
>> +//===------------------------------------------------------
>> ----------------===//
>> +
>> +#include "llvm/ExecutionEngine/Orc/SymbolStringPool.h"
>> +#include "gtest/gtest.h"
>> +
>> +using namespace llvm;
>> +using namespace llvm::orc;
>> +
>> +namespace {
>> +
>> +TEST(SymbolStringPool, UniquingAndEquality) {
>> + SymbolStringPool SP;
>> + auto P1 = SP.intern("hello");
>> +
>> + std::string S("hel");
>> + S += "lo";
>> + auto P2 = SP.intern(S);
>> +
>> + auto P3 = SP.intern("goodbye");
>> +
>> + EXPECT_EQ(P1, P2) << "Failed to unique entries";
>> + EXPECT_NE(P1, P3) << "Inequal pooled symbol strings comparing equal";
>> +}
>> +
>> +TEST(SymbolStringPool, ClearDeadEntries) {
>> + SymbolStringPool SP;
>> + {
>> + auto P1 = SP.intern("s1");
>> + SP.clearDeadEntries();
>> + EXPECT_FALSE(SP.empty()) << "\"s1\" entry in pool should still be
>> retained";
>> + }
>> + SP.clearDeadEntries();
>> + EXPECT_TRUE(SP.empty()) << "pool should be empty";
>> +}
>> +
>> +}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180109/a536441f/attachment.html>
More information about the llvm-commits
mailing list