]> granicus.if.org Git - postgresql/commitdiff
Improve performance of find_all_inheritors()
authorTeodor Sigaev <teodor@sigaev.ru>
Mon, 27 Mar 2017 16:07:48 +0000 (19:07 +0300)
committerTeodor Sigaev <teodor@sigaev.ru>
Mon, 27 Mar 2017 16:07:48 +0000 (19:07 +0300)
Previous coding uses three nested loops which obviously were a pain for
large number of table's children. Patch replaces inner loop with
a hashmap.

Author: Aleksander Alekseev
Reviewed-by: me
https://commitfest.postgresql.org/13/1058/

src/backend/catalog/pg_inherits.c

index 9bd2cd16f678f087ba809fd5ad3049a19128528f..827ad2a850e35eee24cac441f4d9ea8f655860d5 100644 (file)
 #include "utils/fmgroids.h"
 #include "utils/syscache.h"
 #include "utils/tqual.h"
+#include "utils/memutils.h"
 
+/*
+ * Entry of a hash table used in find_all_inheritors. See below.
+ */
+typedef struct SeenRelsEntry
+{
+       Oid                      rel_id;                        /* relation oid */
+       ListCell        *numparents_cell;       /* corresponding list cell */
+} SeenRelsEntry;
 
 /*
  * find_inheritance_children
@@ -157,10 +166,33 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode)
 List *
 find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents)
 {
+       /* hash table for O(1) rel_oid -> rel_numparents cell lookup */
+       HTAB               *seen_rels;
+       HASHCTL                 ctl;
+       MemoryContext   new_ctx;
        List       *rels_list,
                           *rel_numparents;
        ListCell   *l;
 
+       /*
+        * We need a separate memory context for a hash table. This is because
+        * hash table is used only in this procedure. To free a memory we need to
+        * call hash_destroy which is just a wrapper around MemoryContextDelete.
+        */
+       new_ctx = AllocSetContextCreate(CurrentMemoryContext,
+                                                                       "FindAllInheritorsSeenRelsContext",
+                                                                       ALLOCSET_DEFAULT_SIZES);
+
+       memset(&ctl, 0, sizeof(ctl));
+       ctl.keysize = sizeof(Oid);
+       ctl.entrysize = sizeof(SeenRelsEntry);
+       ctl.hcxt = new_ctx;
+
+       seen_rels = hash_create(
+               "find_all_inheritors temporary table",
+               32, /* start small and extend */
+               &ctl, HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
+
        /*
         * We build a list starting with the given rel and adding all direct and
         * indirect children.  We can use a single list as both the record of
@@ -190,26 +222,21 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents)
                foreach(lc, currentchildren)
                {
                        Oid                     child_oid = lfirst_oid(lc);
-                       bool            found = false;
-                       ListCell   *lo;
-                       ListCell   *li;
+                       bool                    found;
+                       SeenRelsEntry   *hash_entry;
 
-                       /* if the rel is already there, bump number-of-parents counter */
-                       forboth(lo, rels_list, li, rel_numparents)
+                       hash_entry = hash_search(seen_rels, &child_oid, HASH_ENTER, &found);
+                       if (found)
                        {
-                               if (lfirst_oid(lo) == child_oid)
-                               {
-                                       lfirst_int(li)++;
-                                       found = true;
-                                       break;
-                               }
+                               /* if the rel is already there, bump number-of-parents counter */
+                               lfirst_int(hash_entry->numparents_cell)++;
                        }
-
-                       /* if it's not there, add it. expect 1 parent, initially. */
-                       if (!found)
+                       else
                        {
+                               /* if it's not there, add it. expect 1 parent, initially. */
                                rels_list = lappend_oid(rels_list, child_oid);
                                rel_numparents = lappend_int(rel_numparents, 1);
+                               hash_entry->numparents_cell = rel_numparents->tail;
                        }
                }
        }
@@ -218,6 +245,9 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents)
                *numparents = rel_numparents;
        else
                list_free(rel_numparents);
+
+       hash_destroy(seen_rels);
+
        return rels_list;
 }