Re: AIX support - alignment issues

Поиск
Список
Период
Сортировка
От Andres Freund
Тема Re: AIX support - alignment issues
Дата
Msg-id 20220706162737.ph7vfqtaztl2amzh@awork3.anarazel.de
обсуждение исходный текст
Ответ на Re: AIX support - alignment issues  (Robert Haas <robertmhaas@gmail.com>)
Ответы Re: AIX support - alignment issues  (Robert Haas <robertmhaas@gmail.com>)
Список pgsql-hackers
Hi,

On 2022-07-06 11:55:57 -0400, Robert Haas wrote:
> On Sat, Jul 2, 2022 at 2:34 PM Andres Freund <andres@anarazel.de> wrote:
> > I strikes me as a remarkably bad idea to manually try to maintain the correct
> > alignment. Even with the tests added it's still quite manual and requires
> > contorted struct layouts (see e.g. [1]).
> >
> > I think we should either teach our system the correct alignment rules or we
> > should drop AIX support.
>
> I raised this same issue at
> http://postgr.es/m/CA+TgmoaK377MXCWJqEXM3VvKDDC-frNUMKb=7u07TJa59wTAeQ@mail.gmail.com
> and discussion ensued from there. I agree that manually maintaining
> alignment, even with a regression test to help, is a really bad plan.
>
> The rule about columns of type "name" can be relaxed easily enough,
> just by insisting that NAMEDATALEN must be a multiple of 8. As Tom
> also said on this thread, adding such a constraint seems to have no
> real downside. But the problem has a second aspect not related to
> NameData, which is that int64 and double have different alignment
> requirements on that platform. To get out from under that part of it,
> it seems we either need to de-support AIX and any other platforms that
> have such a discrepancy, or else have separate typalign values for
> int64-align vs. double-align.

I think my proposal of introducing a version of double that is marked to be 8
byte aligned should do the trick as well, and doesn't have the problem of
changing the meaning of 'double' references in external headers. In fact, we
already have float8 as a type, so we could just add it there.

We don't currently have a float8 in the catalogs afaics, but I think it'd be
better to not rely on that.

It's not pretty, but still seems a lot better than doing this stuff manually.


> From a theoretical point of view, I think what we're doing now is
> pretty unprincipled. I've always found it a bit surprising that we get
> away with just assuming that a bunch of various different primitive
> data types are all going to have the same alignment requirement. The
> purist in me feels that it would be better to have separate typalign
> values for things that aren't guaranteed to behave the same. However,
> there's a practical difficulty with that approach: if the only
> operating system where this issue occurs in practice is AIX, I feel
> it's going to be pretty hard for us to keep the code that caters to
> this unusual situation working properly. And I'd rather have no code
> for it at all than have code which doesn't really work.

The problem with having a lot more alignment values is that it adds a bunch of
overhead to very performance critical paths. We don't want to add more
branches to att_align_nominal() if we can avoid it.

I guess we can try to introduce TYPALIGN_INT64 and then hide the relevant
branch with an ifdef for the common case of TYPALIGN_INT64 == TYPALIGN_DOUBLE.


I'm fairly certain that we're going to add a lot more 64bit ints to catalogs
in the next few years, so this will become a bigger issue over time...


Outside of the catalogs I still think that we should work towards not aligning
byval values (and instead memcpy-ing the values to deal with alignment
sensitive platforms), so we don't waste so much space. And for catalogs we've
been talking about giving up the struct mapping as well, in the thread about
variable length names. In which case we could the cost of handling more
alignment values wouldn't be incurred as frequently.

Greetings,

Andres Freund



В списке pgsql-hackers по дате отправления:

Предыдущее
От: talk to ben
Дата:
Сообщение: Re: archive modules
Следующее
От: Andres Freund
Дата:
Сообщение: Re: Issue with pg_stat_subscription_stats