Re: Removing alignment padding for byval types

On Thu, Oct 31, 2019 at 11:48:21AM -0700, Andres Freund wrote:
>Hi,
>
>We currently align byval types such as int4/8, float4/8, timestamp *,
>date etc, even though we mostly don't need to. When tuples are deformed,
>all byval types are copied out from the tuple data into the
>corresponding Datum array, therefore the original alignment in the tuple
>data doesn't matter. This is different from byref types, where the
>Datum formed will often be a pointer into the tuple data.
>
>While there are some older systems where it could be a bit slower to
>copy data out from unaligned positions into the datum array, this is
>more than bought back by the next point:
>
>
>The fact that these types are aligned has substantial costs:
>
>For one, we often waste substantial amounts of space inside tables with
>alignment padding. It's not uncommon to see about 30% or more of space
>wasted (especially when taking alignment of the first column into
>account).
>
>For another, and this I think is less obvious, we actually waste
>substantial amounts of CPU maintaining the alignment. This is primarily
>the case because we have to perform to align the pointer to the next
>field during tuple [de]forming. Those instructions [1] have to be
>executed taking time, but what's worse, they also reduce the ability of
>out-of-order execution to hide latencies. There's a hard dependency on
>knowing the offset to the next column to be able to continue with the
>next column.
>

Right. Reducing this overhead was one of the goals to allow "logical
ordering" of columns in a table (while arbitrarily reordering the
physical ones), but that patch got out of hand pretty quickly. Also,
it'd still keep some of the overhead, because it was not removing the
alignment/padding entirely.

>
>There's two reasons why we can't just set the alignment for these types
>to 'c'.
>1) pg_upgrade, for fairly obvious reasons
>2) We map catalog table rows to structs, in a *lot* of places.
>
>
>It seems to me that, despite the above, it's still worth trying to
>improve upon the current state, to benefit from reduced space and CPU
>usage.
>
>As it turns out we already separate out the alignment for a type, and a
>column, between pg_type.typalign and pg_attribute.attalign. One way to
>tackle this would be to allow to specify, for byval types only, at
>column creation time whether a column uses a 'struct-mappable' alignment
>or not. When set, set attalign to pg_type.typalign for alignment, when
>not, to 'c'. By changing pg_dump in binary upgrade mode to emit the
>necessary options, and by adding such options during bki processing,
>we'd solve 1) and 2), but otherwise gain the benefits.
>
>Alternatively we could declare such a propert on the table level, but
>that seems more restrictive, without a corresponding upside.
>

I don't know, but it seems entirely sufficient specifying this at the
table level, no? What would be the use case for removing padding for
only some of the columns? I don't see the use case for that.

>
>It's possible that we should do something related with a few varlena
>datatypes. We currently use intalign for types like text, json, and as
>far as I can tell that does not make all that much sense. They're not
>struct mappable *anyway* (and if they were, they'd need to be 8 byte
>aligned on common platforms, __alignof__(void*) is 8). We'd have to take
>a bit of care to treat the varlena header as unaligned - but we need to
>do so anyway, because of 1byte varlenas. Short varlenas seems to make it
>less crucial to pursue this, as the average datum that'd benefit is long
>enough to make padding a non-issue. So I don't think it'd make sense to
>tackle this project at the same time.
>

Not sure, but how come it's not failing on the picky platforms, then? On
x86 it's probably OK because it's pretty permissive, but I'd expect some
platforms (s390, parisc, itanium, powerpc, ...) to be much pickier.

>
>To fully benefit from the increased tuple deforming speed, it might be
>beneficial to branch very early between two different versions within
>slot_deform_heap_tuple, having determined whether there's any byval
>columns with alignment requirements at slot creation /
>ExecSetSlotDescriptor() time (or even set a different callback
>getsomeattrs callback, but that's a bit more complicated).
>
>
>Thoughts?
>

Seems reasonable. I certainly agree this padding is pretty annoying, so
if we can get rid of it without causing issues, that'd be nice.

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services