Reverse the search order in afterTriggerAddEvent().
authorTom Lane <[email protected]>
Thu, 23 Jan 2025 16:08:05 +0000 (11:08 -0500)
committerTom Lane <[email protected]>
Thu, 23 Jan 2025 16:08:05 +0000 (11:08 -0500)
When scanning existing AfterTriggerSharedData records in search
of a match to the event being queued, we were examining the
records from oldest to newest.  But it makes more sense to do
the opposite.  The newest record is likely to be from the current
query, while the oldest is likely to be from some previous command
in the same transaction, which will likely have different details.

There aren't expected to be very many active AfterTriggerSharedData
records at once, so that this change is unlikely to make any
spectacular difference.  Still, having added a nontrivially-expensive
bms_equal call to this loop yesterday, I feel a need to shave cycles
where possible.

Discussion: https://postgr.es/m/4166712.1737583961@sss.pgh.pa.us

src/backend/commands/trigger.c

index 1fa63ab7d0fe59dae912c69d67d6b44cac35316c..711c09a10306f9e150814f0fff64da92a0c1dbd7 100644 (file)
@@ -4104,11 +4104,12 @@ afterTriggerAddEvent(AfterTriggerEventList *events,
 
    /*
     * Try to locate a matching shared-data record already in the chunk. If
-    * none, make a new one.
+    * none, make a new one. The search begins with the most recently added
+    * record, since newer ones are most likely to match.
     */
-   for (newshared = ((AfterTriggerShared) chunk->endptr) - 1;
-        (char *) newshared >= chunk->endfree;
-        newshared--)
+   for (newshared = (AfterTriggerShared) chunk->endfree;
+        (char *) newshared < chunk->endptr;
+        newshared++)
    {
        /* compare fields roughly by probability of them being different */
        if (newshared->ats_tgoid == evtshared->ats_tgoid &&
@@ -4120,8 +4121,9 @@ afterTriggerAddEvent(AfterTriggerEventList *events,
                      evtshared->ats_modifiedcols))
            break;
    }
-   if ((char *) newshared < chunk->endfree)
+   if ((char *) newshared >= chunk->endptr)
    {
+       newshared = ((AfterTriggerShared) chunk->endfree) - 1;
        *newshared = *evtshared;
        /* now we must make a suitably-long-lived copy of the bitmap */
        newshared->ats_modifiedcols = afterTriggerCopyBitmap(evtshared->ats_modifiedcols);