public inbox for [email protected]  
help / color / mirror / Atom feed
From: Tomas Vondra <[email protected]>
To: Melanie Plageman <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Pg Hackers <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: Nazir Bilal Yavuz <[email protected]>
Subject: Re: BitmapHeapScan streaming read user and prelim refactoring
Date: Thu, 29 Feb 2024 13:54:05 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAAKRu_aAwbm-jCJimnce2oVpgpU1jUpRaUbAhPQ7t8WhHXzK+g@mail.gmail.com>
References: <CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA@mail.gmail.com>
	<[email protected]>
	<CAAKRu_bQ9a2dB42Tz-mrsR+2MuG-ojRmqbX-x-TC08Uo_RunNw@mail.gmail.com>
	<20240216173559.xiy5xcl5dqmsprns@liskov>
	<20240227015028.knohvy3spaqwk7lf@liskov>
	<20240227142230.nu3ytvcjwouvczlt@liskov>
	<[email protected]>
	<CAAKRu_ar_C+RCMV_LrCL10ccWA0n-0rZcxfhL67hXFqREpPRfg@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAAKRu_bzhunY_Hha7i6vMRfQkzgmU5YPtgrrFZBw7aF7ysmrJQ@mail.gmail.com>
	<[email protected]>
	<CAAKRu_aAwbm-jCJimnce2oVpgpU1jUpRaUbAhPQ7t8WhHXzK+g@mail.gmail.com>



On 2/29/24 00:40, Melanie Plageman wrote:
> On Wed, Feb 28, 2024 at 6:17 PM Tomas Vondra
> <[email protected]> wrote:
>>
>>
>>
>> On 2/28/24 21:06, Melanie Plageman wrote:
>>> On Wed, Feb 28, 2024 at 2:23 PM Tomas Vondra
>>> <[email protected]> wrote:
>>>>
>>>> On 2/28/24 15:56, Tomas Vondra wrote:
>>>>>> ...
>>>>>
>>>>> Sure, I can do that. It'll take a couple hours to get the results, I'll
>>>>> share them when I have them.
>>>>>
>>>>
>>>> Here are the results with only patches 0001 - 0012 applied (i.e. without
>>>> the patch introducing the streaming read API, and the patch switching
>>>> the bitmap heap scan to use it).
>>>>
>>>> The changes in performance don't disappear entirely, but the scale is
>>>> certainly much smaller - both in the complete results for all runs, and
>>>> for the "optimal" runs that would actually pick bitmapscan.
>>>
>>> Hmm. I'm trying to think how my refactor could have had this impact.
>>> It seems like all the most notable regressions are with 4 parallel
>>> workers. What do the numeric column labels mean across the top
>>> (2,4,8,16...) -- are they related to "matches"? And if so, what does
>>> that mean?
>>>
>>
>> That's the number of distinct values matched by the query, which should
>> be an approximation of the number of matching rows. The number of
>> distinct values in the data set differs by data set, but for 1M rows
>> it's roughly like this:
>>
>> uniform: 10k
>> linear: 10k
>> cyclic: 100
>>
>> So for example matches=128 means ~1% of rows for uniform/linear, and
>> 100% for cyclic data sets.
> 
> Ah, thank you for the explanation. I also looked at your script after
> having sent this email and saw that it is clear in your script what
> "matches" is.
> 
>> As for the possible cause, I think it's clear most of the difference
>> comes from the last patch that actually switches bitmap heap scan to the
>> streaming read API. That's mostly expected/understandable, although we
>> probably need to look into the regressions or cases with e_i_c=0.
> 
> Right, I'm mostly surprised about the regressions for patches 0001-0012.
> 
> Re eic 0: Thomas Munro and I chatted off-list, and you bring up a
> great point about eic 0. In old bitmapheapscan code eic 0 basically
> disabled prefetching but with the streaming read API, it will still
> issue fadvises when eic is 0. That is an easy one line fix. Thomas
> prefers to fix it by always avoiding an fadvise for the last buffer in
> a range before issuing a read (since we are about to read it anyway,
> best not fadvise it too). This will fix eic 0 and also cut one system
> call from each invocation of the streaming read machinery.
> 
>> To analyze the 0001-0012 patches, maybe it'd be helpful to run tests for
>> individual patches. I can try doing that tomorrow. It'll have to be a
>> limited set of tests, to reduce the time, but might tell us whether it's
>> due to a single patch or multiple patches.
> 
> Yes, tomorrow I planned to start trying to repro some of the "red"
> cases myself. Any one of the commits could cause a slight regression
> but a 3.5x regression is quite surprising, so I might focus on trying
> to repro that locally and then narrow down which patch causes it.
> 
> For the non-cached regressions, perhaps the commit to use the correct
> recheck flag (0004) when prefetching could be the culprit. And for the
> cached regressions, my money is on the commit which changes the whole
> control flow of BitmapHeapNext() and the next_block() and next_tuple()
> functions (0010).
> 

I do have some partial results, comparing the patches. I only ran one of
the more affected workloads (cyclic) on the xeon, attached is a PDF
comparing master and the 0001-0014 patches. The percentages are timing
vs. the preceding patch (green - faster, red - slower).

This suggests only patches 0010 and 0014 affect performance, the rest is
just noise. I'll see if I can do more runs and get data from the other
machine (seems it's more significant on old SATA SSDs).

regards

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

Attachments:

  [application/pdf] patch-comparison.pdf (155.3K, ../[email protected]/2-patch-comparison.pdf)
  download

view thread (10+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: BitmapHeapScan streaming read user and prelim refactoring
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox