scott.smith marked 3 inline comments as done.
scott.smith added inline comments.
================
Comment at: source/Utility/ConstString.cpp:49
+ // pointer, we don't need the lock.
const StringPoolEntryType &entry = GetStringMapEntryFromKeyData(ccstr);
return entry.getKey().size();
----------------
labath wrote:
> labath wrote:
> > scott.smith wrote:
> > > zturner wrote:
> > > > Why do we even have this function which digs into the `StringMap`
> > > > internals rather than just calling existing `StringMap` member
> > > > functions? Can Can we just delete `GetStringMapEntryFromKeyData`
> > > > entirely and use `StringMap::find`?
> > > Probably performance. If we have to call Find, then we have to call
> > > hash, fault in the appropriate bucket, and then finally return the entry
> > > that we already have in hand. Plus we'd need the lock.
> > >
> > > Can we just delete GetStringMapEntryFromKeyData entirely and use
> > > StringMap::find?
> > Unfortunately, I don't think that's possible. `StringMap::find` expects a
> > StringRef. In order to construct that, we need to know the length of the
> > string, and we're back where we started :(
> >
> > In reality, this is doing a very different operation than find (which takes
> > a random string and checks whether it's in the map) -- this takes a string
> > which we know to be in the map and get its size.
> >
> > It will take some rearchitecting of the ConstString class to get rid of
> > this hack. Probably it could be fixed by ConstString storing a
> > StringMap::iterator instead of the raw pointer. In any case, that seems out
> > of scope of this change.
> Cool, I didn't notice that one when looking for it. I guess at this point we
> can just delete our copy of `GetStringMapEntryFromKeyData` completely and
> call the StringPool's version instead.
It will push a lot of lines past the 80 char limit. Do you want me to make
that change? If not, can you submit this one since I do not have commit
access? Thanks!
Repository:
rL LLVM
https://reviews.llvm.org/D32306
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
