From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Inlining of couple of functions in pl_exec.c improves performance |
Date: | 2020-07-01 22:17:22 |
Message-ID: | 811580.1593641842@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com> writes:
> There are a couple of function call overheads I observed in pl/pgsql
> code : exec_stmt() and exec_cast_value(). Removing these overheads
> resulted in some performance gains.
I took a look at the 0001/0002 patches (not 0003 as yet). I do not
like 0001 too much. The most concrete problem with it is that
you broke translatability of the error messages, cf the first
translatability guideline at [1]. While that could be fixed by passing
the entire message not just part of it, I don't see anything that we're
gaining by moving that stuff into exec_toplevel_block in the first place.
Certainly, passing a string that describes what will happen *after*
exec_toplevel_block is just weird. I think what you've got here is
a very arbitrary chopping-up of the existing code based on chance
similarities of the existing callers. I think we're better off to make
exec_toplevel_block be as nearly as possible a match for exec_stmts'
semantics.
Hence, I propose the attached 0001 to replace 0001/0002. This should
be basically indistinguishable performance-wise, though I have not
tried to benchmark. Note that for reviewability's sake, I did not
reindent the former body of exec_stmt, though we'd want to do that
before committing.
Also, 0002 is a small patch on top of that to avoid redundant saves
and restores of estate->err_stmt within the loop in exec_stmts. This
may well not be a measurable improvement, but it's a pretty obvious
inefficiency in exec_stmts now that it's refactored this way.
regards, tom lane
[1] https://www.postgresql.org/docs/devel/nls-programmer.html#NLS-GUIDELINES
Attachment | Content-Type | Size |
---|---|---|
0001-inline-exec_stmt-1.patch | text/x-diff | 3.9 KB |
0002-remove-redundant-err_stmt-changes-1.patch | text/x-diff | 1.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2020-07-01 22:22:51 | Re: pg_read_file() with virtual files returns empty string |
Previous Message | Magnus Hagander | 2020-07-01 21:44:46 | Re: Remove Deprecated Exclusive Backup Mode |