[PATCH] D25645: [ADT] Add CachedHashString.

[Justin Lebar via llvm-commits]

jlebar added inline comments.


================
Comment at: llvm/include/llvm/ADT/CachedHashString.h:94
+
+  // TODO: Use small-string optimization to avoid allocating.
+
----------------
timshen wrote:
> timshen wrote:
> > It is unfortunate to implement some of the std::string functionalities from scratch. Is there a way to implement it in terms of std::string?
> More specifically, I'm asking for a member function `string& val();` that returns the underlying std::string object (it needs to exist first), so that CachedHashString compose better with other APIs that don't care about hashing.
We could do this, but at the cost of more than doubling its storage.

Right now this class takes two words of space on 64-bit platforms.  sizeof(std::string) is 32 bytes (with whatever stdlib gcc.godbolt.org uses), so already 4 words.  Plus we'd need extra space for storing the two is-empty and is-tombstone bits.  (We could also use this space for storing the hash code.)  So our modified class would be 40 bytes, instead of 16.

That's pretty significant for a class that's meant to be stored in open-addressing hashtables.

I agree it's unfortunate -- if you have a better idea I'm all ears.


https://reviews.llvm.org/D25645