I've definitely seen cases of the hash function being passed in as key->hash(). If that's the case, then the templated hash function could, for example, look up the hash on the key object.
On Thu, Sep 15, 2016 at 9:43 AM, 'Ross McIlroy' via v8-dev < [email protected]> wrote: > On 15 Sep 2016 9:29 a.m., <[email protected]> wrote: > > > > Hi all, > > > > We (ignition) are looking to use base/hashmap (TemplateHashMap), but > there are a couple of things that I want to change/add for efficiency's > sake. Because these changes would have far-reaching effects, I wanted to > send out the proposed changes before trying to upload any CLs. > > > > My proposed changes are: > > Template the value type, so that small types (e.g. ints) can be stored > inline rather than allocated > > Template the key type, for the same reason > > This has some far reaching effects -- e.g. we can't store holes as > nullptr keys anymore, but have to have a separate boolean existence flag. I > suggest that we have an Entry class that is templated on the key, and > specialised for pointer-typed keys to treat null keys as holes. > > Template the hash function and remove the hash fields, because passing > in our own hash values is asking for trouble > > AFAIK the hash arguments are used by the ast-value-factory such that AST > value objects stores their hash as a field and pass it to lookups to avoid > having to rehash the AST values. We would want something like this for any > replacement. It might be possible to store the cached hash files on the > hashmap entries themselves, but maybe there is some another reason we > currently have them on the AST values. > > Marja, any thoughts? > > > Template the equals/matching function, since we'd be having to template > it on the key type anyway, to skip that dereference > > Move the allocator to a field, because passing different allocators > around as parameters is super sketchy > > More precisely, move it to a private base class to take advantage of the > empty base-class optimisation > > Add a function argument to LookupOrInsert which creates the value, since > the Value type is templated and so we can't rely on null values to mean > newly-inserted entries > > And maybe, though I'm less convinced about these: > > Return value references directly for Lookup/LookupOrInsert, rather than > Entries, to make Entries non-public > > Add an stl-like iterator interface rather than Start/Next, also to make > Entries non-public > > Most of these I could wrap in a particular <Key=void*, Value=void*> > implementation that would fairly closely imitate the original hash map, but > there would (probably?) still be some differences. > > > > Comments/questions/rage? > > > > - Leszek > > > > -- > > -- > > v8-dev mailing list > > [email protected] > > http://groups.google.com/group/v8-dev > > --- > > You received this message because you are subscribed to the Google > Groups "v8-dev" group. > > To unsubscribe from this group and stop receiving emails from it, send > an email to [email protected]. > > For more options, visit https://groups.google.com/d/optout. > > -- > -- > v8-dev mailing list > [email protected] > http://groups.google.com/group/v8-dev > --- > You received this message because you are subscribed to the Google Groups > "v8-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
