> On Oct 17, 2025, at 01:46, Tomas Vondra <tomas@vondra.me> 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,
andmight 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
strongopinion 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/
