Message-ID: <45e3e300-f06a-42a0-8f14-acc6092edacc@enterprisedb.com>
Date: Sat, 2 Mar 2024 23:51:49 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: BitmapHeapScan streaming read user and prelim refactoring
Content-Language: en-US
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Andres Freund <andres@anarazel.de>,
 Pg Hackers <pgsql-hackers@postgresql.org>,
 Thomas Munro <thomas.munro@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>,
 Nazir Bilal Yavuz <byavuz81@gmail.com>
References: <CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA@mail.gmail.com>
 <20240227015028.knohvy3spaqwk7lf@liskov>
 <20240227142230.nu3ytvcjwouvczlt@liskov>
 <40c213cc-6a15-4e2c-9e56-dae344d52a73@enterprisedb.com>
 <CAAKRu_ar_C+RCMV_LrCL10ccWA0n-0rZcxfhL67hXFqREpPRfg@mail.gmail.com>
 <4b751bc5-446f-4357-a1e1-827585bf22b0@enterprisedb.com>
 <d0a5199d-f195-4378-8ac1-fa2534192041@enterprisedb.com>
 <CAAKRu_bzhunY_Hha7i6vMRfQkzgmU5YPtgrrFZBw7aF7ysmrJQ@mail.gmail.com>
 <45bed4f3-a5bf-4a34-b544-7e751bd437e1@enterprisedb.com>
 <CAAKRu_aAwbm-jCJimnce2oVpgpU1jUpRaUbAhPQ7t8WhHXzK+g@mail.gmail.com>
 <91090d58-7d3f-4447-9425-f24ba66e292a@enterprisedb.com>
 <CAAKRu_Y06p8pUPsZHP5PTxO0U3-511PcJPB9VWYfZ9MqvVsiTA@mail.gmail.com>
 <8de6f6d7-9b9d-47a4-98cc-84bf31a314db@enterprisedb.com>
 <CAAKRu_YX6RjjNizWm08H8HdFVCDF9FmNoB5h2TpuvqXftbsALw@mail.gmail.com>
 <CAAKRu_Yos+65UrYcRtF3GQARfvrd8qgp5dO4p=1JzSaP=7cqog@mail.gmail.com>
 <CAAKRu_YHAk2z22Puast888f4izfeezfEUB0wWC0MVWoqh=51vQ@mail.gmail.com>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
In-Reply-To: <CAAKRu_YHAk2z22Puast888f4izfeezfEUB0wWC0MVWoqh=51vQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/45e3e300-f06a-42a0-8f14-acc6092edacc%40enterprisedb.com>
Precedence: bulk

On 3/2/24 23:11, Melanie Plageman wrote:
> On Fri, Mar 1, 2024 at 2:31 PM Melanie Plageman
> <melanieplageman@gmail.com> wrote:
>>
>> ...
>>
>> Hold the phone on this one. I realized why I moved
>> BitmapAdjustPrefetchIterator after table_scan_bitmap_next_block() in
>> the first place -- master calls BitmapAdjustPrefetchIterator after the
>> tbm_iterate() for the current block -- otherwise with eic = 1, it
>> considers the prefetch iterator behind the current block iterator. I'm
>> going to go through and figure out what order this must be done in and
>> fix it.
> 
> So, I investigated this further, and, as far as I can tell, for
> parallel bitmapheapscan the timing around when workers decrement
> prefetch_pages causes the performance differences with patch 0010
> applied. It makes very little sense to me, but some of the queries I
> borrowed from your regression examples are up to 30% slower when this
> code from BitmapAdjustPrefetchIterator() is after
> table_scan_bitmap_next_block() instead of before it.
> 
>         SpinLockAcquire(&pstate->mutex);
>         if (pstate->prefetch_pages > 0)
>             pstate->prefetch_pages--;
>         SpinLockRelease(&pstate->mutex);
> 
> I did some stracing and did see much more time spent in futex/wait
> with this code after the call to table_scan_bitmap_next_block() vs
> before it. (table_scan_bitmap_next_block()) calls ReadBuffer()).
> 
> In my branch, I've now moved only the parallel prefetch_pages-- code
> to before table_scan_bitmap_next_block().
> https://github.com/melanieplageman/postgres/tree/bhs_pgsr
> I'd be interested to know if you see the regressions go away with 0010
> applied (commit message "Make table_scan_bitmap_next_block() async
> friendly" and sha bfdcbfee7be8e2c461).
> 

I'll give this a try once the runs with MAX_BUFFERS_PER_TRANSFER=1
complete. But it seems really bizarre that simply moving this code a
little bit would cause such a regression ...

regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company