On Tue, Aug 23, 2011 at 02:40:56PM +0300, Dimitrios Apostolou wrote:
> I was hoping you'd ask so I'll ask back :-) Why are we doing it that
> way? Why so much indirection in the first place? Why create inline
> functions just to typecast and why do we need this CONST_CAST2
> ugliness in C code. I bet there are things I don't understand so I'd
> be happy to listen...
It is not indirection, it is abstraction, which should make the code more
readable and allow changing the implementation details.
> OK I'll do that in the future. Should I also move some other htab
> functions I saw in var-tracking and rtl? FOR_EACH_HTAB_ELEMENT comes
> to mind, probably other too.
I guess FOR_EACH_HTAB_ELEMENT could move too (together with all the support
though - tree-flow.h and tree-flow-inline.h related stuff).
> It's used only once, that's why I deleted the function. I'll bring
> it back if you think it helps.
Yes, please.
> >> dst->vars = (shared_hash) pool_alloc (shared_hash_pool);
> >> dst->vars->refcount = 1;
> >> dst->vars->htab
> >>- = htab_create (MAX (src1_elems, src2_elems), variable_htab_hash,
> >>+ = htab_create (2 * MAX (src1_elems, src2_elems), variable_htab_hash,
> >> variable_htab_eq, variable_htab_free);
> >
> >This looks wrong, 2 * max is definitely too much.
>
> For a hash table to fit N elements, it has to have at least 4/3*N
> slots, or 2*N slots if htab has the 50% load factor I was proposing.
For var-tracking, 50% load factor is IMHO a very bad idea, memory
consumption of var-tracking is already high right now, we IMHO don't have
the luxury to waste more RAM.
> In total for dataflow_set_destroy I can see that calls to
> attrs_list_clear() have been reduced from 500K to 250K, and I can
> also see a reduction of free() calls from htab_delete(), from 30K to
free calls? If you avoid free calls, then you end up with a memory leak.
I can understand when you avoid pool_free calls...
Jakub
