Re: [HACKERS] Inlining comparators as a performance optimisation

2012-01-28 Thread Bruce Momjian
On Fri, Jan 13, 2012 at 10:48:56AM +0100, Pierre C wrote: Maybe an extra column in pg_proc would do (but then, the proargtypes and friends would describe only the sql-callable version) ? Or an extra table ? pg_cproc ? Or an in-memory hash : hashtable[ fmgr-callable function ] = C version -

Re: [HACKERS] Inlining comparators as a performance optimisation

2012-01-13 Thread Pierre C
On Wed, 21 Sep 2011 18:13:07 +0200, Tom Lane t...@sss.pgh.pa.us wrote: Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: On 21.09.2011 18:46, Tom Lane wrote: The idea that I was toying with was to allow the regular SQL-callable comparison function to somehow return a function

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-16 Thread Greg Smith
I think we can call a new sorting infrastructure popping out and what looks to be over 90 messages on this topic as successful progress on this front. Peter's going to rev a new patch, but with more performance results to review and followup discussion I can't see this one as wrapping for the

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-07 Thread Peter Geoghegan
On 7 December 2011 03:45, Robert Haas robertmh...@gmail.com wrote: In this regard, I think Heikki's remarks upthread are worth some thought.  If inlining is a win just because it avoids saving and restoring registers or allows better instruction scheduling, then inlining is the (probably?) the

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-07 Thread Robert Haas
On Tue, Dec 6, 2011 at 8:46 PM, Tom Lane t...@sss.pgh.pa.us wrote: 1. Adding sortsupport infrastructure for more datatypes. 2. Revising nbtree and related code to use this infrastructure. 3. Integrating Peter's work into this framework. I'll try to take care of #1 for at least a few key

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-07 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Tue, Dec 6, 2011 at 8:46 PM, Tom Lane t...@sss.pgh.pa.us wrote: 1. Adding sortsupport infrastructure for more datatypes. 2. Revising nbtree and related code to use this infrastructure. 3. Integrating Peter's work into this framework. I'll try to

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-07 Thread Robert Haas
On Wed, Dec 7, 2011 at 10:09 AM, Tom Lane t...@sss.pgh.pa.us wrote: There's some stuff that's debatable according to this criterion --- in particular, I wondered whether it'd be worth having a fast path for bttextcmp, especially if we pre-tested the collate_is_c condition and had a separate

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-07 Thread Peter Geoghegan
On 7 December 2011 15:15, Robert Haas robertmh...@gmail.com wrote: On Wed, Dec 7, 2011 at 10:09 AM, Tom Lane t...@sss.pgh.pa.us wrote:  But it would still have to be prepared for detoasting, so in the end I was unenthused.  Anyone who feels like testing could try to prove me wrong about it

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-07 Thread Robert Haas
On Wed, Dec 7, 2011 at 10:58 AM, Peter Geoghegan pe...@2ndquadrant.com wrote: On 7 December 2011 15:15, Robert Haas robertmh...@gmail.com wrote: On Wed, Dec 7, 2011 at 10:09 AM, Tom Lane t...@sss.pgh.pa.us wrote:  But it would still have to be prepared for detoasting, so in the end I was

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-06 Thread Nicolas Barbier
2011/12/5 Tom Lane t...@sss.pgh.pa.us: What is bothering me is that this approach is going to cause substantial bloat of the executable code, and such bloat has got distributed costs, which we don't have any really good way to measure but for sure micro-benchmarks addressing only sort speed

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-06 Thread Robert Haas
On Sun, Dec 4, 2011 at 2:17 PM, Tom Lane t...@sss.pgh.pa.us wrote: * I invented a SortKey struct that replaces ScanKey for tuplesort's purposes.  Right now that's local in tuplesort.c, but we're more than likely going to need it elsewhere as well.  Should we just define it in sortsupport.h?  

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-06 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Sun, Dec 4, 2011 at 2:17 PM, Tom Lane t...@sss.pgh.pa.us wrote: * We're going to want to expose PrepareSortSupportComparisonShim for use outside tuplesort.c too, and possibly refactor tuplesort_begin_heap so that the SortKey setup logic inside it

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-06 Thread Robert Haas
On Tue, Dec 6, 2011 at 12:06 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Sun, Dec 4, 2011 at 2:17 PM, Tom Lane t...@sss.pgh.pa.us wrote: * We're going to want to expose PrepareSortSupportComparisonShim for use outside tuplesort.c too, and possibly

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-06 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: OK. Well, then pushing it out to a separate file probably makes sense. Do you want to do that or shall I have a crack at it? If the latter, what do you think about using the name SortKey for everything rather than SortSupport? I'll take another

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-06 Thread Robert Haas
On Tue, Dec 6, 2011 at 1:07 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: OK.  Well, then pushing it out to a separate file probably makes sense.  Do you want to do that or shall I have a crack at it?  If the latter, what do you think about using the name

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-06 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Tue, Dec 6, 2011 at 1:07 PM, Tom Lane t...@sss.pgh.pa.us wrote: I'll take another crack at it. I'm not entirely sold yet on merging the two structs; I think first we'd better look and see what the needs are in the other potential callers I

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-06 Thread Robert Haas
On Tue, Dec 6, 2011 at 4:23 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Dec 6, 2011 at 1:07 PM, Tom Lane t...@sss.pgh.pa.us wrote: I'll take another crack at it.  I'm not entirely sold yet on merging the two structs; I think first we'd better look

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-06 Thread Peter Geoghegan
On 7 December 2011 00:18, Robert Haas robertmh...@gmail.com wrote: Works for me.  I think we should go ahead and get this part committed first, and then we can look at the inlining stuff as a further optimization for certain cases... Do you mean just inlining, or inlining and the numerous

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-06 Thread Robert Haas
On Tue, Dec 6, 2011 at 8:13 PM, Peter Geoghegan pe...@2ndquadrant.com wrote: On 7 December 2011 00:18, Robert Haas robertmh...@gmail.com wrote: Works for me.  I think we should go ahead and get this part committed first, and then we can look at the inlining stuff as a further optimization for

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-05 Thread Heikki Linnakangas
On 05.12.2011 02:14, Peter Geoghegan wrote: On 4 December 2011 19:17, Tom Lanet...@sss.pgh.pa.us wrote: I have not done any performance testing on this patch, but it might be interesting to check it with the same test cases Peter's been using. I've attached a revision of exactly the same

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-05 Thread Peter Geoghegan
On 5 December 2011 13:23, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: I'm still not convinced the big gain is in inlining the comparison functions. Your patch contains a few orthogonal tricks, too: * A fastpath for the case of just one scankey. No reason we can't do that

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-05 Thread Tom Lane
Peter Geoghegan pe...@2ndquadrant.com writes: Why the strong aversion to what I've done? I accept that it's ugly, but is it really worth worrying about that sort of regression in maintainability for something that was basically untouched since 2006, and will probably remain untouched after

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-04 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: OK, so I tried to code this up. Adding the new amproc wasn't too difficult (see attached). It wasn't obvious to me how to tie it into the tuplesort infrastructure, though, so instead of wasting time guessing what a sensible approach might be I'm

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-04 Thread Peter Geoghegan
On 4 December 2011 19:17, Tom Lane t...@sss.pgh.pa.us wrote: I have not done any performance testing on this patch, but it might be interesting to check it with the same test cases Peter's been using. I've attached a revision of exactly the same benchmark run to get the results in

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-03 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: OK, so I tried to code this up. Adding the new amproc wasn't too difficult (see attached). It wasn't obvious to me how to tie it into the tuplesort infrastructure, though, so instead of wasting time guessing what a sensible approach might be I'm

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-02 Thread Peter Geoghegan
On 2 December 2011 01:13, Tom Lane t...@sss.pgh.pa.us wrote: Regardless of that, I'm still of the opinion that this patch is fundamentally misdesigned.  The way it's set up, it is only possible to have a fast path for types that are hard-wired into tuplesort.c, and that flies in the face of

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-02 Thread Robert Haas
On Fri, Dec 2, 2011 at 12:16 AM, Tom Lane t...@sss.pgh.pa.us wrote: It should be the same or better.  Right now, we are going through FunctionCall2Coll to reach the FCI-style comparator.  The shim function would be more or less equivalent to that, and since it's quite special purpose I would

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-02 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: OK, but I think it's also going to cost you an extra syscache lookup, no? You're going to have to check for this new support function first, and then if you don't find it, you'll have to check for the original one. I don't think there's any

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-02 Thread Bruce Momjian
Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: OK, but I think it's also going to cost you an extra syscache lookup, no? You're going to have to check for this new support function first, and then if you don't find it, you'll have to check for the original one. I don't think

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-02 Thread Pavel Stehule
[ shrug... ] If you are bothered by that, get off your duff and provide the function for your datatype.  But it's certainly going to be in the noise for btree index usage, and I submit that query parsing/setup involves quite a lot of syscache lookups already.  I think that as a performance

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-02 Thread Robert Haas
On Fri, Dec 2, 2011 at 2:35 PM, Bruce Momjian br...@momjian.us wrote: Agreed.  Doing something once and doing something in the sort loop are two different overheads. OK, so I tried to code this up. Adding the new amproc wasn't too difficult (see attached). It wasn't obvious to me how to tie

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-02 Thread Bruce Momjian
Robert Haas wrote: On Fri, Dec 2, 2011 at 2:35 PM, Bruce Momjian br...@momjian.us wrote: Agreed. ?Doing something once and doing something in the sort loop are two different overheads. OK, so I tried to code this up. Adding the new amproc wasn't too difficult (see attached). It wasn't

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-01 Thread Peter Geoghegan
Attached is revision of my patch with some clean-ups. In particular, I'm now using switch statements for greater readability, plus supporting fast path sorting of the time datatype. I've also updated the documentation on Date/Time Types to note the additional disadvantage of using the deprecated

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-01 Thread Robert Haas
On Thu, Dec 1, 2011 at 11:44 AM, Peter Geoghegan pe...@2ndquadrant.com wrote: Attached is revision of my patch with some clean-ups. In particular, I'm now using switch statements for greater readability, plus supporting fast path sorting of the time datatype. I've also updated the

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-01 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Thu, Dec 1, 2011 at 11:44 AM, Peter Geoghegan pe...@2ndquadrant.com wrote: Attached is revision of my patch with some clean-ups. One thing I'm starting to get a bit concerned about is the effect of this patch on the size of the resulting binary.

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-01 Thread Peter Geoghegan
On 1 December 2011 17:15, Robert Haas robertmh...@gmail.com wrote: One thing I'm starting to get a bit concerned about is the effect of this patch on the size of the resulting binary.  The performance results you've posted are getting steadily more impressive as you get into this, which is

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-01 Thread Robert Haas
On Thu, Dec 1, 2011 at 8:29 PM, Peter Geoghegan pe...@2ndquadrant.com wrote: Do your usual compile options include debug symbols? I've been using standard compile options for development of this patch, for obvious reasons. I get 36690 bytes (just under 36 KiB, or a 0.644% increase). They do,

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-01 Thread Robert Haas
On Thu, Dec 1, 2011 at 8:13 PM, Tom Lane t...@sss.pgh.pa.us wrote: What I want to see is some mechanism that pushes this out to the individual datatypes, so that both core and plug-in datatypes have access to the benefits.  There are a number of ways that could be attacked, but the most

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-01 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Thu, Dec 1, 2011 at 8:13 PM, Tom Lane t...@sss.pgh.pa.us wrote: What I want to see is some mechanism that pushes this out to the individual datatypes, so that both core and plug-in datatypes have access to the benefits. There are a number of ways

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-01 Thread Robert Haas
On Thu, Dec 1, 2011 at 9:46 PM, Tom Lane t...@sss.pgh.pa.us wrote: IMO, entries in pg_proc ought to refer to functions that are callable by the standard interface: breaking that basic contract is not going to lead anywhere pleasant.  Nor do I really want yet more columns in pg_proc. I wasn't

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-01 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Thu, Dec 1, 2011 at 9:46 PM, Tom Lane t...@sss.pgh.pa.us wrote: Nor does the register a pointer scheme you suggest make any sense to me: you still need to connect these things to catalog entries, pg_opclass entries in particular, and the

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-01 Thread Robert Haas
On Thu, Dec 1, 2011 at 10:48 PM, Tom Lane t...@sss.pgh.pa.us wrote: But you still need a lot of mechanism to do that mapping, including an initialization function that has noplace to be executed (unless every .so that defines a btree opclass has to be listed in preload_libraries), so it

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-12-01 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Thu, Dec 1, 2011 at 10:48 PM, Tom Lane t...@sss.pgh.pa.us wrote: I am thinking that the btree code, at least, would want to just unconditionally do colsortinfo-comparator(datum1, datum2, colsortinfo) so for an opclass that fails to

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-29 Thread Bruce Momjian
Peter Geoghegan wrote: Attached are the results from performing a similar process to the prior benchmark, but on Greg Smith's high-end server, and with an orderlines table that has been doubled-up until it is 1538 MB, making the same old query perform a quicksort that's over 3GB. Short

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-29 Thread Peter Geoghegan
On 29 November 2011 15:31, Bruce Momjian br...@momjian.us wrote: These are exciting advanced you are producing and I am hopeful we can get this included in Postgres 9.2. Thanks Bruce. I have mentioned already that I think parallelism is the next big Postgres challenge, and of course, one of

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-29 Thread Bruce Momjian
Peter Geoghegan wrote: On 29 November 2011 15:31, Bruce Momjian br...@momjian.us wrote: These are exciting advanced you are producing and I am hopeful we can get this included in Postgres 9.2. Thanks Bruce. I have mentioned already that I think parallelism is the next big Postgres

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-29 Thread Andres Freund
On Tuesday, November 29, 2011 07:48:37 PM Peter Geoghegan wrote: On 29 November 2011 15:31, Bruce Momjian br...@momjian.us wrote: These are exciting advanced you are producing and I am hopeful we can get this included in Postgres 9.2. Thanks Bruce. I have mentioned already that I

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-29 Thread Greg Jaskiewicz
On 28 Nov 2011, at 02:15, Peter Geoghegan wrote: Attached are the results from performing a similar process to the prior benchmark, but on Greg Smith's high-end server, and with an orderlines table that has been doubled-up until it is 1538 MB, making the same old query perform a quicksort

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-27 Thread Peter Geoghegan
Attached are the results from performing a similar process to the prior benchmark, but on Greg Smith's high-end server, and with an orderlines table that has been doubled-up until it is 1538 MB, making the same old query perform a quicksort that's over 3GB. Short version: HEAD is 20468.0ms, with

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-26 Thread Peter Geoghegan
3 items are attached: 1. A spreadsheet, results.ods, which has results for automated multiple runs of the doubled up dellstore orderlines queries (where orderlines is 385 MB). Results are sorted, with the median (or the lower middle value, didn't get the mean of the two middle runs) result for

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-24 Thread Peter Geoghegan
On 23 November 2011 19:24, Robert Haas robertmh...@gmail.com wrote: Well, right now the decision as to which mechanism should be used here gets made in tuplesort_performsort(), which has no good way of communicating with EXPLAIN anyway. You could pretty easily add something to Tuplesortstate

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-23 Thread Simon Riggs
On Fri, Nov 18, 2011 at 2:11 PM, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: On Fri, Nov 18, 2011 at 5:20 AM, Robert Haas robertmh...@gmail.com wrote: I think that we should really consider doing with this patch what Tom suggested upthread; namely, looking for

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-23 Thread Robert Haas
On Tue, Nov 22, 2011 at 8:09 PM, Peter Geoghegan pe...@2ndquadrant.com wrote: I wonder, is it worth qualifying that the Sort Method was a quicksort (fast path) sort within explain analyze output? This is a rather large improvement, so It might actually be something that people look out for, as

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-22 Thread Peter Geoghegan
On 21 November 2011 22:55, Robert Haas robertmh...@gmail.com wrote: Results on my machine, for what they're worth: I was curious as to how much of an improvement I'd made to sorting in isolation. I've adding simple sort timing instrumentation to the code in a revised patch, attached, for the

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-21 Thread Robert Haas
On Tue, Sep 20, 2011 at 7:53 PM, Peter Geoghegan pe...@2ndquadrant.com wrote: I don't think that the fact that that happens is at all significant at this early stage, and it never even occurred to me that you'd think that it might be. I was simply disclosing a quirk of this POC patch. The

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-20 Thread Peter Geoghegan
Part of my problem with not producing specialisations that I really neglected to complain about until now is the inconsistency between types, and the need to deal with that. We benefit from assuming in our specialisation that we're only dealing with POD types, simply not considering things that

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-19 Thread Peter Geoghegan
On 19 November 2011 02:55, Robert Haas robertmh...@gmail.com wrote: Maybe we should look at trying to isolate that a bit better. Indeed. Fortunately, GCC has options to disable each optimisation. Here's potentially relevant flags that we're already implicitly using at -02:

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-19 Thread Peter Geoghegan
On 20 November 2011 03:29, Peter Geoghegan pe...@2ndquadrant.com wrote: ./configure CFLAGS=-fno-inline -fno-inline-small-functions (I could have disabled more -02 optimisations, but this proved sufficient to make my point) I'll isolate this further to tuplesort.c soon, which ought to be a lot

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-18 Thread Simon Riggs
On Fri, Nov 18, 2011 at 5:20 AM, Robert Haas robertmh...@gmail.com wrote: I think that we should really consider doing with this patch what Tom suggested upthread; namely, looking for a mechanism to allow individual datatypes to offer up a comparator function that doesn't require bouncing

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-18 Thread Robert Haas
On Fri, Nov 18, 2011 at 3:53 AM, Simon Riggs si...@2ndquadrant.com wrote: We have no proof that we need to do this for 10 or 100 data types. We only currently have proof that there is gain for the most common types. Well, that's kind of my point. I think this needs more work before we decide

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-18 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes: On Fri, Nov 18, 2011 at 5:20 AM, Robert Haas robertmh...@gmail.com wrote: I think that we should really consider doing with this patch what Tom suggested upthread; namely, looking for a mechanism to allow individual datatypes to offer up a comparator

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-18 Thread Peter Geoghegan
On 18 November 2011 05:20, Robert Haas robertmh...@gmail.com wrote: I think that we should really consider doing with this patch what Tom suggested upthread; namely, looking for a mechanism to allow individual datatypes to offer up a comparator function that doesn't require bouncing through

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-18 Thread Martijn van Oosterhout
On Fri, Nov 18, 2011 at 12:20:26AM -0500, Robert Haas wrote: I think that we should really consider doing with this patch what Tom suggested upthread; namely, looking for a mechanism to allow individual datatypes to offer up a comparator function that doesn't require bouncing through

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-18 Thread Robert Haas
On Fri, Nov 18, 2011 at 4:38 PM, Peter Geoghegan pe...@2ndquadrant.com wrote: I understand that we highly value extensibility and genericity (yes, that's a real word). We may not always be well served by that tendency. True (except that genericity is not a synonym for generality AFAICT). A

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-11-17 Thread Robert Haas
On Sun, Sep 25, 2011 at 10:12 PM, Peter Geoghegan pe...@2ndquadrant.com wrote: I've produced something much neater than my first patch, attached, although I still consider this to be at the POC stage, not least since I'm not exactly sure how I should be selecting the right specialisation in

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-10-05 Thread Robert Haas
On Tue, Oct 4, 2011 at 10:55 PM, Greg Stark st...@mit.edu wrote: I agree that if we wanted to farm out entire plan nodes we would probably end up generating new partial plans which would be handed to other backend processes. That's not unlike what Oracle did with Parallel Query. But i'm a bit

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-10-05 Thread Bruce Momjian
Robert Haas wrote: Markus Wanner took a crack at generalizing the autovacuum machinery that we have now into something that could be used to fire up general-purpose worker processes, but it fell down mostly because I (and, I think, others) weren't convinced that imessages were something we

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-10-04 Thread Bruce Momjian
Peter Geoghegan wrote: On the subject of highly ambitious optimisations to sorting, one possibility I consider much more practicable than GPU-accelerated sorting is simple threading; quicksort can be parallelised very effectively, due to its divide-and-conquer nature. If we could agree on a

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-10-04 Thread Greg Stark
On Wed, Oct 5, 2011 at 2:49 AM, Bruce Momjian br...@momjian.us wrote: Rather than parallelizing the entire backend, I imagine adding threading or helper processes for things like sorts, index scans, executor nodes, and stored procedure languages.  I expect final code to be 2-3 years in the

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-26 Thread Peter Geoghegan
On 26 September 2011 04:46, Robert Haas robertmh...@gmail.com wrote: I don't want to take time to review this in detail right now, because I don't think it would be fair to put this ahead of things that were submitted for the current CommitFest, but I'm impressed. Thank you. Now that I think

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-25 Thread Robert Haas
On Sun, Sep 25, 2011 at 10:12 PM, Peter Geoghegan pe...@2ndquadrant.com wrote: [ new results ] Nice results. I think these are far more convincing than the last set, because (1) the gains are bigger and (2) they survive -O2 and (3) you tested an actual query, not just qsort() itself. I don't

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Heikki Linnakangas
On 21.09.2011 02:53, Peter Geoghegan wrote: C stdlib quick-sort time elapsed: 2.092451 seconds Inline quick-sort time elapsed: 1.587651 seconds Does *that* look attractive to you? Not really, to be honest. That's a 25% speedup in pure qsorting speed. How much of a gain in a real query do you

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Simon Riggs
On Tue, Sep 20, 2011 at 3:51 AM, Tom Lane t...@sss.pgh.pa.us wrote: This performance patch differs from most in that it's difficult in principle to imagine a performance regression occurring. Really?  N copies of the same code could lead to performance loss just due to code bloat (ie, less

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Simon Riggs
On Wed, Sep 21, 2011 at 7:51 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 21.09.2011 02:53, Peter Geoghegan wrote: C stdlib quick-sort time elapsed: 2.092451 seconds Inline quick-sort time elapsed: 1.587651 seconds Does *that* look attractive to you? Not really, to

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Heikki Linnakangas
On 21.09.2011 10:01, Simon Riggs wrote: On Wed, Sep 21, 2011 at 7:51 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 21.09.2011 02:53, Peter Geoghegan wrote: C stdlib quick-sort time elapsed: 2.092451 seconds Inline quick-sort time elapsed: 1.587651 seconds Does *that*

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Peter Geoghegan
On 21 September 2011 01:48, karave...@mail.bg wrote: All -O2 version show 42% speedup with inlined qsort. -O0 showed 25% speedup. Thanks. Looks like the figures I posted last night were fairly conservative. Does anyone else care to report results? -- Peter Geoghegan      

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Amit Kapila
Recent discussions on the threads Double sorting split patch and CUDA sorting raised the possibility that there could be significant performance optimisation low-hanging fruit picked by having the executor treat integers and floats as a special case during sorting, avoiding going to the

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Greg Stark
On Wed, Sep 21, 2011 at 8:08 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: How about almost every primary index creation? Nope. Swamped by everything else. Really? I think it's pretty common for shops to be able to dedicate large amounts of RAM to building initial indexes

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Robert Haas
On Wed, Sep 21, 2011 at 8:47 AM, Greg Stark st...@mit.edu wrote: On Wed, Sep 21, 2011 at 8:08 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: How about almost every primary index creation? Nope. Swamped by everything else. Really? I think it's pretty common for shops to be

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Peter Geoghegan
On 21 September 2011 07:51, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 21.09.2011 02:53, Peter Geoghegan wrote: C stdlib quick-sort time elapsed: 2.092451 seconds Inline quick-sort time elapsed: 1.587651 seconds Does *that* look attractive to you? Not really, to be

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Heikki Linnakangas
On 21.09.2011 17:20, Peter Geoghegan wrote: Even still, I think that the 12.5% figure is pretty pessimistic - I've already sped up the dell store query by almost that much, and that's with a patch that was, due to circumstances, cobbled together. I'm not against making things faster, it's just

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Tom Lane
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: On 21.09.2011 17:20, Peter Geoghegan wrote: Even still, I think that the 12.5% figure is pretty pessimistic - I've already sped up the dell store query by almost that much, and that's with a patch that was, due to circumstances,

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Andrew Dunstan
On 09/21/2011 10:50 AM, Tom Lane wrote: The other question that I'm going to be asking is whether it's not possible to get most of the same improvement with a much smaller code footprint. I continue to suspect that getting rid of the SQL function impedance-match layer (myFunctionCall2Coll

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes: This is a marvellous win, a huge gain from a small, isolated and easily tested change. By far the smallest amount of additional code to sorting we will have added and yet one of the best gains. I think you forgot your cheerleader uniform. A patch

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Peter Geoghegan
On 21 September 2011 15:50, Tom Lane t...@sss.pgh.pa.us wrote: Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: I'm not against making things faster, it's just that I haven't seen solid evidence yet that this will help. Just provide a best-case test case for this that shows a

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: On 09/21/2011 10:50 AM, Tom Lane wrote: The other question that I'm going to be asking is whether it's not possible to get most of the same improvement with a much smaller code footprint. I continue to suspect that getting rid of the SQL function

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Heikki Linnakangas
On 21.09.2011 18:46, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: On 09/21/2011 10:50 AM, Tom Lane wrote: The other question that I'm going to be asking is whether it's not possible to get most of the same improvement with a much smaller code footprint. I continue to suspect

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Greg Stark
On Wed, Sep 21, 2011 at 4:46 PM, Tom Lane t...@sss.pgh.pa.us wrote:  As such, they could not have entries in pg_proc, so it seems like there's no ready way to represent them in the catalogs. Why couldn't they be in pg_proc with a bunch of opaque arguments like the GIST opclass support

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Tom Lane
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: On 21.09.2011 18:46, Tom Lane wrote: The idea that I was toying with was to allow the regular SQL-callable comparison function to somehow return a function pointer to the alternate comparison function, You could have a new

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Heikki Linnakangas
On 21.09.2011 18:46, Tom Lane wrote: Well, we'd have to negotiate what the API ought to be. What I'm envisioning is that datatypes could provide alternate comparison functions that are designed to be qsort-callable rather than SQL-callable. As such, they could not have entries in pg_proc, so

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Tom Lane
Greg Stark st...@mit.edu writes: On Wed, Sep 21, 2011 at 4:46 PM, Tom Lane t...@sss.pgh.pa.us wrote:  As such, they could not have entries in pg_proc, so it seems like there's no ready way to represent them in the catalogs. Why couldn't they be in pg_proc with a bunch of opaque arguments like

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Tom Lane
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: On 21.09.2011 18:46, Tom Lane wrote: Well, we'd have to negotiate what the API ought to be. What I'm envisioning is that datatypes could provide alternate comparison functions that are designed to be qsort-callable rather than

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Greg Stark
On Wed, Sep 21, 2011 at 5:23 PM, Tom Lane t...@sss.pgh.pa.us wrote: None of that stuff is inlinable or constant-foldable today, nor would it be with the patch that Peter was proposing AFAICS, because none of the flags will ever be compile time constant values. I was referring to

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-21 Thread Simon Riggs
On Wed, Sep 21, 2011 at 4:22 PM, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: This is a marvellous win, a huge gain from a small, isolated and easily tested change. By far the smallest amount of additional code to sorting we will have added and yet one of the

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-20 Thread Peter Geoghegan
On 20 September 2011 03:51, Tom Lane t...@sss.pgh.pa.us wrote: Considering that -O2 is our standard optimization level, that observation seems to translate to this patch will be useless in practice.  I think you had better investigate that aspect in some detail before spending more effort. I

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-20 Thread karavelov
- Цитат от Peter Geoghegan (pe...@2ndquadrant.com), на 21.09.2011 в 02:53 - On 20 September 2011 03:51, Tom Lane t...@sss.pgh.pa.us wrote: Considering that -O2 is our standard optimization level, that observation seems to translate to this patch will be useless in practice.  I think

Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-19 Thread Tom Lane
Peter Geoghegan pe...@2ndquadrant.com writes: Once the cache has been warmed, explain analyze very consistently reports a runtime of 123ms for this query on master/HEAD, which varies +/- 1 ms, with a few outliers of maybe +/- 2ms. However, when I apply this patch, that goes down to 107ms +/-