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. Regards, Amul
