Performance issues with v18 SQL-language-function changes
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Cc: Alexander Pyhalov <a.pyhalov@postgrespro.ru>,
Pavel Stehule <pavel.stehule@gmail.com>
Date: 2025-04-13T19:23:24Z
Lists: pgsql-hackers
Attachments
- v1-0001-Minor-performance-improvement-for-SQL-language-fu.patch (text/x-diff) patch v1-0001
- v1-0002-Make-functions.c-mostly-run-in-a-short-lived-memo.patch (text/x-diff) patch v1-0002
- v1-0003-Split-some-storage-out-to-separate-subcontexts-of.patch (text/x-diff) patch v1-0003
- v1-0004-Make-SQLFunctionCache-long-lived-again.patch (text/x-diff) patch v1-0004
- v1-0005-Cache-typlens-of-a-SQL-function-s-input-arguments.patch (text/x-diff) patch v1-0005
In the thread that led up to commit 0dca5d68d [1], we'd convinced
ourselves that the new implementation was faster than the old.
So I was sad to discover that there are common cases where it's
a good bit slower. We'd focused too much on test methods like
do $$
begin
for i in 1..10000000 loop
perform fx((random()*100)::int);
end loop;
end;
$$;
The thing about this test case is that the SQL function under
test is executed only once per calling query (i.e., per PERFORM).
That doesn't allow the old implementation to amortize anything.
If you instead test cases like
create function fx(p_summa bigint) returns text immutable strict
return ltrim(to_char(p_summa, '999 999 999 999 999 999 999 999'));
explain analyze select fx(i) from generate_series(1,1000000) as i(i);
you arrive at the rude discovery that 0dca5d68d is about 50% slower
than 0dca5d68d^, because the old implementation builds a plan for fx()
only once and then re-uses it throughout the query. So does the new
implementation, but it has added GetCachedPlan overhead. Moreover,
I made the unforced error of deciding that we could tear down and
rebuild the SQLFunctionCache and subsidiary data structures for
each call. That overhead is relatively minor in comparison to the
cost of parsing and planning the function; but when comparing cases
where there's no repeated parsing and planning, it becomes
significant.
Hence, the attached patch series reverts that decision and goes back
to the old method of having the SQLFunctionCache and some associated
objects live as long as the FmgrInfo does (without, however, the
poorly-controlled memory leaks of the old code). In my testing
this gets us from a 50% penalty down to about 5%, which I think is
acceptable given the other benefits 0dca5d68d brought us.
I'm inclined to argue that, seeing that 0dca5d68d was mainly intended
as a performance feature, this performance loss is a bug that we
need to do something about even though we're post-feature-freeze.
We could either revert 0dca5d68d or apply the attached. I'd prefer
the latter.
(There are other things we could do to try to reduce the overhead
further, such as trying to not build a Tuplestore or JunkFilter in
simple cases. But that seems like new work not a fix for a bad
decision in existing work, so I think it's out of scope for v18.)
Comments?
regards, tom lane
[1] https://www.postgresql.org/message-id/flat/8216639.NyiUUSuA9g%40aivenlaptop
Commits
-
Don't use a tuplestore if we don't have to for SQL-language functions.
- e83a8ae44729 18.0 landed
-
Change SQL-language functions to use the plan cache.
- 0dca5d68d7be 18.0 cited
-
contrib/pageinspect: Use SQL-standard function bodies.
- 68ff25eef125 18.0 cited