On Fri, 2 Dec 2022 at 19:06, Ranier Vilela <ranier.vf@gmail.com> wrote:
>
> Hi, thanks for reviewing this.
>
> Em sex., 2 de dez. de 2022 às 09:24, John Naylor <john.naylor@enterprisedb.com> escreveu:
>>
>>
>> On Thu, Dec 1, 2022 at 8:02 PM Ranier Vilela <ranier.vf@gmail.com> wrote:
>> >
>> > Hi,
>> >
>> > I believe that has room for improving generation node files.
>> >
>> > The patch attached reduced the size of generated files by 27 kbytes.
>> > From 891 kbytes to 864 kbytes.
>> >
>> > About the patch:
>> > 1. Avoid useless attribution when from->field is NULL, once that
>> > the new node is palloc0.
>> >
>> > 2. Avoid useless declaration variable Size, when it is unnecessary.
>>
>> Not useless -- it prevents a multiple evaluation hazard, which this patch introduces.
>
> It's doubting, that patch introduces some hazard here.
> But I think that casting size_t (typedef Size) to size_t is worse and is unnecessary.
> Adjusted in the v1 patch.
>
>>
>> > 3. Optimize comparison functions like memcmp and strcmp, using
>> > a short-cut comparison of the first element.
>>
>> Not sure if the juice is worth the squeeze. Profiling would tell.
>
> This is a cheaper test and IMO can really optimize, avoiding a function call.
>
>>
>> > 4. Switch several copy attributions like COPY_SCALAR_FIELD or COPY_LOCATION_FIELD
>> > by one memcpy call.
>>
>> My first thought is, it would cause code churn.
>
> It's a weak argument.
> Reduced 27k from source code, really worth it.
>
>>
>> > 5. Avoid useless attribution, ignoring the result of pg_strtok when it is unnecessary.
>>
>> Looks worse.
>
> Better to inform the compiler that we really don't need the result.
The patch does not apply on top of HEAD as in [1], please post a rebased patch:
=== Applying patches on top of PostgreSQL commit ID
e351f85418313e97c203c73181757a007dfda6d0 ===
=== applying patch ./v1-optimize_gen_nodes_support.patch
patching file src/backend/nodes/gen_node_support.pl
Hunk #2 succeeded at 680 with fuzz 2.
Hunk #3 FAILED at 709.
...
Hunk #7 succeeded at 844 (offset 42 lines).
1 out of 7 hunks FAILED -- saving rejects to file
src/backend/nodes/gen_node_support.pl.rej
[1] - http://cfbot.cputube.org/patch_41_4034.log
Regards,
Vignesh
