Messages in this thread | | | Date | Fri, 24 Aug 2012 22:11:55 +0200 | From | Sasha Levin <> | Subject | Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable |
| |
On 08/24/2012 09:59 PM, Tejun Heo wrote: > Hello, Sasha. > > On Fri, Aug 24, 2012 at 09:47:19PM +0200, Sasha Levin wrote: >>> I think this is problematic. It looks exactly like other existing >>> DEFINE macros yet what its semantics is different. I don't think >>> that's a good idea. >> >> I can switch that to be DECLARE_HASHTABLE() if the issue is semantics. > > If this implementation is about the common trivial case, why not just > have the usual DECLARE/DEFINE_HASHTABLE() combination?
When we add the dynamic non-resizable support, how would DEFINE_HASHTABLE() look?
>>> So, I think it would be best to keep this one as straight-forward and >>> trivial as possible. Helper macros to help its users are fine but >>> let's please not go for full encapsulation. >> >> What if we cut off the dynamic allocated (but not resizable) hashtable out for >> the moment, and focus on the most common statically allocated hashtable case? >> >> The benefits would be: >> >> - Getting rid of all the _size() macros, which will make the amount of helpers >> here reasonable. >> - Dynamically allocated hashtable can be easily added as a separate >> implementation using the same API. We already have some of those in the kernel... > > It seems we have enough of this static usage and solving the static > case first shouldn't hinder the dynamic (!resize) case later, so, > yeah, sounds good to me. > >> - When that's ready, I feel it's a shame to lose full encapsulation just due to >> hash_hashed(). > > I don't know. If we stick to the static (or even !resize dymaic) > straight-forward hash - and we need something like that - I don't see > what the full encapsulation buys us other than a lot of trivial > wrappers.
Which macros do you consider as trivial within the current API?
Basically this entire thing could be reduced to DEFINE/DECLARE_HASHTABLE and get_bucket(), but it would make the life of anyone who wants a slightly different hashtable a hell.
I think that right now the only real trivial wrapper is hash_hashed(), and I think it's a price worth paying to have a single hashtable API instead of fragmenting it when more implementations come along.
Thanks, Sasha
> > Thanks. >
| |