On Mon, Oct 02, 2017 at 04:48:48PM -0700, Brandon Williams wrote:
> > Some replies to v1 [1] [2] seem to indicate that simpler non-duplicated
> > code should be preferred over optimizing away the storage of the 4-byte
> > hash code, and I have no objection to that, so I have updated this code
> > to be a thin wrapper over hashmap with the 4-byte overhead.
> >
> > After this patch, if the 4-byte overhead is found to be too much, we can
> > migrate to something similar to v1 relatively easily.
> >
> > I decided not to go with the util pointer method because we will not be
> > able to migrate away from it so easily if need be.
>
> This makes me a bit sad because I tend to lean more towards making
> things simpler. I'm still a supporter of the 'util' pointer but I
> understand why we would choose otherwise.
Right, I kind of wonder if this has fallen into an uncanny value where
we have this almost-hashmap infrastructure, but the end result is not
significantly easier to use than a plain-old hashmap.
I.e., it looks like you still have to declare something like:
struct my_data {
struct oidmap_entry oid;
int value; /* mapping to an int */
};
and handle the allocation of the entry yourself. If we instead just
adding an oidhash() and oidcmpfn(), then callers could those directly.
The invocations are a _little_ longer with a raw hashmap, but not much
(as you can see from the actual oidmap implementation, and the changes
to oidset).
I dunno. I'm not against it per se. The API _is_ a little nicer, but I
just wonder if there's a downside to even the thin wrapper, in that
callers are no longer free to use other parts of the hashmap API. If I
saw some converted callers I might be more convinced.
-Peff
