MIME-Version: 1.0
References: 
 <CAH2L28vHzRankszhqz7deXURxKncxfirnuW68zD7+hVAqaS5GQ@mail.gmail.com>
 <k6f6ynjvb7lvebhygaiqsfrdohq672uughk3q4ve4q5jqljywz@7jbrz724epeq>
 <CAH2L28uL-8EQTRSeyTpW1DqAXsDXRkXCkT1dus2u6p4HYDrxAg@mail.gmail.com>
 <CAH2L28uGLhkXBKDWFKm5XZtp_0nNqpYQ3Hc35vG++mM7wuOhgg@mail.gmail.com>
 <CAN55FZ3cJxy0VkeXpuO3K4BpjzJo3S6oU+iMyc00P6gEjqPztw@mail.gmail.com>
 <CAH2L28uwxJREzB62UjRDBumE87hHWUJJvRwxqqbO+7qFmoZfTg@mail.gmail.com>
 <3e40eeec-d8bf-4496-854e-485dd901f6a2@vondra.me>
 <CAH2L28uG_g1Ljo8aL-g1MupJXO4Y7-a-bUCriE7w2213+KSGdA@mail.gmail.com>
 <6bf7194e-4c34-4e6d-8215-f6acf8903974@vondra.me>
 <6705dbd2-060b-4f3c-9fcb-1c7f10880b26@vondra.me>
In-Reply-To: <6705dbd2-060b-4f3c-9fcb-1c7f10880b26@vondra.me>
From: Rahila Syed <rahilasyed90@gmail.com>
Date: Fri, 28 Mar 2025 16:40:44 +0530
Message-ID: 
 <CAH2L28tzCFEk2bxQ+oYv6zda=LFLfd_9cmq7HzsT4nj9KN1Yvg@mail.gmail.com>
Subject: Re: Improve monitoring of shared memory allocations
To: Tomas Vondra <tomas@vondra.me>
Cc: Nazir Bilal Yavuz <byavuz81@gmail.com>,
 Andres Freund <andres@anarazel.de>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Type: multipart/alternative; boundary="000000000000ddd0510631651f64"
Archived-At: 
 <https://www.postgresql.org/message-id/CAH2L28tzCFEk2bxQ%2BoYv6zda%3DLFLfd_9cmq7HzsT4nj9KN1Yvg%40mail.gmail.com>
Precedence: bulk

--000000000000ddd0510631651f64
Content-Type: text/plain; charset="UTF-8"

Hi Tomas,


1) alignment
>
> There was a comment with a question whether we need to MAXALIGN the
> chunks in dynahash.c, which were originally allocated by ShmemAlloc, but
> now it's part of one large allocation, which is then cut into pieces
> (using pointer arithmetics).
>
> I was not sure whether we need to enforce some alignment, we briefly
> discussed that off-list. I realize you chose to add the alignment, but I
> haven't noticed any comment in the patch why it's needed, and it seems
> to me it may not be quite correct.
>


I have added MAXALIGN to specific allocations, such as HASHHDR and
HASHSEGMENT, with the expectation that allocations in multiples of this,
like dsize * HASHSEGMENT, would automatically align.


> Let me explain what I had in mind, and why I think the way v5 doesn't
> actually do that. It took me a while before I understood what alignment
> is about, and for a while it was haunting my patches, so hopefully this
> will help others ...
>
> The "alignment" is about pointers (or addresses), and when a pointer is
> aligned it means the address is a multiple of some number. For example
> 4B-aligned pointer is a multiple of 4B, so 0x00000100 is 4B-aligned,
> while 0x00000101 is not. Sometimes we use data types to express the
> alignment, e.g. int-aligned is 4B-aligned, but that's a detail. AFAIK
> the alignment is always 2^k, so 1, 2, 4, 8, ...
>
> The primary reason for alignment is that some architectures require the
> pointers to be well-aligned for a given data type. For example (int*)
> needs to be int-aligned. If you have a pointer that's not 4B-aligned,
> it'll trigger SIGBUS or maybe SIGSEGV. This was true for architectures
> like powerpc, I don't think x86/arm64 have this restriction, i.e. it'd
> work, even if there might be a minor performance impact. Anyway, we
> still enforce/expect correct alignment, because we may still support
> some of those alignment-sensitive platforms, and it's "tidy".
>
> The other reason is that we sometimes use alignment to add padding, to
> reduce contention when accessing elements in hot arrays. We want to
> align to cacheline boundaries, so that a struct does not require
> accessing more cachelines than really necessary. And also to reduce
> contention - the more cachelines, the higher the risk of contention.
>
>
Thank you for your explanation. I had a similar understanding. However,
I believed that MAXALIGN and CACHEALIGN are primarily performance
optimizations
that do not impact the correctness of the code. This assumption is based on
the fact
that I have not observed any failures on GitHub CI, even when changing the
alignment
in this part of the code.


