RE: ResourceOwner refactoring - Mailing list pgsql-hackers

From kuroda.hayato@fujitsu.com
Subject RE: ResourceOwner refactoring
Date
Msg-id OSBPR01MB3157BD946853F6FC1FD213B0F5A80@OSBPR01MB3157.jpnprd01.prod.outlook.com
Whole thread Raw
In response to RE: ResourceOwner refactoring  ("kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>)
Responses Re: ResourceOwner refactoring  (Heikki Linnakangas <hlinnaka@iki.fi>)
List pgsql-hackers
Hi,

I put some comments.

Throughout, some components don’t have helper functions.
(e.g. catcache has ResOwnerReleaseCatCache, but tupdesc doesn't.)
I think it should be unified.

[catcache.c]
> +/* support for catcache refcount management */
> +static inline void
> +ResourceOwnerEnlargeCatCacheRefs(ResourceOwner owner)
> +{
> +       ResourceOwnerEnlarge(owner);
> +}

This function is not needed.

[syscache.c]
> -static CatCache *SysCache[SysCacheSize];
> + CatCache *SysCache[SysCacheSize];

Is it right? Compilation is done even if this variable is static...

[fd.c, dsm.c]
In these files helper functions are implemented as the define directive.
Could you explain the reason? For the performance?

> Previously, this was OK, because resources AAA and BBB were kept in 
> separate arrays. But after this patch, it's not guaranteed that the 
> ResourceOwnerRememberBBB() will find an empty slot.
>
> I don't think we do that currently, but to be sure, I'm planning to grep 
> ResourceOwnerRemember and look at each call carefullly. And perhaps we 
> can add an assertion for this, although I'm not sure where.

Indeed, but I think this line works well, isn't it?

>     Assert(owner->narr < RESOWNER_ARRAY_SIZE);

Best regards,
Hayato Kuroda
FUJITSU LIMITED


pgsql-hackers by date:

Previous
From: Павел Еремин
Date:
Subject: profiling
Next
From: Fujii Masao
Date:
Subject: Re: [PATCH] postgres_fdw connection caching - cause remote sessions linger till the local session exit