Re: Change pfree to accept NULL argument

On Tue, 23 Aug 2022 at 06:30, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)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