Обсуждение: Re: [RFC] Lock-free XLog Reservation from WAL
On Wed, 22 Jan 2025 at 22:44, Japin Li <japinli@hotmail.com> wrote: > On Wed, 22 Jan 2025 at 17:02, Yura Sokolov <y.sokolov@postgrespro.ru> wrote: >> I believe, I know why it happens: I was in hurry making v2 by >> cherry-picking internal version. I reverted some changes in >> CalcCuckooPositions manually and forgot to add modulo >> PREV_LINKS_HASH_CAPA. >> >> Here's the fix: >> >> pos->pos[0] = hash % PREV_LINKS_HASH_CAPA; >> - pos->pos[1] = pos->pos[0] + 1; >> + pos->pos[1] = (pos->pos[0] + 1) % PREV_LINKS_HASH_CAPA; >> pos->pos[2] = (hash >> 16) % PREV_LINKS_HASH_CAPA; >> - pos->pos[3] = pos->pos[2] + 2; >> + pos->pos[3] = (pos->pos[2] + 2) % PREV_LINKS_HASH_CAPA; >> >> Any way, here's v3: >> - excess file "v0-0001-Increase..." removed. I believe it was source >> of white-space apply warnings. >> - this mistake fixed >> - more clear slots strategies + "8 positions in two cache-lines" strategy. >> >> You may play with switching PREV_LINKS_HASH_STRATEGY to 2 or 3 and see >> if it affects measurably. > > Thanks for your quick fixing. I will retest it tomorrow. Hi, Yura Sokolov Here is my test result of the v3 patch: | case | min | avg | max | |-------------------------------+------------+------------+------------| | master (44b61efb79) | 865,743.55 | 871,237.40 | 874,492.59 | | v3 | 857,020.58 | 860,180.11 | 864,355.00 | | v3 PREV_LINKS_HASH_STRATEGY=2 | 853,187.41 | 855,796.36 | 858,436.44 | | v3 PREV_LINKS_HASH_STRATEGY=3 | 863,131.97 | 864,272.91 | 865,396.42 | It seems there are some performance decreases :( or something I missed? -- Regrads, Japin Li
23.01.2025 11:46, Japin Li пишет: > On Wed, 22 Jan 2025 at 22:44, Japin Li <japinli@hotmail.com> wrote: >> On Wed, 22 Jan 2025 at 17:02, Yura Sokolov <y.sokolov@postgrespro.ru> wrote: >>> I believe, I know why it happens: I was in hurry making v2 by >>> cherry-picking internal version. I reverted some changes in >>> CalcCuckooPositions manually and forgot to add modulo >>> PREV_LINKS_HASH_CAPA. >>> >>> Here's the fix: >>> >>> pos->pos[0] = hash % PREV_LINKS_HASH_CAPA; >>> - pos->pos[1] = pos->pos[0] + 1; >>> + pos->pos[1] = (pos->pos[0] + 1) % PREV_LINKS_HASH_CAPA; >>> pos->pos[2] = (hash >> 16) % PREV_LINKS_HASH_CAPA; >>> - pos->pos[3] = pos->pos[2] + 2; >>> + pos->pos[3] = (pos->pos[2] + 2) % PREV_LINKS_HASH_CAPA; >>> >>> Any way, here's v3: >>> - excess file "v0-0001-Increase..." removed. I believe it was source >>> of white-space apply warnings. >>> - this mistake fixed >>> - more clear slots strategies + "8 positions in two cache-lines" strategy. >>> >>> You may play with switching PREV_LINKS_HASH_STRATEGY to 2 or 3 and see >>> if it affects measurably. >> >> Thanks for your quick fixing. I will retest it tomorrow. > > Hi, Yura Sokolov > > Here is my test result of the v3 patch: > > | case | min | avg | max | > |-------------------------------+------------+------------+------------| > | master (44b61efb79) | 865,743.55 | 871,237.40 | 874,492.59 | > | v3 | 857,020.58 | 860,180.11 | 864,355.00 | > | v3 PREV_LINKS_HASH_STRATEGY=2 | 853,187.41 | 855,796.36 | 858,436.44 | > | v3 PREV_LINKS_HASH_STRATEGY=3 | 863,131.97 | 864,272.91 | 865,396.42 | > > It seems there are some performance decreases :( or something I missed? > Hi, Japin. (Excuse me for duplicating message, I found I sent it only to you first time). v3 (as well as v2) doesn't increase NUM_XLOGINSERT_LOCKS itself. With only 8 in-progress inserters spin-lock is certainly better than any more complex solution. You need to compare "master" vs "master + NUM_XLOGINSERT_LOCKS=64" vs "master + NUM_XLOGINSERT_LOCKS=64 + v3". And even this way I don't claim "Lock-free reservation" gives any profit. That is why your benchmarking is very valuable! It could answer, does we need such not-small patch, or there is no real problem at all? ---- regards Yura Sokolov aka funny-falcon
On Thu, 23 Jan 2025 at 15:03, Yura Sokolov <y.sokolov@postgrespro.ru> wrote: > 23.01.2025 11:46, Japin Li пишет: >> On Wed, 22 Jan 2025 at 22:44, Japin Li <japinli@hotmail.com> wrote: >>> On Wed, 22 Jan 2025 at 17:02, Yura Sokolov <y.sokolov@postgrespro.ru> wrote: >>>> I believe, I know why it happens: I was in hurry making v2 by >>>> cherry-picking internal version. I reverted some changes in >>>> CalcCuckooPositions manually and forgot to add modulo >>>> PREV_LINKS_HASH_CAPA. >>>> >>>> Here's the fix: >>>> >>>> pos->pos[0] = hash % PREV_LINKS_HASH_CAPA; >>>> - pos->pos[1] = pos->pos[0] + 1; >>>> + pos->pos[1] = (pos->pos[0] + 1) % PREV_LINKS_HASH_CAPA; >>>> pos->pos[2] = (hash >> 16) % PREV_LINKS_HASH_CAPA; >>>> - pos->pos[3] = pos->pos[2] + 2; >>>> + pos->pos[3] = (pos->pos[2] + 2) % PREV_LINKS_HASH_CAPA; >>>> >>>> Any way, here's v3: >>>> - excess file "v0-0001-Increase..." removed. I believe it was source >>>> of white-space apply warnings. >>>> - this mistake fixed >>>> - more clear slots strategies + "8 positions in two cache-lines" strategy. >>>> >>>> You may play with switching PREV_LINKS_HASH_STRATEGY to 2 or 3 and see >>>> if it affects measurably. >>> >>> Thanks for your quick fixing. I will retest it tomorrow. >> Hi, Yura Sokolov >> Here is my test result of the v3 patch: >> | case | min | avg | max >> | >> |-------------------------------+------------+------------+------------| >> | master (44b61efb79) | 865,743.55 | 871,237.40 | 874,492.59 | >> | v3 | 857,020.58 | 860,180.11 | 864,355.00 | >> | v3 PREV_LINKS_HASH_STRATEGY=2 | 853,187.41 | 855,796.36 | 858,436.44 | >> | v3 PREV_LINKS_HASH_STRATEGY=3 | 863,131.97 | 864,272.91 | 865,396.42 | >> It seems there are some performance decreases :( or something I >> missed? >> > > Hi, Japin. > (Excuse me for duplicating message, I found I sent it only to you > first time). > Never mind! > v3 (as well as v2) doesn't increase NUM_XLOGINSERT_LOCKS itself. > With only 8 in-progress inserters spin-lock is certainly better than any > more complex solution. > > You need to compare "master" vs "master + NUM_XLOGINSERT_LOCKS=64" vs > "master + NUM_XLOGINSERT_LOCKS=64 + v3". > > And even this way I don't claim "Lock-free reservation" gives any profit. > > That is why your benchmarking is very valuable! It could answer, does > we need such not-small patch, or there is no real problem at all? > Thanks for your explanation. I will test it based on [1]. [1] https://www.postgresql.org/message-id/ME0P300MB0445471ABC855D0FA6FF0CA5B6E02%40ME0P300MB0445.AUSP300.PROD.OUTLOOK.COM -- Regrads, Japin Li