[PATCH] D133715: [ADT] Add HashMappedTrie

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 13 15:17:45 PST 2022


dexonsmith added a comment.

In D133715#3993311 <https://reviews.llvm.org/D133715#3993311>, @dblaikie wrote:

>> Note that there's an example data structure called ThreadSafeHashMappedTrieSet buried in the unit tests, which has an interface that hides the hash. Could be useful for some clients.
>>
>> - Maybe it would make sense to lift up? Name could be PerfectHashSet
>> - You might want to build a map that also handles the hashing; name could potentially be PerfectHashMap
>>
>> Maybe this could be PerfectHashSetImpl?
>>
>> - Has the "guts" for implementing a client-friendly PerfectHashSet or PerfectHashMap?
>> - ... but can be used directly if you want to manage the hashing yourself?
>
> I take it the current use cases you have in mind/lined up in the CAS use this directly? Maybe a `Raw` prefix? `RawPerfectHashSet`, `RawConcurrentHashSet`, etc... some combination/choice of those sort of things?

@steven_wu can confirm, but that's what I remember. (There was an early branch that used the hash-included-client-friendly set, but the use case for it was refactored IIRC.)

To summarize, seems like the patch needs:

- a new name (`RawConcurrentHashSet` SGTM);
- some header docs explaining how to use it / design motivation;
- a test support friend class that can inspect/test the trie layout without relying on stringification;
- unit tests rewritten to use the friend (and probably dropping the example higher-level data structure).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133715/new/

https://reviews.llvm.org/D133715



More information about the llvm-commits mailing list