Re: Inlining of couple of functions in pl_exec.c improves performance

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

In response to

Responses

Browse pgsql-hackers by date

  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