Re: Optimizing Node Files Support - Mailing list pgsql-hackers

From Ranier Vilela
Subject Re: Optimizing Node Files Support
Date
Msg-id CAEudQArMiszB2x6F3H3TO+3AM46qKuEdyj_XvsMUky2MG2NNVg@mail.gmail.com
Whole thread Raw
In response to Re: Optimizing Node Files Support  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Optimizing Node Files Support
List pgsql-hackers
Em qua., 4 de jan. de 2023 às 19:39, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
vignesh C <vignesh21@gmail.com> writes:
> The patch does not apply on top of HEAD as in [1], please post a rebased patch:

Yeah.  The way that I'd been thinking of optimizing the copy functions
was more or less as attached: continue to write all the COPY_SCALAR_FIELD
macro calls, but just make them expand to no-ops after an initial memcpy
of the whole node.  This preserves flexibility to do something else while
still getting the benefit of substituting memcpy for retail field copies.
Having said that, I'm not very sure it's worth doing, because I do not
see any major reduction in code size:
I think this option is worse.
By disabling these macros, you lose their use in other areas.
By putting more intelligence into gen_node_support.pl, to either use memcpy or the macros is better, IMO.
In cases of one or two macros, would it be faster than memset?


HEAD:
   text    data     bss     dec     hex filename
  53601       0       0   53601    d161 copyfuncs.o
w/patch:
   text    data     bss     dec     hex filename
  49850       0       0   49850    c2ba copyfuncs.o
I haven't tested it on Linux, but on Windows, there is a significant reduction.

head:
8,114,688 postgres.exe
121.281 copyfuncs.funcs.c

patched:
8,108,544 postgres.exe
95.321 copyfuncs.funcs.c


I've not looked at the generated assembly code, but I suspect that
my compiler is converting the memcpy's into inlined code that's
hardly any smaller than field-by-field assignment.  Also, it's
rather debatable that it'd be faster, especially for node types
that are mostly pointer fields, where the memcpy is going to be
largely wasted effort.
IMO, with many field assignments, memcpy would be more faster.
 

I tend to agree with John that the rest of the changes proposed
in the v1 patch are not useful improvements, especially with
no evidence offered that they'd make the code smaller or faster.
I tried using palloc_object, as you proposed, but several tests failed.
So I suspect that some fields are not filled in correctly.
It would be an advantage to avoid memset in the allocation (palloc0), but I chose to keep it because of the errors.

This way, if we use palloc0, there is no need to set NULL on COPY_STRING_FIELD.

Regarding COPY_POINTER_FIELD, it is wasteful to cast size_t to size_t.

v3 attached.

regards,
Ranier Vilela


                        regards, tom lane

Attachment

pgsql-hackers by date:

Previous
From: Tomas Vondra
Date:
Subject: Re: postgres_fdw: using TABLESAMPLE to collect remote sample
Next
From: Tom Lane
Date:
Subject: Re: Resolve iso-8859-1 type to relevant type instead of text type while bulk update using values