Horton

Re: Reduce TupleHashEntryData struct size by half

Jeff Davis <pgsql@j-davis.com>

View thread View on postgr.es
From: Jeff Davis <pgsql@j-davis.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>, pgsql-hackers@postgresql.org
Date: 2024-11-21T20:37:56Z
Lists: pgsql-hackers

Attachments

New patch series attached.

  * a few cleanup patches, but 0001 and 0004 can affect initial hash
table sizing
  * also pack AggStatePerGroupData (to 10 bytes)
  * put additional data in the same chunk as firstTuple to avoid an
extra pointer an an extra allocation

On Tue, 2024-11-19 at 01:30 +0200, Heikki Linnakangas wrote:
> Sounds like a good idea. Needs some changes to the TupleTableSlotOps 
> interface to avoid the memcpy I presume.

Neither firstTuple nor the additional data have a size known at compile
time, so it can't be represented by a struct. It seems best to keep
firstTuple at the beginning so that it matches the SH_KEY_TYPE, and
then just repalloc() to make room at the end for the additional data,
which avoids the memcpy unless it crosses a power-of-two boundary.

> > 
> Queries that have a only a small number of groups might might benefit
> from storing a plain Datum/isnull array instead of a MinimalTuple.
> That 
> would take more memory when you have a lot of groups though.

The current MinimalTuple representation is pretty wasteful for small
grouping keys, so I don't think it would be hard to beat.

> > Alternatively, MinimalTuple is not very "minimal", and perhaps we
> > can
> > just make it better.
> 
> Yeah. It tries to be compatible with HeapTuple, but perhaps we should
> give up on that and pack it more tightly instead.

From the perspective of HashAgg, that seems worthwhile.

In my current patch set, depending on the grouping key and aggregates,
the chunk size for the combination of the firstTuple with the
additional data can be just above 32 bytes, which pushes the chunk size
to 64 bytes. If we cut down the mintuple overhead a bit, more cases
would fit in 32 bytes. And I think we can: 32 bytes seems reasonable to
hold a lot of common cases where there's a small grouping key and a
small pergroup state.

Regards,
	Jeff Davis

Commits

  1. Comment fixups from 626df47ad9.

  2. Remove 'additional' pointer from TupleHashEntryData.

  3. Add ExecCopySlotMinimalTupleExtra().

  4. Create accessor functions for TupleHashEntry.

  5. TupleHashTable: store additional data along with tuple.

  6. ExecInitAgg: update aggstate->numaggs and ->numtrans earlier.

  7. nodeSetOp.c: missing additionalsize for BuildTupleHashTable().

  8. Remove unused TupleHashTableData->entrysize.

  9. Add missing typedefs.list entry for AggStatePerGroupData.