Re: Speedup truncations of temporary relation forks

On 2025/06/02 18:06, Yura Sokolov wrote:
> 31.05.2025 17:23, Daniil Davydov пишет:
>> Hi,
>>
>> On Sat, May 31, 2025 at 7:41 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>>>
>>> Here are a few review comments on the patch:
>>>
>>> +               for (j = 0; j < nforks; j++)
>>>                  {
>>> -                       InvalidateLocalBuffer(bufHdr, true);
>>> +                       if ((buf_state & BM_TAG_VALID) &&
>>> +                               BufTagGetForkNum(&bufHdr->tag) == forkNum[j] &&
>>> +                               bufHdr->tag.blockNum >= firstDelBlock[j])
>>> +                       {
>>> +                               InvalidateLocalBuffer(bufHdr, true);
>>> +                       }
>>>
>>> It looks like the "buf_state & BM_TAG_VALID" check can be moved
>>> outside the loop, along with the BufTagMatchesRelFileLocator() check.
>>> That would avoid unnecessary looping.
>>>
>>> Also, should we add a "break" right after calling InvalidateLocalBuffer()?
>>> Since the buffer has already been invalidated, continuing the loop
>>> may not be necessary.
>>
>> Thanks for the review! I'll fix both remarks. Please see the v2 patch.
> 
> Excuse me for disturbing...
> Wouldn't it be more efficient if we change search data structure for local
> buffers?
> Instead of hash table for RelFileLocator+forknum+BlockNumber it could be
> hash table for RelFileLocator+forknum + included datastructure for
> BlockNumber (hash table or radix tree). Then there will no be need to
> iterate whole local buffers for each relation.
> 
> Given local buffers are not target for concurrent access, both hash tables
> could be implemented using simplehash. It will compensate two-stage lookup,
> given dynahash is much slower than simplehash.

I'm not sure how much this approach improves performance, but it might
be worth trying. If it proves effective, it would also make sense to
apply it to shared buffers, since it's typically larger and takes longer
to scan than local buffers.

Regardless, I think we should go ahead and apply the current patch.
If your approach shows a noticeable performance gain, we can consider
adding it as a follow-up.

Regards,

-- 
Fujii Masao
NTT DATA Japan Corporation