Обсуждение: pgsql: Improve performance of repeated CALLs within plpgsql procedures.
Improve performance of repeated CALLs within plpgsql procedures. This patch essentially is cleaning up technical debt left behind by the original implementation of plpgsql procedures, particularly commit d92bc83c4. That patch (or more precisely, follow-on patches fixing its worst bugs) forced us to re-plan CALL and DO statements each time through, if we're in a non-atomic context. That wasn't for any fundamental reason, but just because use of a saved plan requires having a ResourceOwner to hold a reference count for the plan, and we had no suitable resowner at hand, nor would the available APIs support using one if we did. While it's not that expensive to create a "plan" for CALL/DO, the cycles do add up in repeated executions. This patch therefore makes the following API changes: * GetCachedPlan/ReleaseCachedPlan are modified to let the caller specify which resowner to use to pin the plan, rather than forcing use of CurrentResourceOwner. * spi.c gains a "SPI_execute_plan_extended" entry point that lets callers say which resowner to use to pin the plan. This borrows the idea of an options struct from the recently added SPI_prepare_extended, hopefully allowing future options to be added without more API breaks. This supersedes SPI_execute_plan_with_paramlist (which I've marked deprecated) as well as SPI_execute_plan_with_receiver (which is new in v14, so I just took it out altogether). * I also took the opportunity to remove the crude hack of letting plpgsql reach into SPI private data structures to mark SPI plans as "no_snapshot". It's better to treat that as an option of SPI_prepare_extended. Now, when running a non-atomic procedure or DO block that contains any CALL or DO commands, plpgsql creates a ResourceOwner that will be used to pin the plans of the CALL/DO commands. (In an atomic context, we just use CurrentResourceOwner, as before.) Having done this, we can just save CALL/DO plans normally, whether or not they are used across transaction boundaries. This seems to be good for something like 2X speedup of a CALL of a trivial procedure with a few simple argument expressions. By restricting the creation of an extra ResourceOwner like this, there's essentially zero penalty in cases that can't benefit. Pavel Stehule, with some further hacking by me Discussion: https://postgr.es/m/CAFj8pRCLPdDAETvR7Po7gC5y_ibkn_-bOzbeJb39WHms01194Q@mail.gmail.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/ee895a655ce4341546facd6f23e3e8f2931b96bf Modified Files -------------- doc/src/sgml/spi.sgml | 165 ++++++++----- src/backend/commands/prepare.c | 7 +- src/backend/executor/spi.c | 106 +++++--- src/backend/tcop/postgres.c | 2 +- src/backend/utils/cache/plancache.c | 32 +-- src/backend/utils/mmgr/portalmem.c | 2 +- src/backend/utils/resowner/resowner.c | 8 +- src/include/executor/spi.h | 17 +- src/include/executor/spi_priv.h | 1 - src/include/utils/plancache.h | 4 +- src/pl/plpgsql/src/pl_comp.c | 2 + src/pl/plpgsql/src/pl_exec.c | 440 ++++++++++++++++------------------ src/pl/plpgsql/src/pl_gram.y | 6 + src/pl/plpgsql/src/pl_handler.c | 25 ++ src/pl/plpgsql/src/plpgsql.h | 11 +- 15 files changed, 461 insertions(+), 367 deletions(-)
Re: pgsql: Improve performance of repeated CALLs within plpgsql procedures.
От
Michael Paquier
Дата:
Hi Tom, On Tue, Jan 26, 2021 at 03:28:39AM +0000, Tom Lane wrote: > Improve performance of repeated CALLs within plpgsql procedures. > > This patch essentially is cleaning up technical debt left behind > by the original implementation of plpgsql procedures, particularly > commit d92bc83c4. That patch (or more precisely, follow-on patches > fixing its worst bugs) forced us to re-plan CALL and DO statements > each time through, if we're in a non-atomic context. That wasn't > for any fundamental reason, but just because use of a saved plan > requires having a ResourceOwner to hold a reference count for the > plan, and we had no suitable resowner at hand, nor would the > available APIs support using one if we did. While it's not that > expensive to create a "plan" for CALL/DO, the cycles do add up > in repeated executions. lapwing is generating a warning here: https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=lapwing&dt=2021-01-26%2010%3A40%3A09&stg=make pl_handler.c: In function 'plpgsql_call_handler': pl_handler.c:303:2: error: 'retval' may be used uninitialized in this function [-Werror=maybe-uninitialized] pl_handler.c:228:8: note: 'retval' was declared here -- Michael
Вложения
Michael Paquier <michael@paquier.xyz> writes:
> On Tue, Jan 26, 2021 at 03:28:39AM +0000, Tom Lane wrote:
>> Improve performance of repeated CALLs within plpgsql procedures.
> lapwing is generating a warning here:
Yeah, I saw that last night. It's weird, because that patch surely
did not change anything that should affect defined-ness of retval.
I plan to fix it by just immediately initializing retval, but I want
to wait a little bit, because I'm curious to see if any other compilers
complain.
regards, tom lane
I wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> On Tue, Jan 26, 2021 at 03:28:39AM +0000, Tom Lane wrote:
>>> Improve performance of repeated CALLs within plpgsql procedures.
>> lapwing is generating a warning here:
> Yeah, I saw that last night. It's weird, because that patch surely
> did not change anything that should affect defined-ness of retval.
> I plan to fix it by just immediately initializing retval, but I want
> to wait a little bit, because I'm curious to see if any other compilers
> complain.
Did that now. A survey of the buildfarm finds all of these animals
warning about retval, in addition to lapwing:
curculio | 2021-01-26 11:30:20 | pl_handler.c:228: warning: 'retval' may be used uninitialized in this function
damselfly | 2021-01-26 11:30:28 | pl_handler.c:303:2: warning: 'retval' may be used uninitialized in this function
[-Wmaybe-uninitialized]
locust | 2021-01-26 11:14:42 | pl_handler.c:228: warning: 'retval' may be used uninitialized in this function
mantid | 2021-01-26 11:07:08 | pl_handler.c:303:2: warning: 'retval' may be used uninitialized in this function
[-Wmaybe-uninitialized]
prairiedog | 2021-01-25 13:19:02 | pl_handler.c:227: warning: 'retval' may be used uninitialized in this function
prion | 2021-01-26 10:03:14 |
/home/ec2-user/bf/root/HEAD/pgsql.build/../pgsql/src/pl/plpgsql/src/pl_handler.c:303:2:warning: 'retval' may be used
uninitializedin this function [-Wmaybe-uninitialized]
rhinoceros | 2021-01-26 09:52:17 | pl_handler.c:303:2: warning: 'retval' may be used uninitialized in this function
[-Wmaybe-uninitialized]
sidewinder | 2021-01-26 11:45:19 | pl_handler.c:303:2: warning: 'retval' may be used uninitialized in this function
[-Wmaybe-uninitialized]
skate | 2021-01-26 09:06:47 | pl_handler.c:303:2: warning: 'retval' may be used uninitialized in this function
[-Wmaybe-uninitialized]
takin | 2021-01-26 08:51:19 | pl_handler.c:303:2: warning: 'retval' may be used uninitialized in this function
[-Wmaybe-uninitialized]
Some of these may predate this commit (the one from prairiedog
clearly does). I also noted a few animals complaining about -Wclobber:
francolin | 2021-01-14 02:20:10 |
/home/andres/build/buildfarm-francolin/HEAD/pgsql.build/../pgsql/src/pl/plpgsql/src/pl_handler.c:227:9:warning:
variable'retval' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]
piculet | 2021-01-14 04:16:30 |
/home/andres/build/buildfarm-piculet/HEAD/pgsql.build/../pgsql/src/pl/plpgsql/src/pl_handler.c:227:9:warning: variable
'retval'might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]
rorqual | 2021-01-14 02:20:10 |
/home/andres/build/buildfarm-atomics-spinlocks/HEAD/pgsql.build/../pgsql/src/pl/plpgsql/src/pl_handler.c:227:9:warning:
variable'retval' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]
serinus | 2021-01-26 09:44:08 |
/home/bf/build/buildfarm-serinus/HEAD/pgsql.build/../pgsql/src/pl/plpgsql/src/pl_handler.c:227:16:warning: variable
'procedure_resowner'might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]
serinus | 2021-01-26 09:44:08 |
/home/bf/build/buildfarm-serinus/HEAD/pgsql.build/../pgsql/src/pl/plpgsql/src/pl_handler.c:228:9:warning: variable
'retval'might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]
skink | 2021-01-13 19:41:43 |
/home/andres/build/buildfarm/HEAD/pgsql.build/../pgsql/src/pl/plpgsql/src/pl_handler.c:227:9:warning: variable 'retval'
mightbe clobbered by 'longjmp' or 'vfork' [-Wclobbered]
and made an effort to silence that too.
regards, tom lane