Re: "Allow usage of huge maintenance_work_mem for GIN build" patch

> --- a/src/include/access/gin_private.h
> +++ b/src/include/access/gin_private.h
> @@ -903,7 +903,7 @@ typedef struct GinEntryAccumulator
> typedef struct
> {
> GinState *ginstate;
> - long allocatedMemory;
> + Size allocatedMemory;
> GinEntryAccumulator *entryallocator;
> uint32 eas_used;
> RBTree *tree;
>
> Are you sure this is safe, Teodor? I don't have time to study the
> patch in detail, but offhand I think that it might have been better to
> make allocatedMemory of type int64, just like the tuplesort.c memory
> accounting variables are post-MaxAllocHuge. It's not obvious to me
> that this variable isn't allowed to occasionally become negative, just
> like in tuplesort.c. It looks like that *might* be true -- ginbulk.c
> may let allocatedMemory go negative for a period, which would now be
> broken.
It could not be negative - subtruction is doing only around repalloc call, in
all other places it only grows.

>
> If you did make this exact error, you would not be the first. If it
> isn't actually broken, perhaps you should still make this change,
> simply on general principle. I'd like to hear other opinions on that,
> though.
It could be an int64 without any regressions or problems. I choose Size type
because variable allocatedMemory actually stores a size of piece of memory and
it's the same type as returned by GetMemoryChunkSpace()

AFAIK, size_t type (type Size is a just typedef alias) is a part of C99 and it
should be unsigned and large enough to store size of any chunk of avaliable RAM.
And it at least twice larger than allowed all memory-related GUC variables.

I don't see a reason to use int64 except, may be, general practice in pgsql. But
then why we keep Size type - let us just use int64.

--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/