Re: PG15 beta1 sort performance regression due to Generation context change

Fr, 27/05/2022 в 10:51 -0400, Tom Lane writes:
> Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru> writes:
> > В Вт, 24/05/2022 в 17:39 -0700, Andres Freund пишет:
> > > A variation on your patch would be to only store the offset to the block
> > > header - that should always fit into 32bit (huge allocations being their own
> > > block, which is why this wouldn't work for storing an offset to the
> > > context).
> > I'm +1 for this.
>
> Given David's results in the preceding message, I don't think I am.

But David did the opposite: he removed pointer to block and remain
pointer to context. Then code have to do bsearch to find actual block.

> A scheme like this would add more arithmetic and at least one more
> indirection to GetMemoryChunkContext(), and we already know that
> adding even a test-and-branch there has measurable cost. (I wonder
> if using unlikely() on the test would help? But it's not unlikely
> in a generation-context-heavy use case.)

Well, it should be tested.

> There would also be a good
> deal of complication and ensuing slowdown created by the need for
> oversize chunks to be a completely different kind of animal with a
> different header.

Why? encoded_size could handle both small sizes and larges sizes
given actual (not requested) allocation size is rounded to page size.
There's no need to different chunk header.

> I'm also not very happy about this:
>
> > And with this change every memory context kind can have same header:
>
> IMO that's a bug not a feature. It puts significant constraints on how
> context types can be designed.

Nothing prevents to add additional data before common header.

regards

Yura