Hi, On 2025-07-16 18:27:45 +0300, Yura Sokolov wrote: > 16.07.2025 17:58, Andres Freund пишет: > >> Now, if I simply remove the spinlock in SIGetDataEntries, I see a drop of > >> just ~6% under concurrent DDL. I think this strongly suggests that the > >> spinlock is the bottleneck. > > > > This can be trivially optimized by just using atomics in a slightly more > > heavyweight way than Yura's patch upthread - my criticisim in [1] was purely > > that the reduced barriers make the patch incorrect. I don't understand why > > Yura didn't just move to full memory barriers, instead choosing to write a > > RW > > optimized spinlock. > > > > [1] > > https://postgr.es/m/5ccgykypol3azijw2chqnpg3rhuwjtwsmbfs3pgcqm7fu6laus%40wppo6zcfszay > > Excuse me, but I did at [1]. Not all barriers were changed to full, but one > necessary at the place you'd pointed to.
Then you went on to focus on a completely different approach. > But you just answered then [2]: > > I don't believe we have the whole story here. That was not in reply to the changed patch, but about the performance numbers you relayed. We had no repro, and even with the repro that Sergey has now delivered, we don't see similar levels of what you reported as contention. I still think that workloads bottlenecked by SI consumption could be improved a lot more by focusing on higher level improvements than on the spinlock. > [1] > https://www.postgresql.org/message-id/b798eb5e-35b7-40b5-a245-4170deab56f8%40postgrespro.ru > [2] > https://www.postgresql.org/message-id/4yfgfeu3f6f7fayl5h3kggtd5bkvm4gj3a3ryjs2qhlo6g74bt%40g3cu2u4pgaiw > > I moved to RW optimized spinlock because I found second place where it > helps. And patch includes both those places. > I think it is better to have good reusable tool applied in two places > instead of two custom pieces of code. And RWOptSpin is almost as good as > direct use of atomic operations. Shrug. I don't think it makes sense here, given that the spinlock literally just protects a single variable and that the most straightforward conversion to atomics is like a 10 line change. If somebody else feels different, they can pursue merging RWOptSpin, but I won't. I think it's going 100% in the wrong direction. Greetings, Andres Freund