Now, back to the patch. The code originally did this in ShmemInitStruct
>
>     hashp = ShmemInitStruct(...)
>
> to allocate the hctl, and then
>
>     firstElement = (HASHELEMENT *) ShmemAlloc(nelem * elementSize);
>
> in element_alloc(). But this means the "elements" allocation is aligned
> to PG_CACHE_LINE_SIZE, i.e. 128B, because ShmemAllocRaw() does this:
>
>     size = CACHELINEALIGN(size);
>
> So it distributes memory in multiples of 128B, and I believe it starts
> at a multiple of 128B.
>
> But the patch reworks this to allocate everything at once, and thus it
> won't get this alignment automatically. AFAIK that's not intentional,
> because no one explicitly mentioned this. And it's may not be quite
> desirable, judging by the comment in ShmemAllocRaw().
>
>
Yes, the patch reworks this to allocate all the shared memory at once.
It uses ShmemInitStruct which internally calls ShmemAllocRaw. So the whole
chunk
of memory allocated is still CACHEALIGNed.

I mentioned v5 adds alignment, but I think it does not quite do that
> quite correctly. It adds alignment by changing the macros from:
>
> +#define HASH_ELEMENTS_OFFSET(hctl, nsegs) \
> +       (sizeof(HASHHDR) + \
> +        ((hctl)->dsize * sizeof(HASHSEGMENT)) + \
> +        ((hctl)->ssize * (nsegs) * sizeof(HASHBUCKET)))
>
> to
>
> +#define HASH_ELEMENTS_OFFSET(hctl, nsegs) \
> +       (MAXALIGN(sizeof(HASHHDR)) + \
> +        ((hctl)->dsize * MAXALIGN(sizeof(HASHSEGMENT))) + \
> +        ((hctl)->ssize * (nsegs) * MAXALIGN(sizeof(HASHBUCKET))))
>
> First, it uses MAXALIGN, but that's mostly my fault, because my comment
> suggested that - the ShmemAllocRaw however and makes the case for using
> CACHELINEALIGN.
>

Good catch. For a shared hash table, allocations need to be
CACHELINEALIGNED.
I think hash_get_init_size does not need to call CACHELINEALIGNED
explicitly as ShmemInitStruct already does this.
In that case, the size returned by hash_get_init_size just needs to
MAXALIGN required structs as per hash_create() requirements and
CACHELINEALIGN
will be taken care of in ShmemInitStruct at the time of allocating the
entire chunk.


> But more importantly, it adds alignment to all hctl field, and to every
> element of those arrays. But that's not what the alignment was supposed
> to do - it was supposed to align arrays, not individual elements. Not
> only would this waste memory, it would actually break direct access to
> those array elements.
>

I think existing code has occurrences of both i,.e aligning individual
elements and
arrays.
A similar precedent exists in the function hash_estimate_size(), which only
applies maxalignment to the individual structs like HASHHDR, HASHELEMENT,
entrysize, but also an array of HASHBUCKET headers.

I agree with you that perhaps we don't need maxalignment for all of these
structures.
For ex, HASHBUCKET is a pointer to a linked list of elements, it might not
require alignment
if the elements it points to are already aligned.


> But there's another detail - even before this patch, most of the stuff
> was allocated at once by ShmemInitStruct(). Everything except for the
> elements, so to replicate the alignment we only need to worry about that
> last part. So I think this should do:
>


