Re: [HACKERS] [PATCH] Incremental sort
Alexander Korotkov <a.korotkov@postgrespro.ru>
On Wed, Nov 22, 2017 at 12:01 AM, Thomas Munro < thomas.munro@enterprisedb.com> wrote: > >> I gather that you have > >> determined empirically that it's better to be able to sort groups of > >> at least MIN_GROUP_SIZE than to be able to skip the comparisons on the > >> leading attributes, but why is that the case? > > > > Right. The issue that not only case of one tuple per group could cause > > overhead, but few tuples (like 2 or 3) is also case of overhead. Also, > > overhead is related not only to sorting. While investigate of regression > > case provided by Heikki [1], I've seen extra time spent mostly in extra > > copying of sample tuple and comparison with that. In order to cope this > > overhead I've introduced MIN_GROUP_SIZE which allows to skip copying > sample > > tuples too frequently. > > I see. I wonder if there could ever be a function like > ExecMoveTuple(dst, src). Given the polymorphism involved it'd be > slightly complicated and you'd probably have a general case that just > copies the tuple to dst and clears src, but there might be a bunch of > cases where you can do something more efficient like moving a pointer > and pin ownership. I haven't really thought that through and > there may be fundamental problems with it... > ExecMoveTuple(dst, src) would be good. But, it would be hard to implement "moving a pointer and pin ownership" principle in our current infrastructure. It's because source and destination can have different memory contexts. AFAICS, we can't just move memory area between memory contexts: we have to allocate new area, then memcpy, and then deallocate old area. > If you're going to push the tuples into the sorter every time, then I > guess there are some special cases that could allow future > optimisations: (1) if you noticed that every prefix was different, you > can skip the sort operation (that is, you can use the sorter as a dumb > tuplestore and just get the tuples out in the same order you put them > in; not sure if Tuplesort supports that but it presumably could), In order to notice that every prefix is different, I have to compare every prefix. But that may introduce an overhead. So, there reason why I introduced MIN_GROUP_SIZE is exactly to not compare every prefix... > (2) > if you noticed that every prefix was the same (that is, you have only > one prefix/group in the sorter) then you could sort only on the suffix > (that is, you could somehow tell Tuplesort to ignore the leading > columns), Yes, I did so before. But again, after introducing MIN_GROUP_SIZE, I missed knowledge whether all the prefixes were the same or different. This is why, I've to sort by full column list for now... (3) as a more complicated optimisation for intermediate > group sizes 1 < n < MIN_GROUP_SIZE, you could somehow number the > groups with an integer that increments whenever you see the prefix > change, and somehow tell tuplesort.c to use that instead of the > leading columns. That is interesting idea. The reason we have an overhead in comparison with plain sort is that we do extra comparison (and copying), but knowledge of this comparison result is lost for sorting itself. Thus, sorting can "reuse" prefix comparison, and overhead would be lower. But the problem is that we have to reformat tuples before putting them into tuplesort. I wonder if tuple reformatting could eat potential performance win... ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Commits
-
Further adjustments to Hashagg EXPLAIN ANALYZE output
- 40efbf8706cd 14.0 cited
-
Rework EXPLAIN format for incremental sort
- 6a918c3ac8a6 13.0 landed
-
Fix typos and improve incremental sort comments
- 1a40d37a9faf 13.0 landed
-
Stabilize incremental_sort tests
- cea09246e578 13.0 landed
-
Minor improvements in Incremental Sort explain
- d22782a5392f 13.0 landed
-
Consider Incremental Sort paths at additional places
- ba3e76cc571e 13.0 landed
-
Fix representation of SORT_TYPE_STILL_IN_PROGRESS.
- c7654f6a3779 13.0 landed
-
Fix failures in incremental_sort due to number of workers
- 23ba3b5ee278 13.0 landed
-
Fix show_incremental_sort_info with force_parallel_mode
- 7d6d82a52493 13.0 landed
-
Implement Incremental Sort
- d2d8a229bc58 13.0 landed
-
Fix handling of "Subplans Removed" field in EXPLAIN output.
- 7d91b604d9b5 13.0 cited
-
Fix EXPLAIN (SETTINGS) to follow policy about when to print empty fields.
- 3ec20c7091e9 13.0 cited
-
Ensure plpgsql result tuples have the right composite type marking.
- 5683b34956b4 13.0 cited
-
Propagate sort instrumentation from workers back to leader.
- bf11e7ee2e36 11.0 cited
-
Make new regression test case parallel-safe, and improve its output.
- 1177ab1dabf7 11.0 cited
-
Push limit through subqueries to underlying sort, where possible.
- 1f6d515a67ec 11.0 cited
-
Fix inappropriate printing of never-measured times in EXPLAIN.
- 4b234fd8bf21 9.6.0 cited
-
Fix some infelicities in EXPLAIN output for parallel query plans.
- 8ebb69f85445 9.6.0 cited