On Wed, Jul 7, 2021 at 9:44 AM Amul Sul <sula...@gmail.com> wrote: > > On Tue, Jul 6, 2021 at 11:06 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > > > Amul Sul <sula...@gmail.com> writes: > > > On Tue, Apr 20, 2021 at 6:59 AM Kyotaro Horiguchi > > > <horikyota....@gmail.com> wrote: > > >> I don't mind RelationGetSmgr(index)->smgr_rnode alone or > > >> &variable->member alone and there's not the previous call to > > >> RelationGetSmgr just above. How about using a temporary variable? > > >> > > >> SMgrRelation srel = RelationGetSmgr(index); > > >> smgrwrite(srel, ...); > > >> log_newpage(srel->..); > > > > > Understood. Used a temporary variable for the place where > > > RelationGetSmgr() calls are placed too close or in a loop. > > > > [ squint... ] Doesn't this risk introducing exactly the sort of > > cache-clobber hazard we're trying to prevent? That is, the above is > > not safe unless you are *entirely* certain that there is not and never > > will be any possibility of a relcache flush before you are done using > > the temporary variable. Otherwise it can become a dangling pointer. > > > > Yeah, there will a hazard, even if we sure right but cannot guarantee future > changes in any subroutine that could get call in between. > > > The point of the static-inline function idea was to be cheap enough > > that it isn't worth worrying about this sort of risky optimization. > > Given that an smgr function is sure to involve some kernel calls, > > I doubt it's worth sweating over an extra test-and-branch beforehand. > > So where I was hoping to get to is that smgr objects are *only* > > referenced by RelationGetSmgr() calls and nobody ever keeps any > > other pointers to them across any non-smgr operations. > > > > Ok, will revert changes added in the previous version, thanks. >
Herewith attached version did the same, thanks. Regards, Amul
v4_Add-RelationGetSmgr-inline-function.patch
Description: Binary data