> +#define HASH_ELEMENTS_OFFSET(hctl, nsegs) \
> +    CACHELINEALIGN(sizeof(HASHHDR) + \
> +     ((hctl)->dsize * sizeof(HASHSEGMENT)) + \
> +     ((hctl)->ssize * (nsegs) * sizeof(HASHBUCKET)))
>
> This is what the 0003 patch does. There's still one minor difference, in
> that we used to align each segment independently - each element_alloc()
> call allocated a new CACHELINEALIGN-ed chunk, while now have just a
> single chunk. But I think that's OK.
>
>
Before this patch, following structures were allocated separately using
ShmemAllocRaw
directory, each segment(seg_alloc) and a chunk of elements (element_alloc).
Hence,
I don't understand why v-0003* CACHEALIGNs  in the manner it does.

I think if we want to emulate the current behaviour we should do something
like:
CACHELINEALIGN(sizeof(HASHHDR) + dsize * sizeof(HASHSEGMENT)) +
                + CACHELINEALIGN(sizeof(HASHBUCKET) * ssize) * nsegs
                + CACHELINEALIGN(init_size * elementSize);

Like you mentioned the only difference would be that we would be aligning
all elements
at once instead of aligning individual partitions of elements.


3) I find the comment before hash_get_init_size a bit unclear/confusing.
> It says this:
>
>  * init_size should match the total number of elements allocated during
>  * hash table creation, it could be zero for non-shared hash tables
>  * depending on the value of nelem_alloc. For more explanation see
>  * comments within this function.
>  *
>  * nelem_alloc parameter is not relevant for shared hash tables.
>
> What does "should match" mean here? Doesn't it *determine* the number of
> elements allocated? What if it doesn't match?
>

by should match I mean - init_size  here  *is* equal to nelem in
hash_create() .

>
> AFAICS it means the hash table is sized to expect init_size elements,
> but only nelem_alloc elements are actually pre-allocated, right?


No. All the init_size elements are pre-allocated for shared hash table
irrespective of
nelem_alloc value.
For non-shared hash tables init_size elements are allocated only
if it is less than nelem_alloc, otherwise they are allocated as part of
expansion.


> But the
> comment says it's init_size which determines the number of elements
> allocated during creation. Confusing.
>
> It says "it could be zero ... depending on the value of nelem_alloc".
> Depending how? What's the relationship.
>
>
The relationship is defined in this comment:
 /*
* For a shared hash table, preallocate the requested number of elements.
* This reduces problems with run-time out-of-shared-memory conditions.
*
* For a non-shared hash table, preallocate the requested number of
* elements if it's less than our chosen nelem_alloc.  This avoids wasting
* space if the caller correctly estimates a small table size.
*/

hash_create code is confusing because the nelem_alloc named variable is used
in two different cases, In  the above case  nelem_alloc  refers to the one
returned by choose_nelem_alloc function.

The other nelem_alloc determines the number of elements in each partition
for a partitioned hash table. This is not what is being referred to in the
above
comment.

The bit "For more explanation see comments within this function" is not
> great, if only because there are not many comments within the function,
> so there's no "more explanation". But if there's something important, it
> should be in the main comment, preferably.
>
>
I will improve the comment in the next version.

Thank you,
Rahila Syed

