From: | Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | vignesh C <vignesh21(at)gmail(dot)com>, John Naylor <john(dot)naylor(at)enterprisedb(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Optimizing Node Files Support |
Date: | 2023-01-06 14:49:34 |
Message-ID: | CAEudQArMiszB2x6F3H3TO+3AM46qKuEdyj_XvsMUky2MG2NNVg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Em qua., 4 de jan. de 2023 às 19:39, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> escreveu:
> vignesh C <vignesh21(at)gmail(dot)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 | Content-Type | Size |
---|---|---|
v3-optimize_gen_node_support.patch | application/octet-stream | 4.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2023-01-06 14:58:29 | Re: Resolve UNKNOWN type to relevant type instead of text type while bulk update using values |
Previous Message | Tomas Vondra | 2023-01-06 14:40:36 | Re: postgres_fdw: using TABLESAMPLE to collect remote sample |