Re: Performance degradation of REFRESH MATERIALIZED VIEW

On 5/24/21 9:53 AM, Masahiko Sawada wrote:
> On Sat, May 22, 2021 at 3:10 AM Tomas Vondra
> <tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
>>
>> On 5/21/21 6:43 PM, Andres Freund wrote:
>>> Hi,
>>>
>> > ...
>> >
>>>> Attached are the flame graphs for all three cases. The change in master is
>>>> pretty clearly visible, but I don't see any clear difference between old and
>>>> patched code :-(
>>>
>>> I'm pretty sure it's the additional WAL records?
>>>
>>
>> Not sure. If I understand what you suggested elsewhere in the thread, it
>> should be fine to modify heap_insert to pass the page recptr to
>> visibilitymap_set, roughly per the attached patch.
>>
>> I'm not sure it's correct, but it does eliminate the Heap2/VISIBILITY
>> records for me (when applied on top of your patch). Funnily enough it
>> does make it a wee bit slower:
>>
>> patch #1: 56941.505
>> patch #2: 58099.788
>>
>> I wonder if this might be due to -fno-omit-frame-pointer, though, as
>> without it I get these timings:
>>
>> 0c7d3bb99: 25540.417
>> master: 31868.236
>> patch #1: 26566.199
>> patch #2: 26487.943
>>
>> So without the frame pointers there's no slowdown, but there's no clear
>> improvement after removal of the WAL records either :-(
>
> Can we verify that the additional WAL records are the cause of this
> difference by making the matview unlogged by manually updating
> relpersistence = 'u'?
>
> Here are the results of benchmarks with unlogged matviews on my environment:
>
> 1) head: 22.927 sec
> 2) head w/ Andres’s patch: 16.629 sec
> 3) before 39b66a91b commit: 15.377 sec
> 4) head w/o freezing tuples: 14.551 sec
>
> And here are the results of logged matviews ICYMI:
>
> 1) head: 42.397 sec
> 2) head w/ Andres’s patch: 34.857 sec
> 3) before 39b66a91b commit: 32.556 sec
> 4) head w/o freezing tuples: 32.752 sec
>
> There seems no difference in the tendency. Which means the additional
> WAL is not the culprit?
>

Yeah, I agree the WAL does not seem to be the culprit here.

The patch I posted skips the WAL logging entirely (verified by
pg_waldump, although I have not mentioned that), and there's no clear
improvement. (FWIW I'm not sure the patch is 100% correct, but it does
eliminate the the extra WAL.)

The patch however does not skip the whole visibilitymap_set, it still
does the initial error checks. I wonder if that might play a role ...

Another option might be changes in the binary layout - 5% change is well
within the range that could be attributed to this, but it feels very
hand-wavy and more like an excuse than real analysis.

> Interestingly, my previously proposed patch[1] was a better
> performance. With the patch, we skip all VM-related work on all
> insertions except for when inserting a tuple into a page for the first
> time.
>
> logged matviews: 31.591 sec
> unlogged matviews: 15.317 sec
>

Hmmm, thanks for reminding us that patch. Why did we reject that
approach in favor of the current one?

I think at this point we have these two options:

1) Revert the freeze patches, either completely or just the heap_insert
part, which is what seems to be causing issues. And try again in PG15,
perhaps using a different approach, allow disabling freezing in refresh,
or something like that.

2) Polish and commit the pinning patch from Andres, which does reduce
the slowdown quite a bit. And either call it a day, or continue with the
investigation / analysis regarding the remaining ~5% (but I personally
have no idea what might be the problem ...).

I'd like to keep the improvement, but I find the 5% regression rather
annoying and hard to defend, considering how much we fight for every
little improvement.

regards

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