Re: Inlining comparators as a performance optimisation
От | Robert Haas |
---|---|
Тема | Re: Inlining comparators as a performance optimisation |
Дата | |
Msg-id | CA+TgmoboMXiqDe9uKJvy8aQ-s+eak+5aFsaYsk61=3DGROeqLw@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Inlining comparators as a performance optimisation (Peter Geoghegan <peter@2ndquadrant.com>) |
Ответы |
Re: Inlining comparators as a performance optimisation
(Peter Geoghegan <peter@2ndquadrant.com>)
Re: Inlining comparators as a performance optimisation (Peter Geoghegan <peter@2ndquadrant.com>) |
Список | pgsql-hackers |
On Tue, Nov 22, 2011 at 8:09 PM, Peter Geoghegan <peter@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 it might be tricky to remember the exact > circumstances under which the optimisation kicks in by the time we're > done here. 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. Actually, I think that's a modularity violation; using the address of comparetup_heap as a flag value seems quite ugly. How about moving that logic up to tuplesort_begin_heap() and having it set some state inside the Tuplesort, maybe based on a flag in the opclass (or would it have to attach to the individual operator)? At least on my machine, your latest patch reliably crashes the regression tests in multiple places. The following test case also crashes them for me (perhaps for the same reason the regression tests crash): create table i4 (a int, b int); insert into i4 values (4, 1), (2, 1), (0, 1), (null, 1), (-2, 1), (-7, 1), (4, 2), (4, 3), (4, 4); select * from i4 order by 1, 2; TRAP: FailedAssertion("!(state->nKeys == 1)", File: "tuplesort.c", Line: 1261); The formatting of src/include/utils/template_qsort_arg.h is hard to read. At ts=8, the backslashes line up, but the code doesn't fit in 80 columns. If you set ts=4, then it fits in 80 columns, but the backslashes don't line up any more, and the variable declarations don't either. I believe ts=4 is project standard. I still think it would be a good idea to provide a mechanism to override heap_comparetup() with a type-specific function. I don't think that would take much extra code, and then any data type could get at least that much benefit out of this. It seems like it could be a good idea to do some per-assembler-instruction profiling of this code, and perhaps also of the original code. I'm curious where the time is being spent. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
В списке pgsql-hackers по дате отправления: