Re: Improve monitoring of shared memory allocations

On 3/31/25 01:01, Rahila Syed wrote:
> ...
> 
> 
>     > I will improve the comment in the next version.
>     >
> 
>     OK. Do we even need to pass nelem_alloc to hash_get_init_size? It's not
>     really used except for this bit:
> 
>     +    if (init_size > nelem_alloc)
>     +        element_alloc = false;
> 
>     Can't we determine before calling the function, to make it a bit less
>     confusing?
> 
> 
> Yes, we could determine whether the pre-allocated elements are zero before
> calling the function, I have fixed it accordingly in the attached 0001
> patch. 
> Now, there's no need to pass `nelem_alloc` as a parameter. Instead, I've
> passed this information as a boolean variable-initial_elems. If it is
> false,
> no elements are pre-allocated.
> 
> Please find attached the v7-series, which incorporates your review patches
> and addresses a few remaining comments.
> 

I think it's almost committable. Attached is v8 with some minor review
adjustments, and updated commit messages. Please read through those and
feel free to suggest changes.

I still found the hash_get_init_size() comment unclear, and it also
referenced init_size, which is no longer relevant. I improved the
comment a bit (I find it useful to mimic comments of nearby functions,
so I did that too here). The "initial_elems" name was a bit confusing,
as it seemed to suggest "number of elements", but it's a simple flag. So
I renamed it to "prealloc", which seems clearer to me. I also tweaked
(reordered/reformatted) the conditions a bit.

For the other patch, I realized we can simply MemSet() the whole chunk,
instead of resetting the individual parts.

regards

-- 
Tomas Vondra