Re: Reducing the chunk header sizes on all memory context types - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Reducing the chunk header sizes on all memory context types
Date
Msg-id 3666758.1662563307@sss.pgh.pa.us
Whole thread Raw
In response to Re: Reducing the chunk header sizes on all memory context types  (David Rowley <dgrowleyml@gmail.com>)
Responses Re: Reducing the chunk header sizes on all memory context types
Re: Reducing the chunk header sizes on all memory context types
List pgsql-hackers
David Rowley <dgrowleyml@gmail.com> writes:
> Looks like my analysis wasn't that good in nodeWindowAgg.c.  The
> reason it's crashing is due to int2int4_sum() returning
> Int64GetDatumFast(transdata->sum).

Ugh.  I thought for a bit about whether we could define that as wrong,
but it's returning a portion of its input, which seems kosher enough
(not much different from array subscripting, for instance).

> I'll need to think about how best to fix this. In the meantime, I
> think the other 32-bit animals are probably not going to like this
> either :-(

Yeah.  The basic problem here is that we've greatly reduced the amount
of redundancy in chunk headers.

Perhaps we need to proceed about like this:

1. Check the provided pointer is non-null and maxaligned
(if not, return false).

2. Pull out the mcxt type bits and check that they match the
type of the provided context.

3. If 1 and 2 pass, it's safe enough to call a context-specific
check routine.

4. For aset.c, I'd be inclined to have it compute the AllocBlock
address implied by the putative chunk header, and then run through
the context's alloc blocks and see if any of them match.  If we
do find a match, and the chunk address is within the allocated
length of the block, call it good.  Probably the same could be done
for the other two methods.

Step 4 is annoyingly expensive, but perhaps not too awful given
the way we step up alloc block sizes.  We should make sure that
any context we want to use MemoryContextContains with is allowed
to let its blocks grow large, so that there can't be too many
of them.

I don't see a way to do better if we're afraid to dereference
the computed AllocBlock address.

BTW, if we do it this way, what we'd actually be guaranteeing
is that the address is within some alloc block belonging to
the context; it wouldn't quite prove that the address corresponds
to a currently-allocated chunk.  That'd be good enough probably
for the important use-cases.  In particular it'd be 100% correct
at rejecting chunks of other contexts and chunks gotten from
raw malloc().

            regards, tom lane



pgsql-hackers by date:

Previous
From: Jeff Davis
Date:
Subject: Re: pg_auth_members.grantor is bunk
Next
From: "Jonathan S. Katz"
Date:
Subject: Re: PostgreSQL 15 Beta 4 release announcement draft