On Tue, 23 Aug 2022 at 06:30, Tom Lane <t...@sss.pgh.pa.us> wrote: > > Peter Eisentraut <peter.eisentr...@enterprisedb.com> writes: > > Per discussion in [0], here is a patch set that allows pfree() to accept > > a NULL argument, like free() does. > > So the question is, is this actually a good thing to do?
I think making pfree() accept NULL is a bad idea. The vast majority of cases the pointer will never be NULL, so we're effectively just burdening those with the additional overhead of checking for NULL. We know from [1] that adding branching in the memory management code can be costly. I'm measuring about a 2.6% slowdown from the 0002 patch using a function that I wrote [2] to hammer palloc/pfree. master postgres=# select pg_allocate_memory_test(64, 1024*1024, 10::bigint*1024*1024*1024,'aset'); Time: 2007.527 ms (00:02.008) Time: 1991.574 ms (00:01.992) Time: 2008.945 ms (00:02.009) Time: 2011.410 ms (00:02.011) Time: 2019.317 ms (00:02.019) Time: 2060.832 ms (00:02.061) Time: 2003.066 ms (00:02.003) Time: 2025.039 ms (00:02.025) Time: 2039.744 ms (00:02.040) Time: 2090.384 ms (00:02.090) master + pfree modifed to check for NULLs postgres=# select pg_allocate_memory_test(64, 1024*1024, 10::bigint*1024*1024*1024,'aset'); Time: 2057.625 ms (00:02.058) Time: 2074.699 ms (00:02.075) Time: 2075.629 ms (00:02.076) Time: 2104.581 ms (00:02.105) Time: 2072.620 ms (00:02.073) Time: 2066.916 ms (00:02.067) Time: 2071.962 ms (00:02.072) Time: 2097.520 ms (00:02.098) Time: 2087.421 ms (00:02.087) Time: 2078.695 ms (00:02.079) (~2.62% slowdown) If the aim here is to remove a bunch of ugly if (ptr) pfree(ptr); code, then why don't we just have a[n inline] function or a macro for that and only use it when we need to? David [1] https://www.postgresql.org/message-id/CAApHDvr6qFw3jLBL9d4zUpo3A2Cb6hoZsUnWD0vF1OGsd67v=w...@mail.gmail.com [2] https://www.postgresql.org/message-id/attachment/136801/pg_allocate_memory_test.patch.txt
