[Lldb-commits] [PATCH] D13652: Change ConstString to support massive multi-threaded access

[Zachary Turner via lldb-commits]

zturner added inline comments.

================
Comment at: source/Core/ConstString.cpp:175
@@ -174,3 @@
-    //------------------------------------------------------------------
-    mutable Mutex m_mutex;
-    StringPool m_string_map;
----------------
tberghammer wrote:
> 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.
That's not a big deal though is it?  Average case for lookup and insert are both constant, so doing 2 operations is still constant.  Over time as more and more strings get added, the probability of finding any given string increases, meaning you will converge towards having more reads than writes and the additional concurrency gained from doing a read followed by a write far outweighs the overhead of the extra constant-time operation.

You could do it with double checked locking.  For example:

    lock.acquire_read();
    if (value = map.find(x))
         return value;
    lock.acquire_write();
    if (value = map.find(x))
        return value;
    map.insert(x);
    return x;



http://reviews.llvm.org/D13652