From a8ef30e79b8cfc85d3c5fa393a11b28e304515f5 Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Thu, 16 Oct 2025 13:27:25 +0200 Subject: [PATCH v20251016] Deduplicate entries in ResourceOwner We may track the same resource many times, but the current ResourceOwner does not handle that efficiently. Each reference is a separate entry, which leads to long linear searches in the simple hash table. This adds a simple opportunistic deduplication. Entries get a counter, tracking how many references they represent. It only applies to the "hash" phase, not to the initial array. And when adding an entry to the hash finds an empty slot first, it uses it (instead of continuing to look for a possible match). This means the deduplication is not perfect - there may be multiple entries for the same value. But in practice it seems like a good trade off. It won't affect cases with no duplicates, and it will help cases with many of them. The code also continues to grow the hash table as before. In some cases it might be enough to deduplicate the contents. But it's hard to say in advance, and it's not much cheaper as it still requires walking the current hash table. So there's a risk of attempting deduplication, only to find the hash still needs to be enlarged, doubling the cost. So we just grow the hash table. In the worst case the memory usage is not worse than without deduplication. If deduplication is effective, the hash table grows later or not at all. --- src/backend/utils/resowner/resowner.c | 55 ++++++++++++++++++++++++--- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c index fca84ded6dd..27127c99e40 100644 --- a/src/backend/utils/resowner/resowner.c +++ b/src/backend/utils/resowner/resowner.c @@ -60,10 +60,15 @@ * * All objects managed by this code are required to fit into a Datum, * which is fine since they are generally pointers or integers. + * + * We may see items multiple times (e.g. tuple descriptors when creating + * multiple tuple slots). To handle these cases efficiently we deduplicate + * the entries, and track the number of occurrences. */ typedef struct ResourceElem { Datum item; + uint32 count; /* number of occurrences */ const ResourceOwnerDesc *kind; /* NULL indicates a free hash table slot */ } ResourceElem; @@ -198,7 +203,8 @@ static ResourceReleaseCallbackItem *ResourceRelease_callbacks = NULL; /* Internal routines */ static inline uint32 hash_resource_elem(Datum value, const ResourceOwnerDesc *kind); static void ResourceOwnerAddToHash(ResourceOwner owner, Datum value, - const ResourceOwnerDesc *kind); + const ResourceOwnerDesc *kind, + uint32 count); static int resource_priority_cmp(const void *a, const void *b); static void ResourceOwnerSort(ResourceOwner owner); static void ResourceOwnerReleaseAll(ResourceOwner owner, @@ -239,7 +245,8 @@ hash_resource_elem(Datum value, const ResourceOwnerDesc *kind) * Adds 'value' of given 'kind' to the ResourceOwner's hash table */ static void -ResourceOwnerAddToHash(ResourceOwner owner, Datum value, const ResourceOwnerDesc *kind) +ResourceOwnerAddToHash(ResourceOwner owner, Datum value, const ResourceOwnerDesc *kind, + uint32 count) { uint32 mask = owner->capacity - 1; uint32 idx; @@ -250,12 +257,21 @@ ResourceOwnerAddToHash(ResourceOwner owner, Datum value, const ResourceOwnerDesc idx = hash_resource_elem(value, kind) & mask; for (;;) { + /* found an exact match - just increment the counter */ + if ((owner->hash[idx].kind == kind) && + (owner->hash[idx].item == value)) + { + owner->hash[idx].count += count; + return; + } + if (owner->hash[idx].kind == NULL) break; /* found a free slot */ idx = (idx + 1) & mask; } owner->hash[idx].item = value; owner->hash[idx].kind = kind; + owner->hash[idx].count = count; owner->nhash++; } @@ -377,6 +393,7 @@ ResourceOwnerReleaseAll(ResourceOwner owner, ResourceReleasePhase phase, uint32 idx = nitems - 1; Datum value = items[idx].item; const ResourceOwnerDesc *kind = items[idx].kind; + uint32 count = items[idx].count; if (kind->release_phase > phase) break; @@ -392,7 +409,14 @@ ResourceOwnerReleaseAll(ResourceOwner owner, ResourceReleasePhase phase, elog(WARNING, "resource was not closed: %s", res_str); pfree(res_str); } - kind->ReleaseResource(value); + + /* invoke the release callback for each ocurrence */ + while (count > 0) + { + kind->ReleaseResource(value); + count--; + } + nitems--; } if (owner->nhash == 0) @@ -496,7 +520,8 @@ ResourceOwnerEnlarge(ResourceOwner owner) for (i = 0; i < oldcap; i++) { if (oldhash[i].kind != NULL) - ResourceOwnerAddToHash(owner, oldhash[i].item, oldhash[i].kind); + ResourceOwnerAddToHash(owner, oldhash[i].item, oldhash[i].kind, + oldhash[i].count); } /* And release old hash table. */ @@ -506,7 +531,8 @@ ResourceOwnerEnlarge(ResourceOwner owner) /* Move items from the array to the hash */ for (int i = 0; i < owner->narr; i++) - ResourceOwnerAddToHash(owner, owner->arr[i].item, owner->arr[i].kind); + ResourceOwnerAddToHash(owner, owner->arr[i].item, owner->arr[i].kind, + owner->arr[i].count); owner->narr = 0; Assert(owner->nhash <= owner->grow_at); @@ -543,6 +569,7 @@ ResourceOwnerRemember(ResourceOwner owner, Datum value, const ResourceOwnerDesc idx = owner->narr; owner->arr[idx].item = value; owner->arr[idx].kind = kind; + owner->arr[idx].count = 1; owner->narr++; } @@ -597,8 +624,17 @@ ResourceOwnerForget(ResourceOwner owner, Datum value, const ResourceOwnerDesc *k if (owner->hash[idx].item == value && owner->hash[idx].kind == kind) { + /* an entry representing multiple items, just decrement */ + if (owner->hash[idx].count > 1) + { + owner->hash[idx].count--; + return; + } + + /* remove the entry entirely */ owner->hash[idx].item = (Datum) 0; owner->hash[idx].kind = NULL; + owner->hash[idx].count = 0; owner->nhash--; #ifdef RESOWNER_STATS @@ -847,12 +883,19 @@ ResourceOwnerReleaseAllOfKind(ResourceOwner owner, const ResourceOwnerDesc *kind if (owner->hash[i].kind == kind) { Datum value = owner->hash[i].item; + uint32 count = owner->hash[i].count; owner->hash[i].item = (Datum) 0; owner->hash[i].kind = NULL; + owner->hash[i].count = 0; owner->nhash--; - kind->ReleaseResource(value); + /* invoke the release callback once for each entry */ + while (count > 0) + { + kind->ReleaseResource(value); + count--; + } } } owner->releasing = false; -- 2.51.0