Re: ResourceOwner refactoring - Mailing list pgsql-hackers

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@gmail.com> wrote:
>>> On Mon, Jan 18, 2021 at 10:19 AM Heikki Linnakangas <hlinnaka@iki.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