Re: ResourceOwner refactoring

On 19/01/2021 11:09, Heikki Linnakangas wrote:
> On 18/01/2021 18:11, Robert Haas wrote:
>> On Mon, Jan 18, 2021 at 11:11 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>> On Mon, Jan 18, 2021 at 10:19 AM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>>>> On 18/01/2021 16:34, Alvaro Herrera wrote:
>>>>> So according to your performance benchmark, we're willing to accept a
>>>>> 30% performance loss on an allegedly common operation -- numkeep=0
>>>>> numsnaps=10 becomes 49.8ns from 37.6ns. That seems a bit shocking.
>>>>> Maybe you can claim that these operations aren't exactly hot spots, and
>>>>> so the fact that we remain in the same power-of-ten is sufficient. Is
>>>>> that the argument?
>>>>
>>>> That's right. The fast path is fast, and that's important. The slow path
>>>> becomes 30% slower, but that's acceptable.
>>
>> I don't know whether a 30% slowdown will hurt anybody, but it seems
>> like kind of a lot, and I'm not sure I understand what corresponding
>> benefit we're getting.
>
> The benefit is to make it easy for extensions to use resource owners to
> track whatever resources they need to track. And arguably, the new
> mechanism is nicer for built-in code, too.
>
> I'll see if I can optimize the slow paths, to make it more palatable.

Ok, here's a new set of patches, and new test results. I replaced the
hash function with a cheaper one. I also added the missing wrappers that
Alvaro and Hayato Kuroda commented on, and fixed the typos that Michael
Paquier pointed out.

In the test script, I increased the number of iterations used in the
tests, to make them run longer and produce more stable results. There is
still a fair amount of jitter in the results, so take any particular
number with a grain of salt, but the overall trend is repeatable.

The results now look like this:

Unpatched
---------

postgres=# \i snaptest.sql
numkeep | numsnaps | lifo_time_ns | fifo_time_ns
---------+----------+--------------+--------------
0 | 1 | 32.7 | 32.3
0 | 5 | 33.0 | 32.9
0 | 10 | 34.1 | 35.5
0 | 60 | 32.5 | 38.3
0 | 70 | 47.6 | 48.9
0 | 100 | 47.9 | 49.3
0 | 1000 | 52.9 | 52.7
0 | 10000 | 61.7 | 62.4
9 | 10 | 38.4 | 37.6
9 | 100 | 42.3 | 42.3
9 | 1000 | 43.9 | 45.0
9 | 10000 | 62.2 | 62.5
65 | 70 | 42.4 | 42.9
65 | 100 | 43.2 | 43.9
65 | 1000 | 44.0 | 45.1
65 | 10000 | 62.4 | 62.6
(16 rows)

Patched
-------

postgres=# \i snaptest.sql
numkeep | numsnaps | lifo_time_ns | fifo_time_ns
---------+----------+--------------+--------------
0 | 1 | 32.8 | 34.2
0 | 5 | 33.8 | 32.2
0 | 10 | 37.2 | 40.2
0 | 60 | 40.2 | 45.1
0 | 70 | 40.9 | 48.4
0 | 100 | 41.9 | 45.2
0 | 1000 | 49.0 | 55.6
0 | 10000 | 62.4 | 67.2
9 | 10 | 33.6 | 33.0
9 | 100 | 39.6 | 39.7
9 | 1000 | 42.2 | 45.0
9 | 10000 | 60.7 | 63.9
65 | 70 | 32.5 | 33.6
65 | 100 | 37.5 | 39.7
65 | 1000 | 42.3 | 46.3
65 | 10000 | 61.9 | 64.8
(16 rows)

For easier side-by-side comparison, here are the same numbers with the
patched and unpatched results side by side, and their ratio (ratio < 1
means the patched version is faster):

LIFO tests:
numkeep | numsnaps | unpatched | patched | ratio
---------+----------+-----------+---------+-------
0 | 1 | 32.7 | 32.8 | 1.00
0 | 5 | 33.0 | 33.8 | 1.02
0 | 10 | 34.1 | 37.2 | 1.09
0 | 60 | 32.5 | 40.2 | 1.24
0 | 70 | 47.6 | 40.9 | 0.86
0 | 100 | 47.9 | 41.9 | 0.87
0 | 1000 | 52.9 | 49.0 | 0.93
0 | 10000 | 61.7 | 62.4 | 1.01
9 | 10 | 38.4 | 33.6 | 0.88
9 | 100 | 42.3 | 39.6 | 0.94
9 | 1000 | 43.9 | 42.2 | 0.96
9 | 10000 | 62.2 | 60.7 | 0.98
65 | 70 | 42.4 | 32.5 | 0.77
65 | 100 | 43.2 | 37.5 | 0.87
65 | 1000 | 44.0 | 42.3 | 0.96
65 | 10000 | 62.4 | 61.9 | 0.99
(16 rows)

FIFO tests:
numkeep | numsnaps | unpatched | patched | ratio
---------+----------+-----------+---------+-------
0 | 1 | 32.3 | 34.2 | 1.06
0 | 5 | 32.9 | 32.2 | 0.98
0 | 10 | 35.5 | 40.2 | 1.13
0 | 60 | 38.3 | 45.1 | 1.18
0 | 70 | 48.9 | 48.4 | 0.99
0 | 100 | 49.3 | 45.2 | 0.92
0 | 1000 | 52.7 | 55.6 | 1.06
0 | 10000 | 62.4 | 67.2 | 1.08
9 | 10 | 37.6 | 33.0 | 0.88
9 | 100 | 42.3 | 39.7 | 0.94
9 | 1000 | 45.0 | 45.0 | 1.00
9 | 10000 | 62.5 | 63.9 | 1.02
65 | 70 | 42.9 | 33.6 | 0.78
65 | 100 | 43.9 | 39.7 | 0.90
65 | 1000 | 45.1 | 46.3 | 1.03
65 | 10000 | 62.6 | 64.8 | 1.04
(16 rows)

Summary: In the the worst scenario, the patched version is still 24%
slower than unpatched. But many other scenarios are now faster with the
patch.

- Heikki