Horton

Thread

  1. Optimize PL/Perl function argument passing [PATCH]

    Tim Bunce <tim.bunce@pobox.com> — 2010-12-07T14:24:17Z

    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.

  2. Re: Optimize PL/Perl function argument passing [PATCH]

    Andrew Dunstan <andrew@dunslane.net> — 2010-12-07T15:00:28Z

    
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

  3. Re: Optimize PL/Perl function argument passing [PATCH]

    Tim Bunce <tim.bunce@pobox.com> — 2010-12-08T17:14:31Z

    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.

  4. Re: Optimize PL/Perl function argument passing [PATCH]

    David Wheeler <david@kineticode.com> — 2010-12-08T17:21:05Z

    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

  5. Re: Optimize PL/Perl function argument passing [PATCH]

    Tim Bunce <tim.bunce@pobox.com> — 2010-12-09T17:32:32Z

    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
>

  6. Re: Optimize PL/Perl function argument passing [PATCH]

    Alexey Klyukin <alexk@commandprompt.com> — 2010-12-13T14:42:51Z

    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

  7. Re: Optimize PL/Perl function argument passing [PATCH]

    Andrew Dunstan <andrew@dunslane.net> — 2010-12-13T14:55:42Z

    
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

  8. Re: Optimize PL/Perl function argument passing [PATCH]

    Alex Hunsaker <badalex@gmail.com> — 2011-01-15T05:31:03Z

    On Tue, Dec 7, 2010 at 07:24, Tim Bunce <Tim.Bunce@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, 10000000);
Time: 30908.675 ms
Time: 30916.995 ms
Time: 31173.122 ms

-- post patch
SELECT sum(func(1)) from generate_series(1, 10000000);
Time: 26460.987 ms
Time: 26465.480 ms
Time: 25958.016 ms

-- pre patch, trigger
insert into t (a) select generate_series(1, 1000000);
Time: 18186.390 ms
Time: 21291.721 ms
Time: 20782.238 ms

-- post
insert into t (a) select generate_series(1, 1000000);
Time: 19136.633 ms
Time: 21140.095 ms
Time: 22062.578 ms

  9. Re: Optimize PL/Perl function argument passing [PATCH]

    Andrew Dunstan <andrew@dunslane.net> — 2011-01-31T19:22:37Z

    
On 01/15/2011 12:31 AM, Alex Hunsaker wrote:
> On Tue, Dec 7, 2010 at 07:24, Tim Bunce<Tim.Bunce@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

  10. Re: Optimize PL/Perl function argument passing [PATCH]

    Alex Hunsaker <badalex@gmail.com> — 2011-02-01T16:51:55Z

    On Mon, Jan 31, 2011 at 12:22, Andrew Dunstan <andrew@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.

  11. Re: Optimize PL/Perl function argument passing [PATCH]

    Tim Bunce <tim.bunce@pobox.com> — 2011-02-02T16:45:52Z

    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 Bunce<Tim.Bunce@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.

  12. Re: Optimize PL/Perl function argument passing [PATCH]

    Andrew Dunstan <andrew@dunslane.net> — 2011-02-02T17:10:59Z

    
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 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.
>
>

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

  13. Re: Optimize PL/Perl function argument passing [PATCH]

    Tim Bunce <tim.bunce@pobox.com> — 2011-02-03T16:09:51Z

    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 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.
> 
> 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.