Re: Bug in huge simplehash - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Bug in huge simplehash
Date
Msg-id 20210813101543.bkncnioyqi472man@alap3.anarazel.de
Whole thread Raw
In response to Re: Bug in huge simplehash  (Yura Sokolov <y.sokolov@postgrespro.ru>)
Responses Re: Bug in huge simplehash
List pgsql-hackers
Hi,

On 2021-08-13 12:44:17 +0300, Yura Sokolov wrote:
> Andres Freund писал 2021-08-13 12:21:
> > 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.

We don't have a great way right now :(. I think the best is to have a
SQL callable function that uses some API. See e.g. test_atomic_ops() et
al in src/test/regress/regress.c


> > >  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?

Personally I find it more obvious to understand the intended behaviour
with ~0 (i.e. all bits set) than with a width truncation.

Greetings,

Andres Freund



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Next
From: Ram Charan Kallem
Date:
Subject: RE: Multiple Postgres process are running in background