From: | "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Euler Taveira <euler(at)eulerto(dot)com> |
Subject: | RE: Delay the variable initialization in get_rel_sync_entry |
Date: | 2021-12-24 13:27:26 |
Message-ID: | OS0PR01MB57163629CCA1DA6374C54DF6947F9@OS0PR01MB5716.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Friday, December 24, 2021 8:13 AM Michael Paquier <michael(at)paquier(dot)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(dot)fnst(at)fujitsu(dot)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
Attachment | Content-Type | Size |
---|---|---|
perf.zip | application/x-zip-compressed | 68.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Justin Pryzby | 2021-12-24 13:49:46 | Re: add recovery, backup, archive, streaming etc. activity messages to server logs along with ps display |
Previous Message | Ashutosh Bapat | 2021-12-24 13:12:00 | Re: Add new function to convert 32-bit XID to 64-bit |