On Tue, 1 Sept 2026 at 11:10, Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
>
> On Tue, 1 Sept 2026 at 00:04, David Rowley <dgrowleyml@gmail.com> wrote:
> > Shouldn't nchunks be uint64 anyway? Nothing guarantees Size is bigger
> > than int, even on 64-bit.
>
> True. But AFAIK, Size must be able to contain the largest possible
> pointer difference, and that implies that we can't have more than
> SIZE_MAX allocated chunks. Using Size for maths in those cases seems
> appropriate, to avoid requiring expensive oversized registers on
> 32-bit builds.
Pointer difference relates to the size of a chunk. We're talking about
the number of chunks. If a 64-bit platform exists with a 32-bit
size_t, and you create > 2^32 chunks, then the counter will wrap and
you'll mistakenly raise a WARNING.
I also think it's better to just remove the chunks_allocated struct
field. It's not testing any code that exists in
non-MEMORY_CONTEXT_CHECKING. All that's happening is you're adding a
bunch of new code in MEMORY_CONTEXT_CHECKING builds and verifying that
new code is ok. If you didn't add it, you wouldn't need to check it.
More reviewing of 0004:
1. These Asserts also seem strange as they check something that's
already been checked:
+ Assert(total_allocated == context->mem_allocated);
+ Assert(chunks_allocated == ctx->chunks_allocated);
2. I think ExternalChunkGetBlock should be called something else as it
returns a pointer to a ProxyChunk. Maybe MemoryChunkGetProxyChunk? You
should also document what the parameter is to that macro.
3. Is "#include <limits.h>" just for INT_MAX? Normally we'd use
PG_INT32_MAX from c.h.
4. If you are keen to save more memory, you could move away from using
MemoryChunk and write your own version that maintains the lower 4-bits
for the MemoryContextMethodID and encodes the size in the remaining 60
bits. That might be more trouble than it's worth, however.
5. It would be good to see proxy.c using set_sentinel() and sentinel_ok(),
David
