Andres Freund писал 2021-08-13 12:21:
Hi,
On 2021-08-10 11:52:59 +0300, Yura Sokolov wrote:
- sizemask is set only in SH_COMPUTE_PARAMETERS . And it is set in
this way:
/* now set size */
tb->size = size;
if (tb->size == SH_MAX_SIZE)
tb->sizemask = 0;
else
tb->sizemask = tb->size - 1;
that means, when we are resizing to SH_MAX_SIZE, sizemask becomes
zero.
I think that was intended to be ~0.
I believe so.
Ahh... ok, patch is updated to fix this as well.
Any chance you'd write a test for simplehash with such huge amount of
values? It'd require a small bit of trickery to be practical. On
systems
with MAP_NORESERVE it should be feasible.
Which way C structures should be tested in postgres?
dynahash/simplhash - do they have any tests currently?
I'll do if hint is given.
static inline void
-SH_COMPUTE_PARAMETERS(SH_TYPE * tb, uint32 newsize)
+SH_COMPUTE_PARAMETERS(SH_TYPE * tb, uint64 newsize)
{
uint64 size;
@@ -322,11 +322,7 @@ SH_COMPUTE_PARAMETERS(SH_TYPE * tb, uint32
newsize)
/* now set size */
tb->size = size;
-
- if (tb->size == SH_MAX_SIZE)
- tb->sizemask = 0;
- else
- tb->sizemask = tb->size - 1;
+ tb->sizemask = (uint32)(size - 1);
ISTM using ~0 would be nicer here?
I don't think so.
To be rigid it should be `~(uint32)0`.
But I believe it doesn't differ from `tb->sizemask = (uint32)(size - 1)`
that is landed with patch, therefore why `if` is needed?
Greetings,
Andres Freund
