> On Oct 17, 2025, at 01:46, Tomas Vondra <[email protected]> wrote:
>
> --
> Tomas Vondra<v20251016-0001-Deduplicate-entries-in-ResourceOwner.patch>
Nice patch! I eyeball reviewed the patch, only got a few small comments:
1
```
@@ -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;
}
```
I think this is the “trade-off” you mention in your email: if a free slot found
earlier, then still gets duplicated entries. I have no concern to this
“trade-off”, but please add a comment here, otherwise future readers may be
confused, and might potentially consider that were a bug, without reading your
original design email.
2
```
typedef struct ResourceElem
{
Datum item;
+ uint32 count; /* number of
occurrences */
```
Nit suggestion. People usually name this type of count as “refcnt”, which looks
more meaningful. But I don’t have a strong opinion here. I am absolutely fine
if you don’t want to change.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
