Committed, thanks for the review!
I made one last-minute change: Instead of allocating a memory context in
intset_create(), it is left to the caller. The GiST vacuum code created
two integer sets, and it made more sense for it to use the same context
for both. Also, the VACUUM tid list patch would like to use a memory
context with very large defaults, so that malloc() will decide to
mmap() it, so that the memory can be returned to the OS. Because the
desired memory allocation varies between callers, so it's best to leave
it to the caller.
On 20/03/2019 19:50, Julien Rouhaud wrote:
> I forgot to mention a minor gripe about the intset_binsrch_uint64 /
> intset_binsrch_leaf function, which are 99% duplicates. But I don't
> know if fixing that (something like passing the array as a void * and
> passing a getter function?) is worth the trouble.
Yeah. I felt that it's simpler to have two almost-identical functions,
even though it's a bit repetitive, than try to have one function serve
both uses. The 'nextkey' argument is always passed as false to
intset_binsrch_leaf() function, but I kept it anyway, to keep the
functions identical. The compiler will surely optimize it away, so it
makes no difference to performance.
On 20/03/2019 04:48, Andrey Borodin wrote:
> 1. I'm not sure it is the best interface for iteration
>
> uint64
> intset_iterate_next(IntegerSet *intset, bool *found)
>
> we will use it like
>
> while
> {
> bool found;
> BlockNumber x = (BlockNumber) intset_iterate_next(is, &found);
> if (!found)
> break;
> // do stuff
> }
>
> we could use it like
>
> BlockNumber x;
> while(intset_iterate_next(is, &x))
> {
> // do stuff
> }
>
> But that's not a big difference.
Agreed, I changed it that way.
- Heikki