From: | Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
Subject: | Re: Bug in huge simplehash |
Date: | 2021-08-13 11:12:59 |
Message-ID: | CAEudQAr+pYmPJq6afJboaCWGp-G_Wj=n1kpvX6Sk8M4WkW6uzw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Em sex., 13 de ago. de 2021 às 07:15, Andres Freund <andres(at)anarazel(dot)de>
escreveu:
> 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.
>
https://godbolt.org/z/57WcjKqMj
The generated code is identical.
regards,
Ranier Vilela
From | Date | Subject | |
---|---|---|---|
Next Message | Yura Sokolov | 2021-08-13 11:40:08 | Re: Bug in huge simplehash |
Previous Message | Ajin Cherian | 2021-08-13 11:00:58 | Re: logical replication empty transactions |