From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Cc: | Alexander Pyhalov <a(dot)pyhalov(at)postgrespro(dot)ru>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
Subject: | Performance issues with v18 SQL-language-function changes |
Date: | 2025-04-13 19:23:24 |
Message-ID: | 1112592.1744572204@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
v1-0001-Minor-performance-improvement-for-SQL-language-fu.patch | text/x-diff | 3.0 KB |
v1-0002-Make-functions.c-mostly-run-in-a-short-lived-memo.patch | text/x-diff | 15.0 KB |
v1-0003-Split-some-storage-out-to-separate-subcontexts-of.patch | text/x-diff | 7.5 KB |
v1-0004-Make-SQLFunctionCache-long-lived-again.patch | text/x-diff | 18.6 KB |
v1-0005-Cache-typlens-of-a-SQL-function-s-input-arguments.patch | text/x-diff | 2.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Jelte Fennema-Nio | 2025-04-13 19:30:46 | Re: Bump soft open file limit (RLIMIT_NOFILE) to hard limit on startup |
Previous Message | Andrew Dunstan | 2025-04-13 18:47:55 | Re: Buildfarm: Enabling injection points on basilisk/dogfish (Alpine / musl) |