pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: hlinnaka <hlinnaka@iki.fi>
Subject: Re: SerializeParamList vs machines with strict alignment
Date: Mon, 10 Sep 2018 09:35:00 -0400
Message-ID: <8352.1536586500@sss.pgh.pa.us> (raw)
In-Reply-To: <CAA4eK1JfEc=cqiUiRvUcHYCf=PVEwM_bZ_QiOMHpqGpUMdY8gA@mail.gmail.com>
References: <11629.1536550032@sss.pgh.pa.us>
	<CAA4eK1JfEc=cqiUiRvUcHYCf=PVEwM_bZ_QiOMHpqGpUMdY8gA@mail.gmail.com>

Amit Kapila <amit.kapila16@gmail.com> writes:
> On Mon, Sep 10, 2018 at 8:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> In particular, SerializeParamList does this:
>> 
>> /* Write flags. */
>> memcpy(*start_address, &prm->pflags, sizeof(uint16));
>> *start_address += sizeof(uint16);
>> 
>> immediately followed by this:
>> 
>> datumSerialize(prm->value, prm->isnull, typByVal, typLen,
>> start_address);

> datumSerialize does this:

> memcpy(*start_address, &header, sizeof(int));
> *start_address += sizeof(int);

> before calling EOH_flatten_into, so it seems to me it should be 4-byte aligned.

But that doesn't undo the fact that you're now on an odd halfword
boundary.  In the case I observed, EA_flatten_into was trying to
store an int32 through a pointer whose hex value ended in E, which
is explained by the "+= sizeof(uint16)".

> Yeah, I think as suggested by you, start_address should be maxaligned.

A localized fix would be for datumSerialize to temporarily palloc the
space for EOH_flatten_into to write into, and then it could memcpy
that to whatever random address it'd been passed.  Seems kind of
inefficient though, especially since you'd likely have to do the same
thing on the receiving side.

A larger issue is that even on machines where misalignment is permitted,
memcpy'ing to/from misaligned addresses is rather inefficient.

I think it'd be better to redesign the whole thing so that every field
is maxaligned, and you can e.g. just store and retrieve integer fields
as integers without a memcpy.  The "wasted" padding space doesn't seem
very significant given the short-lived nature of the serialized data.

However, that's not exactly a trivial change.  Probably the best way
forward is to adopt the extra-palloc solution as a back-patchable
fix, and then work on a more performant solution in HEAD only.

			regards, tom lane




view thread (26+ messages)  latest in thread

Message-ID: <8352.1536586500@sss.pgh.pa.us>
Permalink:  ../8352.1536586500@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/8352.1536586500@sss.pgh.pa.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, amit.kapila16@gmail.com, pgsql-hackers@lists.postgresql.org, hlinnaka@iki.fi
  Subject: Re: SerializeParamList vs machines with strict alignment
  In-Reply-To: <8352.1536586500@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox