public inbox for [email protected]  

help / color / mirror / Atom feed

From: Sutou Kouhei <[email protected]>
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Make COPY format extendable: Extract COPY TO format implementations
Date: Thu, 14 Aug 2025 15:36:54 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAD21AoBa0Wm3C2H12jaqkvLidP2zEhsC+gf=3w7XiA4LQnvx0g@mail.gmail.com>
References: <[email protected]>
	<CAD21AoB01+FZ_bOR4uOq_x_qBE8on1ASdJWm+UexxOwC1RGD-A@mail.gmail.com>
	<CAD21AoBa0Wm3C2H12jaqkvLidP2zEhsC+gf=3w7XiA4LQnvx0g@mail.gmail.com>

Hi,

In <CAD21AoBa0Wm3C2H12jaqkvLidP2zEhsC+gf=3w7XiA4LQnvx0g@mail.gmail.com>
  "Re: Make COPY format extendable: Extract COPY TO format implementations" on Mon, 28 Jul 2025 22:19:36 -0700,
  Masahiko Sawada <[email protected]> wrote:

> The fields in 1 are mostly static fields, and the fields in 2 and 3
> are likely to be accessed in hot functions during COPY FROM. Would it
> be a good idea to restructure these fields so that we can hide the
> fields in 1 from callback functions and having the fields in 3 in a
> separate format-specific struct that can be accessed via an opaque
> pointer? But could the latter change potentially cause performance
> overheads?

Yes. It changes memory layout (1 continuous memory chunk ->
2 separated memory chunks) and introduces indirect member
accesses (x->y -> x->z->y). They may not have performance
impact but we need to measure it if we want to use this
approach.


BTW, how about the following approach?

copyapi.h:

typedef struct CopyToStateData
{
	/* public members */
	/* ... */
} CopyToStateData;

copyto.c:

typedef struct CopyToStateInternalData
{
	CopyToStateData parent;

	/* private members */
	/* ... */
} CopyToStateInternalData;

We export CopyToStateData only with public members. We don't
export CopyToStateInternalData that has members only for
built-in formats.

CopyToStateInternalData has the same memory layout as
CopyToStateData. So we can use CopyToStateInternalData as
CopyToStateData.

We use CopyToStateData not CopyToStateInternalData in public
API. We cast CopyToStateData to CopyToStateInternalData when
we need to use private members:

static void
CopySendData(CopyToState cstate, const void *databuf, int datasize)
{
	CopyToStateInternal cstate_internal = (CopyToStateInternal) cstate;
	appendBinaryStringInfo(cstate_internal->fe_msgbuf, databuf, datasize);
}

It's still direct member access.


With this approach, we can keep the same memory layout (1
continuous memory chunk) and direct member access. I think
that this approach doesn't have performance impact.

See the attached patch for PoC of this approach.

Drawback: This approach always allocates
CopyToStateInternalData not CopyToStateData. So we need to
allocate needless memories for extensions. But this will
prevent performance regression of built-in formats. Is it
acceptable?


Thanks,
-- 
kou

view thread (125+ messages)  latest in thread

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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Make COPY format extendable: Extract COPY TO format implementations
  In-Reply-To: <[email protected]>

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

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