--000000000000ddd0510631651f64
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi Tomas,<br><br><br></div><div class=3D"=
gmail_quote gmail_quote_container"><blockquote class=3D"gmail_quote" style=
=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding=
-left:1ex">
1) alignment<br>
<br>
There was a comment with a question whether we need to MAXALIGN the<br>
chunks in dynahash.c, which were originally allocated by ShmemAlloc, but<br=
>
now it&#39;s part of one large allocation, which is then cut into pieces<br=
>
(using pointer arithmetics).<br>
<br>
I was not sure whether we need to enforce some alignment, we briefly<br>
discussed that off-list. I realize you chose to add the alignment, but I<br=
>
haven&#39;t noticed any comment in the patch why it&#39;s needed, and it se=
ems<br>
to me it may not be quite correct.<br></blockquote><div>=C2=A0</div><div><b=
r>I have added MAXALIGN to specific allocations, such as HASHHDR and <br>HA=
SHSEGMENT, with the expectation that allocations in multiples of this, <br>=
like dsize * HASHSEGMENT, would automatically align.<br><br></div><div><br>=
</div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;b=
order-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Let me explain what I had in mind, and why I think the way v5 doesn&#39;t<b=
r>
actually do that. It took me a while before I understood what alignment<br>
is about, and for a while it was haunting my patches, so hopefully this<br>
will help others ...<br>
<br>
The &quot;alignment&quot; is about pointers (or addresses), and when a poin=
ter is<br>
aligned it means the address is a multiple of some number. For example<br>
4B-aligned pointer is a multiple of 4B, so 0x00000100 is 4B-aligned,<br>
while 0x00000101 is not. Sometimes we use data types to express the<br>
alignment, e.g. int-aligned is 4B-aligned, but that&#39;s a detail. AFAIK<b=
r>
the alignment is always 2^k, so 1, 2, 4, 8, ...<br>
<br>
The primary reason for alignment is that some architectures require the<br>
pointers to be well-aligned for a given data type. For example (int*)<br>
needs to be int-aligned. If you have a pointer that&#39;s not 4B-aligned,<b=
r>
it&#39;ll trigger SIGBUS or maybe SIGSEGV. This was true for architectures<=
br>
like powerpc, I don&#39;t think x86/arm64 have this restriction, i.e. it=
9;d<br>
work, even if there might be a minor performance impact. Anyway, we<br>
still enforce/expect correct alignment, because we may still support<br>
some of those alignment-sensitive platforms, and it&#39;s &quot;tidy&quot;.=
<br>
<br>
The other reason is that we sometimes use alignment to add padding, to<br>
reduce contention when accessing elements in hot arrays. We want to<br>
align to cacheline boundaries, so that a struct does not require<br>
accessing more cachelines than really necessary. And also to reduce<br>
contention - the more cachelines, the higher the risk of contention.<br>
<br></blockquote><div>=C2=A0</div><div>Thank you for your explanation. I ha=
d a similar understanding. However, <br>I believed that MAXALIGN and CACHEA=
LIGN are primarily performance optimizations<br>that do not impact the corr=
ectness of the code. This assumption is based on the fact<br>that I have no=
t observed any failures on GitHub CI, even when changing the alignment<br>i=
n this part of the code.<br><br><br></div><blockquote class=3D"gmail_quote"=
 style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);p=
adding-left:1ex">
Now, back to the patch. The code originally did this in ShmemInitStruct<br>
<br>
=C2=A0 =C2=A0 hashp =3D ShmemInitStruct(...)<br>
<br>
to allocate the hctl, and then<br>
<br>
=C2=A0 =C2=A0 firstElement =3D (HASHELEMENT *) ShmemAlloc(nelem * elementSi=
ze);<br>
<br>
in element_alloc(). But this means the &quot;elements&quot; allocation is a=
ligned<br>
to PG_CACHE_LINE_SIZE, i.e. 128B, because ShmemAllocRaw() does this:<br>
<br>
=C2=A0 =C2=A0 size =3D CACHELINEALIGN(size);<br>
<br>
So it distributes memory in multiples of 128B, and I believe it starts<br>
at a multiple of 128B.<br>
<br>
But the patch reworks this to allocate everything at once, and thus it<br>
won&#39;t get this alignment automatically. AFAIK that&#39;s not intentiona=
l,<br>
because no one explicitly mentioned this. And it&#39;s may not be quite<br>
desirable, judging by the comment in ShmemAllocRaw().<br>
<br></blockquote><div><br>Yes, the patch reworks this to allocate all the s=
hared memory at once.<br>It uses ShmemInitStruct which internally calls Shm=
emAllocRaw. So the whole chunk <br>of memory allocated is still CACHEALIGNe=
d.=C2=A0<br><br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px=
 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
I mentioned v5 adds alignment, but I think it does not quite do that<br>
quite correctly. It adds alignment by changing the macros from:<br>
<br>
+#define HASH_ELEMENTS_OFFSET(hctl, nsegs) \<br>
+=C2=A0 =C2=A0 =C2=A0 =C2=A0(sizeof(HASHHDR) + \<br>
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 ((hctl)-&gt;dsize * sizeof(HASHSEGMENT)) + \<b=
r>
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 ((hctl)-&gt;ssize * (nsegs) * sizeof(HASHBUCKE=
T)))<br>
<br>
to<br>
<br>
+#define HASH_ELEMENTS_OFFSET(hctl, nsegs) \<br>
+=C2=A0 =C2=A0 =C2=A0 =C2=A0(MAXALIGN(sizeof(HASHHDR)) + \<br>
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 ((hctl)-&gt;dsize * MAXALIGN(sizeof(HASHSEGMEN=
T))) + \<br>
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 ((hctl)-&gt;ssize * (nsegs) * MAXALIGN(sizeof(=
HASHBUCKET))))<br>
<br>
First, it uses MAXALIGN, but that&#39;s mostly my fault, because my comment=
<br>
suggested that - the ShmemAllocRaw however and makes the case for using<br>
CACHELINEALIGN.<br></blockquote><div><br>Good catch. For a shared hash tabl=
e, allocations need to be<br>CACHELINEALIGNED.=C2=A0=C2=A0<br>I think hash_=
get_init_size does not need to call CACHELINEALIGNED <br>explicitly as Shme=
mInitStruct already does this. <br>In that case, the size returned by hash_=
get_init_size just needs to<br>MAXALIGN required structs as per hash_create=
() requirements and CACHELINEALIGN<br>will be taken care of in ShmemInitStr=
uct at the time of allocating the entire chunk.<br><br></div><blockquote cl=
ass=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid=
 rgb(204,204,204);padding-left:1ex">
<br>
But more importantly, it adds alignment to all hctl field, and to every<br>
element of those arrays. But that&#39;s not what the alignment was supposed=
<br>
to do - it was supposed to align arrays, not individual elements. Not<br>
only would this waste memory, it would actually break direct access to<br>
those array elements.<br></blockquote><div><br>I think existing code has oc=
currences of both i,.e aligning individual elements and=C2=A0<br>arrays.<br=
>A similar precedent exists in the function hash_estimate_size(), which onl=
y<br>applies maxalignment to the individual structs like HASHHDR, HASHELEME=
NT, <br>entrysize, but also an array of HASHBUCKET headers.=C2=A0</div><div=
><br></div><div>I agree with you that perhaps we don&#39;t need maxalignmen=
t for all of these structures. <br>For ex, HASHBUCKET is a pointer to a lin=
ked list of elements, it might not require alignment <br>if the elements it=
 points=C2=A0to are already aligned.<br><br></div><blockquote class=3D"gmai=
