Hi,

When reviewing some logical replication patches. I noticed that in
function get_rel_sync_entry() we always invoke get_rel_relispartition() 
and get_rel_relkind() at the beginning which could cause unnecessary
cache access.

---
get_rel_sync_entry(PGOutputData *data, Oid relid)
{
        RelationSyncEntry *entry;
        bool            am_partition = get_rel_relispartition(relid);
        char            relkind = get_rel_relkind(relid);
---

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

So, I think it would be better if we do the initialization only when
RelationSyncEntry in invalid.

Attach a small patch which delay the initialization.

Thoughts ?

Best regards,
Hou zj

Attachment: 0001-Delay-the-variable-initialization-in-get_rel_sync_en.patch
Description: 0001-Delay-the-variable-initialization-in-get_rel_sync_en.patch

Reply via email to