tberghammer added inline comments.
================
Comment at: source/Core/ConstString.cpp:147-152
@@ -165,7 +146,8 @@
protected:
- //------------------------------------------------------------------
- // Typedefs
- //------------------------------------------------------------------
- typedef StringPool::iterator iterator;
- typedef StringPool::const_iterator const_iterator;
+ uint8_t
+ hash(const llvm::StringRef &s)
+ {
+ uint32_t h = llvm::HashString(s);
+ return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff;
+ }
----------------
clayborg wrote:
> Is there a way we can use the hash that llvm uses for its string pool here?
> We are calculating two hashes: one for this to see which pool it will go
> into, and then another when the string is hashed into the string pool object.
> It would be nice if we can calculate the hash once and maybe do/add a string
> pool insertion that the pool with use to verify the string is in the pool or
> insert it using the supplied hash?
I don't see any reasonable way to avoid using 2 hash function without
re-implementing llvm::StringMap with multi-threaded support in mind with per
bucket mutextes. One of the issue is that llvm::StringMap don't have any
interface where we can specify a hash value for an insert to avoid the
calculation of the hash. The other problem is that we want to use a different
hash function for selecting the pool and then selecting the bucket to achieve a
uniform distribution between the buckets inside the StringMap. I am already a
little bit concerned because we use 2 very similar hash function (StringMap use
the LSB of llvm::HashString) what can cause some performance degradation.
I think a nice solution would be to use a hash map with built in multi-threaded
support, or even better with a lock-free implementation (lock/unlock takes a
lot of time) but I don't think implementing it would worth the effort.
================
Comment at: source/Core/ConstString.cpp:175
@@ -174,3 @@
- //------------------------------------------------------------------
- mutable Mutex m_mutex;
- StringPool m_string_map;
----------------
zturner wrote:
> Did you consider changing this to an `llvm::RWMutex`?
I haven't tried it, but I don't see any easy way to use it because we use a
single StringMap::insert call to read and possibly write to the map. If we want
to get the advantage out from RWMutex then we should split it into a
StringMap::find and then a StringMap::insert call what is doing 2 lookup.
http://reviews.llvm.org/D13652
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
