On 5/11/21 7:35 PM, Tomas Vondra wrote:
>
>
> On 5/11/21 7:25 PM, Andres Freund wrote:
>> Hi,
>>
>> On 2021-05-11 16:07:44 +0200, Tomas Vondra wrote:
>>> On 5/11/21 11:04 AM, Masahiko Sawada wrote:
>>>> I think the changes for heap_multi_insert() are fine so we can revert
>>>> only heap_insert() part if we revert something from the v14 tree,
>>>> although we will end up not inserting frozen tuples into toast tables.
>>>>
>>>
>>> I'd be somewhat unhappy about reverting just this bit, because it'd mean
>>> that we freeze rows in the main table but not rows in the TOAST
>>> tables (that
>>> was kinda why we concluded we need the heap_insert part too).
>>
>> Is there a reason not to apply a polished version of my proposal? And
>> then to look at the remaining difference?
>>
>
> Probably not, I was just a little bit confused what exactly is going on,
> unsure what to do about it. But if RMV freezes the rows, that probably
> explains it and your patch is the way to go.
>
>>
>>> I'm still a bit puzzled where does the extra overhead (in cases when
>>> freeze
>>> is not requested) come from, TBH. Intuitively, I'd hope there's a way to
>>> eliminate that entirely, and only pay the cost when requested (with the
>>> expectation that it's cheaper than freezing it that later).
>>
>> I'd like to see a profile comparison between those two cases. Best with
>> both profiles done in master, just once with the freeze path disabled...
>>
>
> OK. I'm mostly afk at the moment, I'll do that once I get back home,
> sometime over the weekend / maybe early next week.
>
OK, so here are the flamegraphs, for all three cases - current master,
0c7d3bb99 (i.e. before heap_insert changes) and with the pinning patch
applied. I did this using the same test case as before (50M table), but
with -fno-omit-frame-pointer to get better profiles. It may add some
overhead, but hopefully that applies to all cases equally.
The first 10 runs for each case look like this:
old master patched
----------------------
55045 74284 58246
53927 74283 57273
54090 74114 57336
54194 74059 57223
54189 74186 57287
54090 74113 57278
54095 74036 57176
53896 74215 57303
54101 74060 57524
54062 74021 57278
----------------------
54168 74137 57392
1.36x 1.05x
which is mostly in line with previous findings (the master overhead is a
bit worse, possibly due to the frame pointers).
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 :-(
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
