On Thu, 8 Sept 2022 at 05:32, Oscar Salvador <osalvador@xxxxxxx> wrote: > > On Wed, Sep 07, 2022 at 09:14:35AM +0200, Marco Elver wrote: > > Why are you casting a stack_record** to a stack_record*? stack_table > > is already appropriately typed, and there should be no need to cast > > things around. > > > > 'stacks' is supposed to be the bucket? In which case you need to > > dereference it to get the first entry in the bucket: bucket = > > stack_table[table_i]; > > > > stack_i cannot be used to index into the bucket, because the elements > > in it are a linked list and not necessarily adjacent in memory. You > > have to traverse the linked list stack_i elements to get to the start: > > Yea, I figured that much after thinking about more, but I was overly > eager. > > > for (int i = 0; stack && i < stack_i; stack = stack->next, ++i); > > But this seems suboptimal. > With this code, we have to walk the list till we find the right index > every time we enter the function, while the actual code of v2 > or even the patch from v1 [1], we do not really need to do that > because we already have the pointer to the stack. > > So I much rather prefer that, than having to traverse the stacks > till the find the right one. I would not prematurely optimize this. It's a hash map, and the only problem is if there are tons of collisions. Also, this function isn't performance critical, it's only used for printing, which itself is slow. I suggest you collect some stats how many entries each bucket has on average. If the average is <10, I'd go with the cleaner interface.
