From 71da5c3d406efe9a11dd9cf73646d7dc7e510122 Mon Sep 17 00:00:00 2001 From: James Coleman Date: Sat, 28 Mar 2020 22:35:49 -0400 Subject: [PATCH v44 7/7] explain fixes --- src/backend/commands/explain.c | 70 ++++++++++++---------- src/backend/executor/nodeIncrementalSort.c | 34 ++++++----- src/include/nodes/execnodes.h | 2 +- src/include/utils/tuplesort.h | 11 ++-- 4 files changed, 65 insertions(+), 52 deletions(-) diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 39d51848b6..24acde506e 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -2713,26 +2713,41 @@ show_sort_info(SortState *sortstate, ExplainState *es) */ static void show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo, - const char *groupLabel, ExplainState *es) + const char *groupLabel, bool indent, ExplainState *es) { ListCell *methodCell; - int methodCount = list_length(groupInfo->sortMethods); + List *methodNames = NIL; + + /* Generate a list of sort methods used across all groups. */ + for (int bit = 0; bit < sizeof(Size); ++bit) + { + if (groupInfo->sortMethods & (1 << bit)) + { + TuplesortMethod sortMethod = (1 << bit); + const char *methodName; + + methodName = tuplesort_method_name(sortMethod); + methodNames = lappend(methodNames, unconstify(char *, methodName)); + } + } if (es->format == EXPLAIN_FORMAT_TEXT) { - appendStringInfoSpaces(es->str, es->indent * 2); - appendStringInfo(es->str, "%s Groups: %ld (Methods: ", groupLabel, + if (indent) + appendStringInfoSpaces(es->str, es->indent * 2); + appendStringInfo(es->str, "%s Groups: %ld Sort Method", groupLabel, groupInfo->groupCount); - foreach(methodCell, groupInfo->sortMethods) + /* plural/singular based on methodNames size */ + if (list_length(methodNames) > 1) + appendStringInfo(es->str, "s: "); + else + appendStringInfo(es->str, ": "); + foreach(methodCell, methodNames) { - const char *sortMethodName; - - sortMethodName = tuplesort_method_name(methodCell->int_value); - appendStringInfo(es->str, "%s", sortMethodName); - if (foreach_current_index(methodCell) < methodCount - 1) + appendStringInfo(es->str, "%s", (char *) methodCell->ptr_value); + if (foreach_current_index(methodCell) < list_length(methodNames) - 1) appendStringInfo(es->str, ", "); } - appendStringInfo(es->str, ")"); if (groupInfo->maxMemorySpaceUsed > 0) { @@ -2740,7 +2755,7 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo, const char *spaceTypeName; spaceTypeName = tuplesort_space_type_name(SORT_SPACE_TYPE_MEMORY); - appendStringInfo(es->str, " %s: %ldkB (avg), %ldkB (max)", + appendStringInfo(es->str, " %s: avg=%ldkB peak=%ldkB", spaceTypeName, avgSpace, groupInfo->maxMemorySpaceUsed); } @@ -2755,7 +2770,7 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo, /* Add a semicolon separator only if memory stats were printed. */ if (groupInfo->maxMemorySpaceUsed > 0) appendStringInfo(es->str, ";"); - appendStringInfo(es->str, " %s: %ldkB (avg), %ldkB (max)", + appendStringInfo(es->str, " %s: avg=%ldkB peak=%ldkB", spaceTypeName, avgSpace, groupInfo->maxDiskSpaceUsed); } @@ -2764,7 +2779,6 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo, } else { - List *methodNames = NIL; StringInfoData groupName; initStringInfo(&groupName); @@ -2772,12 +2786,6 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo, ExplainOpenGroup("Incremental Sort Groups", groupName.data, true, es); ExplainPropertyInteger("Group Count", NULL, groupInfo->groupCount, es); - foreach(methodCell, groupInfo->sortMethods) - { - const char *sortMethodName = tuplesort_method_name(methodCell->int_value); - - methodNames = lappend(methodNames, unconstify(char *, sortMethodName)); - } ExplainPropertyList("Sort Methods Used", methodNames, es); if (groupInfo->maxMemorySpaceUsed > 0) @@ -2834,10 +2842,10 @@ show_incremental_sort_info(IncrementalSortState *incrsortstate, if (!(es->analyze && fullsortGroupInfo->groupCount > 0)) return; - show_incremental_sort_group_info(fullsortGroupInfo, "Full-sort", es); + show_incremental_sort_group_info(fullsortGroupInfo, "Full-sort", true, es); prefixsortGroupInfo = &incrsortstate->incsort_info.prefixsortGroupInfo; if (prefixsortGroupInfo->groupCount > 0) - show_incremental_sort_group_info(prefixsortGroupInfo, "Presorted", es); + show_incremental_sort_group_info(prefixsortGroupInfo, "Presorted", true, es); if (incrsortstate->shared_info != NULL) { @@ -2860,20 +2868,18 @@ show_incremental_sort_info(IncrementalSortState *incrsortstate, prefixsortGroupInfo->groupCount == 0) continue; - if (!opened_group) - { - ExplainOpenGroup("Workers", "Workers", false, es); - opened_group = true; - } + if (es->workers_state) + ExplainOpenWorker(n, es); if (fullsortGroupInfo->groupCount > 0) - show_incremental_sort_group_info(fullsortGroupInfo, "Full-sort", es); + show_incremental_sort_group_info(fullsortGroupInfo, "Full-sort", + es->workers_state == NULL, es); if (prefixsortGroupInfo->groupCount > 0) - show_incremental_sort_group_info(prefixsortGroupInfo, "Presorted", es); - } + show_incremental_sort_group_info(prefixsortGroupInfo, "Presorted", true, es); - if (opened_group) - ExplainCloseGroup("Workers", "Workers", false, es); + if (es->workers_state) + ExplainCloseWorker(n, es); + } } } diff --git a/src/backend/executor/nodeIncrementalSort.c b/src/backend/executor/nodeIncrementalSort.c index 9fe93d5979..004165ccc1 100644 --- a/src/backend/executor/nodeIncrementalSort.c +++ b/src/backend/executor/nodeIncrementalSort.c @@ -100,6 +100,22 @@ instrumentSortedGroup(PlanState *pstate, IncrementalSortGroupInfo *groupInfo, IncrementalSortState *node = castNode(IncrementalSortState, pstate); TuplesortInstrumentation sort_instr; + /* Record shared stats if we're a parallel worker. */ + if (node->shared_info && node->am_worker) + { + Assert(IsParallelWorker()); + Assert(ParallelWorkerNumber <= node->shared_info->num_workers); + + /* + * XXX This is a bit a gross, but it would take inventing some new + * enum or flag to this method to make it not necessary... + */ + if (groupInfo == &node->incsort_info.fullsortGroupInfo) + groupInfo = &node->shared_info->sinfo[ParallelWorkerNumber].fullsortGroupInfo; + else if (groupInfo == &node->incsort_info.prefixsortGroupInfo) + groupInfo = &node->shared_info->sinfo[ParallelWorkerNumber].prefixsortGroupInfo; + } + groupInfo->groupCount++; tuplesort_get_stats(sortState, &sort_instr); @@ -122,19 +138,7 @@ instrumentSortedGroup(PlanState *pstate, IncrementalSortGroupInfo *groupInfo, } /* Track each sort method we've used. */ - if (!list_member_int(groupInfo->sortMethods, sort_instr.sortMethod)) - groupInfo->sortMethods = lappend_int(groupInfo->sortMethods, - sort_instr.sortMethod); - - /* Record shared stats if we're a parallel worker. */ - if (node->shared_info && node->am_worker) - { - Assert(IsParallelWorker()); - Assert(ParallelWorkerNumber <= node->shared_info->num_workers); - - memcpy(&node->shared_info->sinfo[ParallelWorkerNumber], - &node->incsort_info, sizeof(IncrementalSortInfo)); - } + groupInfo->sortMethods |= sort_instr.sortMethod; } /* ---------------------------------------------------------------- @@ -1026,13 +1030,13 @@ ExecInitIncrementalSort(IncrementalSort *node, EState *estate, int eflags) fullsortGroupInfo->totalDiskSpaceUsed = 0; fullsortGroupInfo->maxMemorySpaceUsed = 0; fullsortGroupInfo->totalMemorySpaceUsed = 0; - fullsortGroupInfo->sortMethods = NIL; + fullsortGroupInfo->sortMethods = 0; prefixsortGroupInfo->groupCount = 0; prefixsortGroupInfo->maxDiskSpaceUsed = 0; prefixsortGroupInfo->totalDiskSpaceUsed = 0; prefixsortGroupInfo->maxMemorySpaceUsed = 0; prefixsortGroupInfo->totalMemorySpaceUsed = 0; - prefixsortGroupInfo->sortMethods = NIL; + prefixsortGroupInfo->sortMethods = 0; } /* diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 6127ab5912..8d1b944472 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -2034,7 +2034,7 @@ typedef struct IncrementalSortGroupInfo long totalDiskSpaceUsed; long maxMemorySpaceUsed; long totalMemorySpaceUsed; - List *sortMethods; + Size sortMethods; /* bitmask of TuplesortMethod */ } IncrementalSortGroupInfo; typedef struct IncrementalSortInfo diff --git a/src/include/utils/tuplesort.h b/src/include/utils/tuplesort.h index 0e9ab4e586..96e970339c 100644 --- a/src/include/utils/tuplesort.h +++ b/src/include/utils/tuplesort.h @@ -61,14 +61,17 @@ typedef struct SortCoordinateData *SortCoordinate; * Data structures for reporting sort statistics. Note that * TuplesortInstrumentation can't contain any pointers because we * sometimes put it in shared memory. + * + * TuplesortMethod is used in a bitmask in Increment Sort's shared memory + * instrumentation so needs to have each value be a separate bit. */ typedef enum { SORT_TYPE_STILL_IN_PROGRESS = 0, - SORT_TYPE_TOP_N_HEAPSORT, - SORT_TYPE_QUICKSORT, - SORT_TYPE_EXTERNAL_SORT, - SORT_TYPE_EXTERNAL_MERGE + SORT_TYPE_TOP_N_HEAPSORT = 2, + SORT_TYPE_QUICKSORT = 4, + SORT_TYPE_EXTERNAL_SORT = 8, + SORT_TYPE_EXTERNAL_MERGE = 16 } TuplesortMethod; typedef enum -- 2.17.1