Thread: [HACKERS] type cache for concat functions
Hi
Now concat is 2x times slower than || operator. With cached FmgrInfo for output function it will be only 5%.
Regards
Pavel
Attachment
> On 20 May 2017 at 10:03, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
> Now concat is 2x times slower than || operator. With cached FmgrInfo for
> output function it will be only 5%.
Looks nice and does what's expected (what else one may need from a Saturday
evening). I just can mention that from what I see the previous version of
`concat` is slower the more arguments are getting involved, so looks like it
can be more than 2x.
Also, it was a little bit confusing to see that the function `concat` behaves
differently from operator `||` in terms of performance. When you're looking at
the code it's becoming obvious, but I couldn't find any mention about that in
the documentation.
2017-06-18 0:50 GMT+02:00 Dmitry Dolgov <9erthalion6@gmail.com>:
> On 20 May 2017 at 10:03, Pavel Stehule <pavel.stehule@gmail.com> wrote:>> Now concat is 2x times slower than || operator. With cached FmgrInfo for> output function it will be only 5%.Looks nice and does what's expected (what else one may need from a Saturdayevening). I just can mention that from what I see the previous version of`concat` is slower the more arguments are getting involved, so looks like itcan be more than 2x.Also, it was a little bit confusing to see that the function `concat` behavesdifferently from operator `||` in terms of performance. When you're looking atthe code it's becoming obvious, but I couldn't find any mention about that inthe documentation.
There was few steps how to evaluate expressions more faster, but no step how to evaluate faster polymorphic "any" functions. A detecting of used argument type and looking to type cache are expensive. These functions was designed for string processing in error messages, and the performance was not too important. Now the concat function can be used often as replacement of || operator when you do migration from Oracle due similar behave. And the performance is more interesting now.
I think so can be interesting to allow some activity in analyzing/planning time and prexec stage to user function developers. Some is possible via planning time hooks, but for controlling few functions it is not user friendly - and there are no space for data from this stages.
Regards
Pavel
Hi Pavel, I tried applying your patch, it applies and compiles fine, check and checkworld pass. I ran a simple performance test, select concat(generate_series(1,100000), ... [x5 total]) vs select generate_series(1,100000)::text || ... . Operator || runs in 60 ms, while unpatched concat takes 90 and patched -- 55 ms. About the code: * There seems to be an extra tab here: FmgrInfo *typcache; It's not aligned with the previous declaration with tab size 4. * You could allocate the cache and store it into the context inside rebuildConcatCache. It would return return the cache pointer, and the calling code would look cleaner -- just one line. This is a matter of taste though. * The nearby functions use snake_case, so it should be rebuild_concat_cache too. Overall the patch looks good to me. -- Alexander Kuzmenkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Hi
2017-08-24 19:24 GMT+02:00 Alexander Kuzmenkov <a.kuzmenkov@postgrespro.ru>:
Hi Pavel,
I tried applying your patch, it applies and compiles fine, check and checkworld pass.
I ran a simple performance test, select concat(generate_series(1,100000), ... [x5 total]) vs select generate_series(1,100000)::tex t || ... .
Operator || runs in 60 ms, while unpatched concat takes 90 and patched -- 55 ms.
About the code:
* There seems to be an extra tab here:
FmgrInfo *typcache;
It's not aligned with the previous declaration with tab size 4.
* You could allocate the cache and store it into the context inside rebuildConcatCache. It would return return the cache pointer, and the calling code would look cleaner -- just one line. This is a matter of taste though.
* The nearby functions use snake_case, so it should be rebuild_concat_cache too.
all should be fixed.
Regards
Pavel
Overall the patch looks good to me.
--
Alexander Kuzmenkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachment
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation: tested, passed Looks good to me. The new status of this patch is: Ready for Committer -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
2017-09-19 12:18 GMT+02:00 Alexander Kuzmenkov <a.kuzmenkov@postgrespro.ru>:
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, passed
Looks good to me.
The new status of this patch is: Ready for Committer
Thank you very much
Pavel
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Pavel Stehule <pavel.stehule@gmail.com> writes:
> [ faster-concat-2.patch ]
Pushed with some cosmetic adjustments (mostly better comments).
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2017-09-19 21:11 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:
Pavel Stehule <pavel.stehule@gmail.com> writes:
> [ faster-concat-2.patch ]
Pushed with some cosmetic adjustments (mostly better comments).
Thank you very much
Pavel
regards, tom lane