Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On Wed, Feb 02, 2011 at 12:10:59PM -0500, Andrew Dunstan wrote: On 02/02/2011 11:45 AM, Tim Bunce wrote: But why are we bothering to keep $prolog at all any more, if all we're going to pass it isPL_sv_no all the time? Maybe we'll have a use for it in the future, but right now we don't appear to unless I'm missing something. PostgreSQL::PLPerl::NYTProf would break if it was removed, so I'd rather it wasn't. I could work around that if there's an easy way for perl code to tell what version of PostgreSQL. If there isn't I think it would be worth adding. Not really. It might well be good to add but that needs to wait for another time. Ok. I gather you're plugging in a replacement for mkfunc? Yes. For now I'll add a comment to the code saying why it's there. Thanks. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On Mon, Jan 31, 2011 at 02:22:37PM -0500, Andrew Dunstan wrote: On 01/15/2011 12:31 AM, Alex Hunsaker wrote: On Tue, Dec 7, 2010 at 07:24, Tim Buncetim.bu...@pobox.com wrote: Changes: Sets the local $_TD via C instead of passing an extra argument. So functions no longer start with our $_TD; local $_TD = shift; Pre-extend stack for trigger arguments for slight performance gain. Passes installcheck. Cool, surprisingly in the non trigger case I saw up to an 18% speedup. That's great. The trigger case remained about the same, I suppose im I/O bound. Find attached a v2 with some minor fixes, If it looks good to you Ill mark this as Ready for Commit. Changes: - move up a declaration to make it c90 safe - avoid using tg_trigger before it was initialized - only extend the stack to the size we need (there was + 1 which unless I am missing something was needed because we used to push $_TD on the stack, but we dont any more) This looks pretty good. But why are we bothering to keep $prolog at all any more, if all we're going to pass it is PL_sv_no all the time? Maybe we'll have a use for it in the future, but right now we don't appear to unless I'm missing something. PostgreSQL::PLPerl::NYTProf would break if it was removed, so I'd rather it wasn't. I could work around that if there's an easy way for perl code to tell what version of PostgreSQL. If there isn't I think it would be worth adding. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On 02/02/2011 11:45 AM, Tim Bunce wrote: But why are we bothering to keep $prolog at all any more, if all we're going to pass it isPL_sv_no all the time? Maybe we'll have a use for it in the future, but right now we don't appear to unless I'm missing something. PostgreSQL::PLPerl::NYTProf would break if it was removed, so I'd rather it wasn't. I could work around that if there's an easy way for perl code to tell what version of PostgreSQL. If there isn't I think it would be worth adding. Not really. It might well be good to add but that needs to wait for another time. I gather you're plugging in a replacement for mkfunc? For now I'll add a comment to the code saying why it's there. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On Mon, Jan 31, 2011 at 12:22, Andrew Dunstan and...@dunslane.net wrote: This looks pretty good. But why are we bothering to keep $prolog at all any more, if all we're going to pass it is PL_sv_no all the time? Maybe we'll have a use for it in the future, but right now we don't appear to unless I'm missing something. I don't see any reason to keep it around. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On 01/15/2011 12:31 AM, Alex Hunsaker wrote: On Tue, Dec 7, 2010 at 07:24, Tim Buncetim.bu...@pobox.com wrote: Changes: Sets the local $_TD via C instead of passing an extra argument. So functions no longer start with our $_TD; local $_TD = shift; Pre-extend stack for trigger arguments for slight performance gain. Passes installcheck. Cool, surprisingly in the non trigger case I saw up to an 18% speedup. The trigger case remained about the same, I suppose im I/O bound. Find attached a v2 with some minor fixes, If it looks good to you Ill mark this as Ready for Commit. Changes: - move up a declaration to make it c90 safe - avoid using tg_trigger before it was initialized - only extend the stack to the size we need (there was + 1 which unless I am missing something was needed because we used to push $_TD on the stack, but we dont any more) This looks pretty good. But why are we bothering to keep $prolog at all any more, if all we're going to pass it is PL_sv_no all the time? Maybe we'll have a use for it in the future, but right now we don't appear to unless I'm missing something. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On Tue, Dec 7, 2010 at 07:24, Tim Bunce tim.bu...@pobox.com wrote:
Changes:
Sets the local $_TD via C instead of passing an extra argument.
So functions no longer start with our $_TD; local $_TD = shift;
Pre-extend stack for trigger arguments for slight performance gain.
Passes installcheck.
Cool, surprisingly in the non trigger case I saw up to an 18% speedup.
The trigger case remained about the same, I suppose im I/O bound.
Find attached a v2 with some minor fixes, If it looks good to you Ill
mark this as Ready for Commit.
Changes:
- move up a declaration to make it c90 safe
- avoid using tg_trigger before it was initialized
- only extend the stack to the size we need (there was + 1 which
unless I am missing something was needed because we used to push $_TD
on the stack, but we dont any more)
Benchmarks:
---
create table t (a int);
create or replace function func(int) returns int as $$ return $_[0];
$$ language plperl;
create or replace function trig() returns trigger as $$
$_TD-{'new'}{'a'}++; return 'MODIFY'; $$ language plperl;
-- pre patch, simple function call, 3 runs
SELECT sum(func(1)) from generate_series(1, 1000);
Time: 30908.675 ms
Time: 30916.995 ms
Time: 31173.122 ms
-- post patch
SELECT sum(func(1)) from generate_series(1, 1000);
Time: 26460.987 ms
Time: 26465.480 ms
Time: 25958.016 ms
-- pre patch, trigger
insert into t (a) select generate_series(1, 100);
Time: 18186.390 ms
Time: 21291.721 ms
Time: 20782.238 ms
-- post
insert into t (a) select generate_series(1, 100);
Time: 19136.633 ms
Time: 21140.095 ms
Time: 22062.578 ms
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 5595baa..7fb69df 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -1421,7 +1421,7 @@ plperl_create_sub(plperl_proc_desc *prodesc, char *s, Oid fn_oid)
EXTEND(SP, 4);
PUSHs(sv_2mortal(newSVstring(subname)));
PUSHs(sv_2mortal(newRV_noinc((SV *) pragma_hv)));
- PUSHs(sv_2mortal(newSVstring(our $_TD; local $_TD=shift;)));
+ PUSHs(PL_sv_no); /* unused */
PUSHs(sv_2mortal(newSVstring(s)));
PUTBACK;
@@ -1493,9 +1493,7 @@ plperl_call_perl_func(plperl_proc_desc *desc, FunctionCallInfo fcinfo)
SAVETMPS;
PUSHMARK(SP);
- EXTEND(sp, 1 + desc-nargs);
-
- PUSHs(PL_sv_undef); /* no trigger data */
+ EXTEND(sp, desc-nargs);
for (i = 0; i desc-nargs; i++)
{
@@ -1575,21 +1573,22 @@ plperl_call_perl_trigger_func(plperl_proc_desc *desc, FunctionCallInfo fcinfo,
SV *td)
{
dSP;
- SV *retval;
- Trigger*tg_trigger;
- int i;
- int count;
+ SV *retval, *TDsv;
+ int i, count;
+ Trigger*tg_trigger = ((TriggerData *) fcinfo-context)-tg_trigger;
ENTER;
SAVETMPS;
- PUSHMARK(sp);
+ TDsv = get_sv(_TD, GV_ADD);
+ SAVESPTR(TDsv); /* local $_TD */
+ sv_setsv(TDsv, td);
- XPUSHs(td);
+ PUSHMARK(sp);
+ EXTEND(sp, tg_trigger-tgnargs);
- tg_trigger = ((TriggerData *) fcinfo-context)-tg_trigger;
for (i = 0; i tg_trigger-tgnargs; i++)
- XPUSHs(sv_2mortal(newSVstring(tg_trigger-tgargs[i])));
+ PUSHs(sv_2mortal(newSVstring(tg_trigger-tgargs[i])));
PUTBACK;
/* Do NOT use G_KEEPERR here */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On Dec 9, 2010, at 7:32 PM, Tim Bunce wrote:
On Wed, Dec 08, 2010 at 09:21:05AM -0800, David E. Wheeler wrote:
On Dec 8, 2010, at 9:14 AM, Tim Bunce wrote:
Do you have any more improvements in the pipeline?
I'd like to add $arrayref = decode_array_literal('{2,3}') and
maybe $hashref = decode_hstore_literal('x=1, y=2').
I don't know how much works would be involved in those though.
Those would be handy, but for arrays, at least, I'd rather have a GUC
to turn on so that arrays are passed to PL/perl functions as array
references.
Understood. At this stage I don't know what the issues are so I'm
nervous of over promising (plus I don't know how much time I'll have).
It's possible a blessed ref with string overloading would avoid
backwards compatibility issues.
I used to work on a patch that converts postgres arrays into perl array
references:
http://archives.postgresql.org/pgsql-hackers/2009-11/msg01552.php
I have a newer patch, which is, however, limited to one-dimensional resulting
arrays. If there's an interest in that approach I can update it for the
current code base, add support multi-dimensional arrays (I used to implement
that, but lost the changes accidentally) and post it for review.
/A
--
Alexey Klyukin http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On 12/13/2010 09:42 AM, Alexey Klyukin wrote: I used to work on a patch that converts postgres arrays into perl array references: http://archives.postgresql.org/pgsql-hackers/2009-11/msg01552.php I have a newer patch, which is, however, limited to one-dimensional resulting arrays. If there's an interest in that approach I can update it for the current code base, add support multi-dimensional arrays (I used to implement that, but lost the changes accidentally) and post it for review. Yes please. I had forgotten that you'd done that, so I duplicated some of your work yesterday, but it sounds like you have (or had) the guts of what I am still missing. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On Wed, Dec 08, 2010 at 09:21:05AM -0800, David E. Wheeler wrote:
On Dec 8, 2010, at 9:14 AM, Tim Bunce wrote:
Do you have any more improvements in the pipeline?
I'd like to add $arrayref = decode_array_literal('{2,3}') and
maybe $hashref = decode_hstore_literal('x=1, y=2').
I don't know how much works would be involved in those though.
Those would be handy, but for arrays, at least, I'd rather have a GUC
to turn on so that arrays are passed to PL/perl functions as array
references.
Understood. At this stage I don't know what the issues are so I'm
nervous of over promising (plus I don't know how much time I'll have).
It's possible a blessed ref with string overloading would avoid
backwards compatibility issues.
Tim.
Another possible improvement would be rewriting encode_array_literal()
in C, so returning arrays would be much faster.
+1
Best,
David
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On Tue, Dec 07, 2010 at 10:00:28AM -0500, Andrew Dunstan wrote:
On 12/07/2010 09:24 AM, Tim Bunce wrote:
Changes:
Sets the local $_TD via C instead of passing an extra argument.
So functions no longer start with our $_TD; local $_TD = shift;
Pre-extend stack for trigger arguments for slight performance gain.
Passes installcheck.
Please add it to the January commitfest.
Done. https://commitfest.postgresql.org/action/patch_view?id=446
Have you measured the speedup gained from this?
No. I doubt it's significant, but every little helps :)
Do you have any more improvements in the pipeline?
I'd like to add $arrayref = decode_array_literal('{2,3}') and
maybe $hashref = decode_hstore_literal('x=1, y=2').
I don't know how much works would be involved in those though.
Another possible improvement would be rewriting encode_array_literal()
in C, so returning arrays would be much faster.
I'd also like to #define PERL_NO_GET_CONTEXT, which would give a useful
performance boost by avoiding all the many hidden calls to lookup
thread-local storage. (PERL_SET_CONTEXT() would go and instead the
'currrent interpreter' would be passed around as an extra argument.)
That's a fairly mechanical change but the diff may be quite large.
Tim.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On Dec 8, 2010, at 9:14 AM, Tim Bunce wrote:
Do you have any more improvements in the pipeline?
I'd like to add $arrayref = decode_array_literal('{2,3}') and
maybe $hashref = decode_hstore_literal('x=1, y=2').
I don't know how much works would be involved in those though.
Those would be handy, but for arrays, at least, I'd rather have a GUC to turn
on so that arrays are passed to PL/perl functions as array references.
Another possible improvement would be rewriting encode_array_literal()
in C, so returning arrays would be much faster.
+1
Best,
David
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Optimize PL/Perl function argument passing [PATCH]
Changes:
Sets the local $_TD via C instead of passing an extra argument.
So functions no longer start with our $_TD; local $_TD = shift;
Pre-extend stack for trigger arguments for slight performance gain.
Passes installcheck.
Tim.
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 5595baa..a924cfd 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
*** plperl_create_sub(plperl_proc_desc *prod
*** 1421,1427
EXTEND(SP, 4);
PUSHs(sv_2mortal(newSVstring(subname)));
PUSHs(sv_2mortal(newRV_noinc((SV *) pragma_hv)));
! PUSHs(sv_2mortal(newSVstring(our $_TD; local $_TD=shift;)));
PUSHs(sv_2mortal(newSVstring(s)));
PUTBACK;
--- 1421,1427
EXTEND(SP, 4);
PUSHs(sv_2mortal(newSVstring(subname)));
PUSHs(sv_2mortal(newRV_noinc((SV *) pragma_hv)));
! PUSHs(PL_sv_no); /* unused */
PUSHs(sv_2mortal(newSVstring(s)));
PUTBACK;
*** plperl_call_perl_func(plperl_proc_desc *
*** 1495,1502
PUSHMARK(SP);
EXTEND(sp, 1 + desc-nargs);
- PUSHs(PL_sv_undef); /* no trigger data */
-
for (i = 0; i desc-nargs; i++)
{
if (fcinfo-argnull[i])
--- 1495,1500
*** plperl_call_perl_trigger_func(plperl_pro
*** 1583,1595
ENTER;
SAVETMPS;
! PUSHMARK(sp);
! XPUSHs(td);
tg_trigger = ((TriggerData *) fcinfo-context)-tg_trigger;
for (i = 0; i tg_trigger-tgnargs; i++)
! XPUSHs(sv_2mortal(newSVstring(tg_trigger-tgargs[i])));
PUTBACK;
/* Do NOT use G_KEEPERR here */
--- 1581,1596
ENTER;
SAVETMPS;
! SV *TDsv = get_sv(_TD, GV_ADD);
! SAVESPTR(TDsv); /* local $_TD */
! sv_setsv(TDsv, td);
! PUSHMARK(sp);
! EXTEND(sp, 1 + tg_trigger-tgnargs);
tg_trigger = ((TriggerData *) fcinfo-context)-tg_trigger;
for (i = 0; i tg_trigger-tgnargs; i++)
! PUSHs(sv_2mortal(newSVstring(tg_trigger-tgargs[i])));
PUTBACK;
/* Do NOT use G_KEEPERR here */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On 12/07/2010 09:24 AM, Tim Bunce wrote: Changes: Sets the local $_TD via C instead of passing an extra argument. So functions no longer start with our $_TD; local $_TD = shift; Pre-extend stack for trigger arguments for slight performance gain. Passes installcheck. Please add it to the January commitfest. Have you measured the speedup gained from this? Do you have any more improvements in the pipeline? cheers anrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
