Re: [Patch] Optimize dropping of relation buffers using dlist

At Fri, 02 Oct 2020 11:44:46 +0900 (JST), Kyotaro Horiguchi  wrote in 
> At Thu, 1 Oct 2020 12:55:34 +0000, "k.jamison@fujitsu.com"  wrote in 
> - *		XXX currently it sequentially searches the buffer pool, should be
> - *		changed to more clever ways of searching.  However, this routine
> - *		is used only in code paths that aren't very performance-critical,
> - *		and we shouldn't slow down the hot paths to make it faster ...
> + *		XXX The relation might have extended before this, so this path is
> + *		only optimized during recovery when we can get a reliable cached
> + *		value of blocks for specified relation.  In addition, it is safe to
> + *		do this since there are no other processes but the startup process
> + *		that changes the relation size during recovery.  Otherwise, or if
> + *		not in recovery, proceed to usual invalidation process, where it
> + *		sequentially searches the buffer pool.
> 
> This should no longer be a XXX comment.  It seems to me somewhat
> describing too-detailed at this function's level. How about something
> like the follwoing? (excpet its syntax, or phrasing:p)
> 
> ===
> If the expected maximum number of buffers to drop is small enough
> compared to NBuffers, individual buffers are located by
> BufTableLookup. Otherwise we scan through all buffers. Snnce we
> mustn't leave a buffer behind, we take the latter way unless the
> number is not reliably identified.  See smgrcachednblocks() for
> details.
> ===

The second to last phrase is inversed, and some typos are found. FWIW
this is the revised version.

====
If we are expected to drop buffers less enough, we locate individual
buffers using BufTableLookup. Otherwise we scan through all
buffers. Since we mustn't leave a buffer behind, we take the latter
way unless the sizes of all the involved forks are known to be
accurte. See smgrcachednblocks() for details.
====

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center