[PATCH] D22681: [esan] Add generic resizing hashtable

[Mike Aizatsky via llvm-commits]

aizatsky requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: lib/esan/esan_hashtable.h:24
@@ +23,3 @@
+// KeyTy must have an operator== and an operator size_t() (for hashing).
+// Pointer types will be automatically de-referenced prior to either operator==
+// or a size_t hash cast.
----------------
Key pointer types...

================
Comment at: lib/esan/esan_hashtable.h:24
@@ +23,3 @@
+// KeyTy must have an operator== and an operator size_t() (for hashing).
+// Pointer types will be automatically de-referenced prior to either operator==
+// or a size_t hash cast.
----------------
aizatsky wrote:
> Key pointer types...
I actually wonder if this is a functionality you want to build in rather then provide through type traits.

Like this:

template <typename KeyTy, typename DataTy, typename KeyTraits = DefaultTraits<KeyTy>>

And later something like:

KeyTraits::hash(key)
KeyTraits::eq(key1, key2).


================
Comment at: lib/esan/esan_hashtable.h:42
@@ +41,3 @@
+  ~HashTable();
+  bool lookup(KeyTy Key, DataTy &Payload);
+  bool add(KeyTy Key, DataTy Payload);
----------------
any particular reason why not

DataTy* lookup(KeyTy Key) ?

Seems to me it would always save a declaration line for clients.

================
Comment at: lib/esan/esan_hashtable.h:42
@@ +41,3 @@
+  ~HashTable();
+  bool lookup(KeyTy Key, DataTy &Payload);
+  bool add(KeyTy Key, DataTy Payload);
----------------
aizatsky wrote:
> any particular reason why not
> 
> DataTy* lookup(KeyTy Key) ?
> 
> Seems to me it would always save a declaration line for clients.
const

================
Comment at: lib/esan/esan_hashtable.h:45
@@ +44,3 @@
+  bool remove(KeyTy Key);
+  u32 size();
+  // If the table is internally-synchronized, this lock must not be held
----------------
const

================
Comment at: lib/esan/esan_hashtable.h:66
@@ +65,3 @@
+  u32 Entries;
+  u32 ResizeFactor;
+  BlockingMutex Mutex;
----------------
const this and other non-changed fields.


https://reviews.llvm.org/D22681