-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Improvement](sort) Free sort blocks if this block is exhausted #39306
Conversation
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
} else { | ||
RETURN_IF_ERROR(partition_sort_read(block, eos, state->batch_size())); | ||
} | ||
RETURN_IF_ERROR(partition_sort_read(block, eos, state->batch_size())); | ||
} | ||
return Status::OK(); | ||
} | ||
|
||
Status PartitionSorter::partition_sort_read(Block* output_block, bool* eos, int batch_size) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: function 'partition_sort_read' has cognitive complexity of 55 (threshold 50) [readability-function-cognitive-complexity]
Status PartitionSorter::partition_sort_read(Block* output_block, bool* eos, int batch_size) {
^
Additional context
be/src/vec/common/sort/partition_sorter.cpp:102: +1, including nesting penalty of 0, nesting level increased to 1
if (priority_queue.empty()) {
^
be/src/vec/common/sort/partition_sorter.cpp:114: +1, including nesting penalty of 0, nesting level increased to 1
while (!priority_queue.empty()) {
^
be/src/vec/common/sort/partition_sorter.cpp:117: +2, including nesting penalty of 1, nesting level increased to 2
if (UNLIKELY(_previous_row->impl == nullptr)) {
^
be/src/vec/common/sort/partition_sorter.cpp:121: +2, including nesting penalty of 1, nesting level increased to 2
switch (_top_n_algorithm) {
^
be/src/vec/common/sort/partition_sorter.cpp:124: +3, including nesting penalty of 2, nesting level increased to 3
if ((current_output_rows + _output_total_rows) < _partition_inner_limit) {
^
be/src/vec/common/sort/partition_sorter.cpp:125: +4, including nesting penalty of 3, nesting level increased to 4
for (size_t i = 0; i < num_columns; ++i) {
^
be/src/vec/common/sort/partition_sorter.cpp:128: +1, nesting level increased to 3
} else {
^
be/src/vec/common/sort/partition_sorter.cpp:140: +3, including nesting penalty of 2, nesting level increased to 3
if (_has_global_limit &&
^
be/src/vec/common/sort/partition_sorter.cpp:140: +1
if (_has_global_limit &&
^
be/src/vec/common/sort/partition_sorter.cpp:145: +3, including nesting penalty of 2, nesting level increased to 3
if (_has_global_limit) {
^
be/src/vec/common/sort/partition_sorter.cpp:147: +1, nesting level increased to 3
} else {
^
be/src/vec/common/sort/partition_sorter.cpp:150: +4, including nesting penalty of 3, nesting level increased to 4
if (cmp_res == false) {
^
be/src/vec/common/sort/partition_sorter.cpp:152: +5, including nesting penalty of 4, nesting level increased to 5
if (_output_distinct_rows >= _partition_inner_limit) {
^
be/src/vec/common/sort/partition_sorter.cpp:159: +3, including nesting penalty of 2, nesting level increased to 3
for (size_t i = 0; i < num_columns; ++i) {
^
be/src/vec/common/sort/partition_sorter.cpp:169: +3, including nesting penalty of 2, nesting level increased to 3
if (_has_global_limit &&
^
be/src/vec/common/sort/partition_sorter.cpp:169: +1
if (_has_global_limit &&
^
be/src/vec/common/sort/partition_sorter.cpp:176: +3, including nesting penalty of 2, nesting level increased to 3
if (cmp_res == false) {
^
be/src/vec/common/sort/partition_sorter.cpp:178: +4, including nesting penalty of 3, nesting level increased to 4
if ((current_output_rows + _output_total_rows) >= _partition_inner_limit) {
^
be/src/vec/common/sort/partition_sorter.cpp:184: +3, including nesting penalty of 2, nesting level increased to 3
for (size_t i = 0; i < num_columns; ++i) {
^
be/src/vec/common/sort/partition_sorter.cpp:194: +2, including nesting penalty of 1, nesting level increased to 2
if (!current->is_last()) {
^
be/src/vec/common/sort/partition_sorter.cpp:199: +2, including nesting penalty of 1, nesting level increased to 2
if (current_output_rows == batch_size || get_enough_data == true) {
^
be/src/vec/common/sort/partition_sorter.cpp:199: +1
if (current_output_rows == batch_size || get_enough_data == true) {
^
be/src/vec/common/sort/partition_sorter.cpp:205: +1, including nesting penalty of 0, nesting level increased to 1
if (current_output_rows == 0 || get_enough_data == true) {
^
be/src/vec/common/sort/partition_sorter.cpp:205: +1
if (current_output_rows == 0 || get_enough_data == true) {
^
run buildall |
run buildall |
TPC-H: Total hot run time: 40202 ms
|
TPC-DS: Total hot run time: 185209 ms
|
ClickBench: Total hot run time: 31.1 s
|
run buildall |
TPC-H: Total hot run time: 37515 ms
|
TPC-DS: Total hot run time: 189880 ms
|
ClickBench: Total hot run time: 30.85 s
|
run buildall |
run buildall |
TPC-H: Total hot run time: 37459 ms
|
TPC-DS: Total hot run time: 189039 ms
|
ClickBench: Total hot run time: 31.1 s
|
run buildall |
TPC-H: Total hot run time: 37797 ms
|
run buildall |
TPC-H: Total hot run time: 37718 ms
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
} else { | ||
RETURN_IF_ERROR(partition_sort_read(block, eos, state->batch_size())); | ||
} | ||
RETURN_IF_ERROR(partition_sort_read(block, eos, state->batch_size())); | ||
} | ||
return Status::OK(); | ||
} | ||
|
||
Status PartitionSorter::partition_sort_read(Block* output_block, bool* eos, int batch_size) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: function 'partition_sort_read' has cognitive complexity of 55 (threshold 50) [readability-function-cognitive-complexity]
Status PartitionSorter::partition_sort_read(Block* output_block, bool* eos, int batch_size) {
^
Additional context
be/src/vec/common/sort/partition_sorter.cpp:97: +1, including nesting penalty of 0, nesting level increased to 1
if (priority_queue.empty()) {
^
be/src/vec/common/sort/partition_sorter.cpp:109: +1, including nesting penalty of 0, nesting level increased to 1
while (!priority_queue.empty()) {
^
be/src/vec/common/sort/partition_sorter.cpp:112: +2, including nesting penalty of 1, nesting level increased to 2
if (UNLIKELY(_previous_row->impl == nullptr)) {
^
be/src/vec/common/sort/partition_sorter.cpp:116: +2, including nesting penalty of 1, nesting level increased to 2
switch (_top_n_algorithm) {
^
be/src/vec/common/sort/partition_sorter.cpp:119: +3, including nesting penalty of 2, nesting level increased to 3
if ((current_output_rows + _output_total_rows) < _partition_inner_limit) {
^
be/src/vec/common/sort/partition_sorter.cpp:120: +4, including nesting penalty of 3, nesting level increased to 4
for (size_t i = 0; i < num_columns; ++i) {
^
be/src/vec/common/sort/partition_sorter.cpp:123: +1, nesting level increased to 3
} else {
^
be/src/vec/common/sort/partition_sorter.cpp:135: +3, including nesting penalty of 2, nesting level increased to 3
if (_has_global_limit &&
^
be/src/vec/common/sort/partition_sorter.cpp:135: +1
if (_has_global_limit &&
^
be/src/vec/common/sort/partition_sorter.cpp:140: +3, including nesting penalty of 2, nesting level increased to 3
if (_has_global_limit) {
^
be/src/vec/common/sort/partition_sorter.cpp:142: +1, nesting level increased to 3
} else {
^
be/src/vec/common/sort/partition_sorter.cpp:145: +4, including nesting penalty of 3, nesting level increased to 4
if (cmp_res == false) {
^
be/src/vec/common/sort/partition_sorter.cpp:147: +5, including nesting penalty of 4, nesting level increased to 5
if (_output_distinct_rows >= _partition_inner_limit) {
^
be/src/vec/common/sort/partition_sorter.cpp:154: +3, including nesting penalty of 2, nesting level increased to 3
for (size_t i = 0; i < num_columns; ++i) {
^
be/src/vec/common/sort/partition_sorter.cpp:164: +3, including nesting penalty of 2, nesting level increased to 3
if (_has_global_limit &&
^
be/src/vec/common/sort/partition_sorter.cpp:164: +1
if (_has_global_limit &&
^
be/src/vec/common/sort/partition_sorter.cpp:171: +3, including nesting penalty of 2, nesting level increased to 3
if (cmp_res == false) {
^
be/src/vec/common/sort/partition_sorter.cpp:173: +4, including nesting penalty of 3, nesting level increased to 4
if ((current_output_rows + _output_total_rows) >= _partition_inner_limit) {
^
be/src/vec/common/sort/partition_sorter.cpp:179: +3, including nesting penalty of 2, nesting level increased to 3
for (size_t i = 0; i < num_columns; ++i) {
^
be/src/vec/common/sort/partition_sorter.cpp:189: +2, including nesting penalty of 1, nesting level increased to 2
if (!current->is_last()) {
^
be/src/vec/common/sort/partition_sorter.cpp:194: +2, including nesting penalty of 1, nesting level increased to 2
if (current_output_rows == batch_size || get_enough_data == true) {
^
be/src/vec/common/sort/partition_sorter.cpp:194: +1
if (current_output_rows == batch_size || get_enough_data == true) {
^
be/src/vec/common/sort/partition_sorter.cpp:200: +1, including nesting penalty of 0, nesting level increased to 1
if (current_output_rows == 0 || get_enough_data == true) {
^
be/src/vec/common/sort/partition_sorter.cpp:200: +1
if (current_output_rows == 0 || get_enough_data == true) {
^
TPC-H: Total hot run time: 38323 ms
|
TPC-H: Total hot run time: 38347 ms
|
run buildall |
TPC-H: Total hot run time: 37790 ms
|
TPC-DS: Total hot run time: 195787 ms
|
ClickBench: Total hot run time: 31.64 s
|
run buildall |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
TPC-H: Total hot run time: 38100 ms
|
TPC-H: Total hot run time: 38081 ms
|
TPC-H: Total hot run time: 37782 ms
|
TPC-H: Total hot run time: 38229 ms
|
… exhausted (apache#39306)" (apache#40211)" This reverts commit 1768169.
Proposed changes
Issue Number: close #xxx