On Wed, Aug 2, 2023 at 12:11 PM Yuya Watari <watari.y...@gmail.com> wrote:
> Hello, > > I really appreciate sharing very useful scripts and benchmarking results. > > On Fri, Jul 28, 2023 at 6:51 PM Ashutosh Bapat > <ashutosh.bapat....@gmail.com> wrote: > > Given that most of the developers run assert enabled builds it would > > be good to bring down the degradation there while keeping the > > excellent speedup in non-assert builds. > > From my observation, this degradation in assert enabled build is > caused by verifying the iteration results of EquivalenceMembers. My > patch uses Bitmapset-based indexes to speed up the iteration. When > assertions are enabled, we verify that the result of the iteration is > the same with and without the indexes. This verification results in > executing a similar loop three times, which causes the degradation. I > measured planning time by using your script without this verification. > The results are as follows: > > Master: 144.55 ms > Patched (v19): 529.85 ms > Patched (v19) without verification: 78.84 ms > (*) All runs are with assertions. > > As seen from the above, verifying iteration results was the cause of > the performance degradation. I agree that we should avoid such > degradation because it negatively affects the development of > PostgreSQL. Removing the verification when committing this patch is > one possible option. > If you think that the verification is important to catch bugs, you may want to encapsulate it with an #ifdef .. #endif such that the block within is not compiled by default. See OPTIMIZER_DEBUG for example. > > > Queries on partitioned tables eat a lot of memory anyways, increasing > > that further should be avoided. > > > > I have not studied the patches. But I think the memory increase has to > > do with our Bitmapset structure. It's space inefficient when there are > > thousands of partitions involved. See my comment at [2] > > Thank you for pointing this out. I have never considered the memory > usage impact of this patch. As you say, the Bitmapset structure caused > this increase. I will try to look into this further. > > Do you think that the memory measurement patch I have shared in those threads is useful in itself? If so, I will start another proposal to address it. -- Best Wishes, Ashutosh Bapat
