RE: Delay the variable initialization in get_rel_sync_entry

On Friday, December 24, 2021 8:13 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Dec 23, 2021 at 12:54:41PM -0300, Euler Taveira wrote:
> > On Wed, Dec 22, 2021, at 10:11 AM, houzj.fnst@fujitsu.com wrote:
> >> The extra cost could sometimes be noticeable because get_rel_sync_entry is
> a
> >> hot function which is executed for each change. And the 'am_partition' and
> >> 'relkind' are necessary only when we need to rebuild the RelationSyncEntry.
> >>
> >> Here is the perf result for the case when inserted large amounts of data into
> a
> >> un-published table in which case the cost is noticeable.
> >>
> >> --12.83%--pgoutput_change
> >>     |--11.84%--get_rel_sync_entry
> >>     |--4.76%--get_rel_relispartition
> >> |--4.70%--get_rel_relkind
>
> How does the perf balance change once you apply the patch?  Do we have
> anything else that stands out?  Getting rid of this bottleneck is fine
> by itself, but I am wondering if there are more things to worry about
> or not.

Thanks for the response.
Here is the perf result of pgoutput_change after applying the patch.
I didn't notice something else that stand out.

        |--2.99%--pgoutput_change
            |--1.80%--get_rel_sync_entry
                    |--1.56%--hash_search

Also attach complete profiles.

> > Good catch. WFM. Deferring variable initialization close to its first use is
> > good practice.
>
> Yeah, it is usually a good practice to have the declaration within
> the code block that uses it rather than piling everything at the
> beginning of the function.  Being able to see that in profiles is
> annoying, and the change is simple, so I'd like to backpatch it.

+1

> This is a period of vacations for a lot of people, so I'll wait until
> the beginning-ish of January before doing anything.

Thanks, added it to CF.
https://commitfest.postgresql.org/36/3471/

Best regards,
Hou zj