l_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,20=
4,204);padding-left:1ex">
<br>
But there&#39;s another detail - even before this patch, most of the stuff<=
br>
was allocated at once by ShmemInitStruct(). Everything except for the<br>
elements, so to replicate the alignment we only need to worry about that<br=
>
last part. So I think this should do:<br></blockquote><div>=C2=A0</div><blo=
ckquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left=
:1px solid rgb(204,204,204);padding-left:1ex">
+#define HASH_ELEMENTS_OFFSET(hctl, nsegs) \<br>
+=C2=A0 =C2=A0 CACHELINEALIGN(sizeof(HASHHDR) + \<br>
+=C2=A0 =C2=A0 =C2=A0((hctl)-&gt;dsize * sizeof(HASHSEGMENT)) + \<br>
+=C2=A0 =C2=A0 =C2=A0((hctl)-&gt;ssize * (nsegs) * sizeof(HASHBUCKET)))<br>
<br>
This is what the 0003 patch does. There&#39;s still one minor difference, i=
n<br>
that we used to align each segment independently - each element_alloc()<br>
call allocated a new CACHELINEALIGN-ed chunk, while now have just a<br>
single chunk. But I think that&#39;s OK.<br>
<br></blockquote><div>=C2=A0</div><div>Before this patch, following structu=
res were allocated separately using ShmemAllocRaw<br></div><div>directory, =
each segment(seg_alloc) and a chunk of elements (element_alloc). Hence,=C2=
=A0</div><div>I don&#39;t understand why v-0003* CACHEALIGNs=C2=A0 in the m=
anner it does.<br><br>I think if we want to emulate the current behaviour w=
e should do something like:<br>CACHELINEALIGN(sizeof(HASHHDR) + dsize * siz=
eof(HASHSEGMENT)) +<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 + CACHELINEALIGN(sizeof(HASHBUCKET) * ssize) * nsegs<br>=C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 + CACHELINEALIGN(init_size * elem=
entSize);<br><br>Like you mentioned the only difference would be that we wo=
uld be aligning all elements<br>at once instead of aligning individual part=
itions of elements.<br><br><br></div><blockquote class=3D"gmail_quote" styl=
e=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);paddin=
g-left:1ex">
3) I find the comment before hash_get_init_size a bit unclear/confusing.<br=
>
It says this:<br>
<br>
=C2=A0* init_size should match the total number of elements allocated durin=
g<br>
=C2=A0* hash table creation, it could be zero for non-shared hash tables<br=
>
=C2=A0* depending on the value of nelem_alloc. For more explanation see<br>
=C2=A0* comments within this function.<br>
=C2=A0*<br>
=C2=A0* nelem_alloc parameter is not relevant for shared hash tables.<br>
<br>
What does &quot;should match&quot; mean here? Doesn&#39;t it *determine* th=
e number of<br>
elements allocated? What if it doesn&#39;t match?<br></blockquote><div>=C2=
=A0</div><div>by should match I mean - init_size=C2=A0 here=C2=A0 *is* equa=
l to nelem in hash_create() .</div><blockquote class=3D"gmail_quote" style=
=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding=
-left:1ex">
<br>
AFAICS it means the hash table is sized to expect init_size elements,<br>
but only nelem_alloc elements are actually pre-allocated, right?</blockquot=
e><div>=C2=A0</div><div>No. All the init_size elements are pre-allocated fo=
r shared hash table irrespective of=C2=A0<br>nelem_alloc value.<br>For non-=
shared hash tables init_size elements are allocated only<br>if it is less t=
han nelem_alloc, otherwise they are allocated as part of expansion.<br>=C2=
=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8e=
x;border-left:1px solid rgb(204,204,204);padding-left:1ex"> But the<br>
comment says it&#39;s init_size which determines the number of elements<br>
allocated during creation. Confusing.<br>
<br>
It says &quot;it could be zero ... depending on the value of nelem_alloc&qu=
ot;.<br>
Depending how? What&#39;s the relationship.<br>
<br></blockquote><div>=C2=A0</div><div>The relationship is defined in this =
comment:</div><div>=C2=A0/*<br>	 * For a shared hash table, preallocate the=
 requested number of elements.<br>	 * This reduces problems with run-time o=
ut-of-shared-memory conditions.<br>	 *<br>	 * For a non-shared hash table, =
preallocate the requested number of<br>	 * elements if it&#39;s less than o=
ur chosen nelem_alloc.=C2=A0 This avoids wasting<br>	 * space if the caller=
 correctly estimates a small table size.<br>	 */<br><br>hash_create code is=
 confusing because the nelem_alloc named variable is used<br>in two differe=
nt cases, In=C2=A0 the above case=C2=A0 nelem_alloc=C2=A0 refers to the one=
=C2=A0<br>returned by choose_nelem_alloc function.<br><br>The other nelem_a=
lloc determines the number of elements in each partition<br>for a partition=
ed=C2=A0hash table. This is not what is being referred to in the above=C2=
=A0<br>comment.<br><br></div><blockquote class=3D"gmail_quote" style=3D"mar=
gin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1=
ex">
The bit &quot;For more explanation see comments within this function&quot; =
is not<br>
great, if only because there are not many comments within the function,<br>
so there&#39;s no &quot;more explanation&quot;. But if there&#39;s somethin=
g important, it<br>
should be in the main comment, preferably.<br>
<br></blockquote><div>=C2=A0</div><div>I will improve the comment in the ne=
xt version.<br><br>Thank you,<br>Rahila Syed</div></div></div>

--000000000000ddd0510631651f64--