On Mon, Aug 5, 2024 at 4:16 AM Alexander Korotkov <aekorot...@gmail.com> wrote: > I've revised the patchset. First of all, I've re-ordered the patches. > > 0001-0002 (former 0002-0003) > Comprises hash_search_with_hash_value() function and its application > to avoid full hash iteration in InvalidateAttoptCacheCallback() and > TypeCacheTypCallback(). I think this is quite straightforward > optimization without negative side effects. I've revised comments, > commit message and did some code beautification. I'm going to push > this if no objections. > > 0003 (former 0001) > I've revised this patch. I think main concerns expressed in the > thread about this path is that we don't have invalidation mechanism > for relid => typid map. Finally due to oid wraparound same relids > could get reused. That could lead to invalid entries in the map about > existing relids and typeids. This is rather messy, but I don't think > this could cause a material bug. The maps items are used only for > cache invalidation. Extra invalidation doesn't cause a bug. If type > with same relid will be cached, then correspoding map item will be > overridden, so no missing invalidation. However, I see the following > reasons for keeping consistent state of relid => typid map. > > 1) As the main use-case for this optimization is flood of temporary > tables, it would be nice not let relid => typid map bloat in this > case. I see that TypeCacheHash would get bloated, because its entries > are never deleted. However, I would prefer to not get this situation > even worse. > 2) In future we may find some more use-cases for relid => typid map > besides cache invalidation. Keeping that in consistent state could be > advantage then. > > In the attached patch, I'm keeping relid => typid map when > corresponding typentry have either TCFLAGS_HAVE_PG_TYPE_DATA, or > TCFLAGS_OPERATOR_FLAGS, or tupdesc. Thus, when temporary table gets > deleted, we would invalidate the map item. > > It will be also nice to get rid of iteration over all the cached > domain types in TypeCacheRelCallback(). However, this typically > shouldn't be a problem since domain types are less tended to bloat. > Domain types are created manually, unlike composite types which are > automatically created for every temporary table. We will probably > need to optimize this in future, but I don't feel this to be necessary > in present patch. > > I think the revised 0003 requires review.
The rebased remaining patch is attached. ------ Regards, Alexander Korotkov Supabase
v8-0001-Avoid-looping-over-all-type-cache-entries-in-Type.patch
Description: Binary data
