You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/01/13 12:07:35 UTC
[GitHub] [incubator-doris] zuochunwei opened a new pull request #7751: [vectorized](improving) (exec) optimize VDataStreamSender's send() performance #7747
zuochunwei opened a new pull request #7751:
URL: https://github.com/apache/incubator-doris/pull/7751
# Proposed changes
gather channel's rows, and then send together
handle each column's different rows at a time is good for improving cache hit
comparing test data, wait @zenoyang to provide
Issue Number: close #xxx
## Problem Summary:
Describe the overview of changes.
## Checklist(Required)
1. Does it affect the original behavior: (Yes/No/I Don't know)
2. Has unit tests been added: (Yes/No/No Need)
3. Has document been added or modified: (Yes/No/No Need)
4. Does it need to update dependencies: (Yes/No)
5. Are there any changes that cannot be rolled back: (Yes/No)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] zuochunwei commented on a change in pull request #7751: [vectorized](improving) (exec) optimize VDataStreamSender's send() performance #7747
Posted by GitBox <gi...@apache.org>.
zuochunwei commented on a change in pull request #7751:
URL: https://github.com/apache/incubator-doris/pull/7751#discussion_r784496185
##########
File path: be/src/vec/core/block.cpp
##########
@@ -843,6 +843,17 @@ void MutableBlock::add_row(const Block* block, int row) {
}
}
+void MutableBlock::add_rows(const Block* block, const int* row_begin, const int* row_end) {
+ auto& src_columns_with_schema = block->get_columns_with_type_and_name();
+ for (size_t i = 0; i < _columns.size(); ++i) {
+ auto& dst = _columns[i];
+ auto& src = *src_columns_with_schema[i].column.get();
+ for (const int* row = row_begin; row != row_end; ++row) {
+ dst->insert_from(src, *row);
Review comment:
yes, that 's what i will todo next
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] HappenLee merged pull request #7751: [vectorized](improving) (exec) optimize VDataStreamSender's send() performance #7747
Posted by GitBox <gi...@apache.org>.
HappenLee merged pull request #7751:
URL: https://github.com/apache/incubator-doris/pull/7751
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] zenoyang commented on pull request #7751: [vectorized](improving) (exec) optimize VDataStreamSender's send() performance #7747
Posted by GitBox <gi...@apache.org>.
zenoyang commented on pull request #7751:
URL: https://github.com/apache/incubator-doris/pull/7751#issuecomment-1012165597
| | before PR (s) | after PR (s) |
| ---- | ---------------- | ---------------- |
| Q2.1 | 4.22、4.15、4.55 | 4.28、4.21、4.18 |
| Q3.1 | 3.99、3.88、4.14 | 4.00、4.04、4.12 |
Since DataStreamSender is fast, the complete query time may not reflect the PR difference.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] zenoyang commented on pull request #7751: [vectorized](improving) (exec) optimize VDataStreamSender's send() performance #7747
Posted by GitBox <gi...@apache.org>.
zenoyang commented on pull request #7751:
URL: https://github.com/apache/incubator-doris/pull/7751#issuecomment-1012810037
new test:
| | before PR (s) | after PR (s) |
| ---- | ---------------- | ---------------- |
| Q2.1 | 4.22、4.15、4.55 | 4.26、4.23、4.51 |
| Q3.1 | 3.99、3.88、4.14 | 3.93、3.90、3.70 |
note: hash join
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] zuochunwei commented on pull request #7751: [vectorized](improving) (exec) optimize VDataStreamSender's send() performance #7747
Posted by GitBox <gi...@apache.org>.
zuochunwei commented on pull request #7751:
URL: https://github.com/apache/incubator-doris/pull/7751#issuecomment-1012736294
> before PR (s) after PR (s)
> Q2.1 4.22、4.15、4.55 4.28、4.21、4.18
> Q3.1 3.99、3.88、4.14 4.00、4.04、4.12
> Since DataStreamSender is fast, the complete query time may not reflect the PR difference.
please test for the recently commit, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] HappenLee commented on a change in pull request #7751: [vectorized](improving) (exec) optimize VDataStreamSender's send() performance #7747
Posted by GitBox <gi...@apache.org>.
HappenLee commented on a change in pull request #7751:
URL: https://github.com/apache/incubator-doris/pull/7751#discussion_r783903210
##########
File path: be/src/vec/core/block.cpp
##########
@@ -843,6 +843,17 @@ void MutableBlock::add_row(const Block* block, int row) {
}
}
+void MutableBlock::add_rows(const Block* block, const int* row_begin, const int* row_end) {
+ auto& src_columns_with_schema = block->get_columns_with_type_and_name();
+ for (size_t i = 0; i < _columns.size(); ++i) {
+ auto& dst = _columns[i];
+ auto& src = *src_columns_with_schema[i].column.get();
+ for (const int* row = row_begin; row != row_end; ++row) {
+ dst->insert_from(src, *row);
Review comment:
here will be virtual function call. so we should implement a function in column.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] HappenLee commented on a change in pull request #7751: [vectorized](improving) (exec) optimize VDataStreamSender's send() performance #7747
Posted by GitBox <gi...@apache.org>.
HappenLee commented on a change in pull request #7751:
URL: https://github.com/apache/incubator-doris/pull/7751#discussion_r785614606
##########
File path: be/src/vec/columns/column_decimal.h
##########
@@ -95,6 +95,13 @@ class ColumnDecimal final : public COWHelper<ColumnVectorHelper, ColumnDecimal<T
void insert_from(const IColumn& src, size_t n) override {
data.push_back(static_cast<const Self&>(src).get_data()[n]);
}
+
+ void insert_indices_from(const IColumn& src, const int* indices_begin, const int* indices_end) override {
+ for (auto x = indices_begin; x != indices_end; ++x) {
+ Self::insert_from(src, *x);
Review comment:
same to the column_vector
##########
File path: be/src/vec/columns/column_vector.cpp
##########
@@ -221,6 +222,15 @@ void ColumnVector<T>::insert_range_from(const IColumn& src, size_t start, size_t
memcpy(data.data() + old_size, &src_vec.data[start], length * sizeof(data[0]));
}
+template <typename T>
+void ColumnVector<T>::insert_indices_from(const IColumn& src, const int* indices_begin, const int* indices_end) {
+ const Self& src_vec = assert_cast<const Self&>(src);
Review comment:
use `push_back_without_reserve`
##########
File path: be/src/vec/columns/column_nullable.cpp
##########
@@ -131,6 +131,12 @@ void ColumnNullable::insert_range_from(const IColumn& src, size_t start, size_t
get_nested_column().insert_range_from(*nullable_col.nested_column, start, length);
}
+void ColumnNullable::insert_indices_from(const IColumn& src, const int* indices_begin, const int* indices_end) {
+ for (auto x = indices_begin; x != indices_end; ++x) {
Review comment:
which will cause to many virtural function call, refactor it
##########
File path: be/src/vec/sink/vdata_stream_sender.h
##########
@@ -261,5 +274,24 @@ class VDataStreamSender::Channel {
size_t _capacity;
bool _is_local;
};
+
+template <typename Channels, typename HashVals>
+Status VDataStreamSender::channel_add_rows(Channels& channels, int num_channels, const HashVals& hash_vals, int rows, Block* block) {
+ std::vector<int> channel2rows[num_channels];
Review comment:
same to the up, we should reuse the mem
##########
File path: be/src/vec/sink/vdata_stream_sender.cpp
##########
@@ -394,69 +430,62 @@ Status VDataStreamSender::send(RuntimeState* state, Block* block) {
int num_channels = _channels.size();
// will only copy schema
// we don't want send temp columns
- auto send_block = *block;
- std::vector<int> result(_partition_expr_ctxs.size());
- int counter = 0;
+ int result_size = _partition_expr_ctxs.size();
+ int result[result_size];
+ RETURN_IF_ERROR(get_partition_column_result(block, result));
- for (auto ctx : _partition_expr_ctxs) {
- RETURN_IF_ERROR(ctx->execute(block, &result[counter++]));
- }
// vectorized caculate hash
int rows = block->rows();
// for each row, we have a siphash val
std::vector<SipHash> siphashs(rows);
// result[j] means column index, i means rows index
- for (int j = 0; j < result.size(); ++j) {
+ for (int j = 0; j < result_size; ++j) {
auto column = block->get_by_position(result[j]).column;
for (int i = 0; i < rows; ++i) {
column->update_hash_with_value(i, siphashs[i]);
}
}
+ // channel2rows' subscript means channel id
+ std::vector<vectorized::UInt64> hash_vals(rows);
for (int i = 0; i < rows; i++) {
- auto target_channel_id = siphashs[i].get64() % num_channels;
- RETURN_IF_ERROR(_channels[target_channel_id]->add_row(&send_block, i));
+ hash_vals[i] = siphashs[i].get64();
}
+ RETURN_IF_ERROR(channel_add_rows(_channels, num_channels, hash_vals, rows, block));
} else if (_part_type == TPartitionType::BUCKET_SHFFULE_HASH_PARTITIONED) {
// 1. caculate hash
// 2. dispatch rows to channel
int num_channels = _channel_shared_ptrs.size();
- auto send_block = *block;
- std::vector<int> result(_partition_expr_ctxs.size());
- int counter = 0;
- for (auto ctx : _partition_expr_ctxs) {
- RETURN_IF_ERROR(ctx->execute(block, &result[counter++]));
- }
+
+ int result_size = _partition_expr_ctxs.size();
+ int result[result_size];
+ RETURN_IF_ERROR(get_partition_column_result(block, result));
+
// vectorized caculate hash val
int rows = block->rows();
// for each row, we have a hash_val
std::vector<size_t> hash_vals(rows);
Review comment:
we should reuse the mem
##########
File path: be/src/vec/sink/vdata_stream_sender.cpp
##########
@@ -394,69 +430,62 @@ Status VDataStreamSender::send(RuntimeState* state, Block* block) {
int num_channels = _channels.size();
// will only copy schema
// we don't want send temp columns
- auto send_block = *block;
- std::vector<int> result(_partition_expr_ctxs.size());
- int counter = 0;
+ int result_size = _partition_expr_ctxs.size();
+ int result[result_size];
+ RETURN_IF_ERROR(get_partition_column_result(block, result));
- for (auto ctx : _partition_expr_ctxs) {
- RETURN_IF_ERROR(ctx->execute(block, &result[counter++]));
- }
// vectorized caculate hash
int rows = block->rows();
// for each row, we have a siphash val
std::vector<SipHash> siphashs(rows);
// result[j] means column index, i means rows index
- for (int j = 0; j < result.size(); ++j) {
+ for (int j = 0; j < result_size; ++j) {
auto column = block->get_by_position(result[j]).column;
for (int i = 0; i < rows; ++i) {
column->update_hash_with_value(i, siphashs[i]);
}
}
+ // channel2rows' subscript means channel id
+ std::vector<vectorized::UInt64> hash_vals(rows);
Review comment:
we should reuse the mem
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] zenoyang commented on pull request #7751: [vectorized](improving) (exec) optimize VDataStreamSender's send() performance #7747
Posted by GitBox <gi...@apache.org>.
zenoyang commented on pull request #7751:
URL: https://github.com/apache/incubator-doris/pull/7751#issuecomment-1012991943
test sql: `select count(distinct lo_partkey) from lineorder`
### before pr
Total: 15s147ms
```
VDataStreamSender (dst_id=3, dst_fragments=[{"0297b49f50144c67-8175137460f3732a", "0297b49f50144c67-8175137460f3732b", "0297b49f50144c67-8175137460f3732c"}]):(Active: 13s155ms, % non-child: 86.86%)
VDataStreamSender (dst_id=3, dst_fragments=[{"0297b49f50144c67-8175137460f3732a", "0297b49f50144c67-8175137460f3732b", "0297b49f50144c67-8175137460f3732c"}]):(Active: 13s87ms, % non-child: 86.41%)
VDataStreamSender (dst_id=3, dst_fragments=[{"0297b49f50144c67-8175137460f3732a", "0297b49f50144c67-8175137460f3732b", "0297b49f50144c67-8175137460f3732c"}]):(Active: 12s833ms, % non-child: 84.73%)
VDataStreamSender cost:
13155*0.8686=11426.433 ms
13870*0.8641=11985.067 ms
12883*0.8473=10915.766 ms
```
### after pr
Total: 13s414ms
```
VDataStreamSender (dst_id=3, dst_fragments=[{"f191f98c5e644df6-a05e405fde783be2", "f191f98c5e644df6-a05e405fde783be3", "f191f98c5e644df6-a05e405fde783be4"}]):(Active: 11s129ms, % non-child: 82.97%)
VDataStreamSender (dst_id=3, dst_fragments=[{"f191f98c5e644df6-a05e405fde783be2", "f191f98c5e644df6-a05e405fde783be3", "f191f98c5e644df6-a05e405fde783be4"}]):(Active: 10s322ms, % non-child: 76.95%)
VDataStreamSender (dst_id=3, dst_fragments=[{"f191f98c5e644df6-a05e405fde783be2", "f191f98c5e644df6-a05e405fde783be3", "f191f98c5e644df6-a05e405fde783be4"}]):(Active: 9s120ms, % non-child: 67.99%)
VDataStreamSender cost:
11129*0.8297=9233.731 ms
10322*0.7695=7942.779 ms
9120*0.6799=6200.688 ms
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] zuochunwei commented on a change in pull request #7751: [vectorized](improving) (exec) optimize VDataStreamSender's send() performance #7747
Posted by GitBox <gi...@apache.org>.
zuochunwei commented on a change in pull request #7751:
URL: https://github.com/apache/incubator-doris/pull/7751#discussion_r785674394
##########
File path: be/src/vec/columns/column_nullable.cpp
##########
@@ -131,6 +131,12 @@ void ColumnNullable::insert_range_from(const IColumn& src, size_t start, size_t
get_nested_column().insert_range_from(*nullable_col.nested_column, start, length);
}
+void ColumnNullable::insert_indices_from(const IColumn& src, const int* indices_begin, const int* indices_end) {
+ for (auto x = indices_begin; x != indices_end; ++x) {
Review comment:
ok
##########
File path: be/src/vec/columns/column_vector.cpp
##########
@@ -221,6 +222,15 @@ void ColumnVector<T>::insert_range_from(const IColumn& src, size_t start, size_t
memcpy(data.data() + old_size, &src_vec.data[start], length * sizeof(data[0]));
}
+template <typename T>
+void ColumnVector<T>::insert_indices_from(const IColumn& src, const int* indices_begin, const int* indices_end) {
+ const Self& src_vec = assert_cast<const Self&>(src);
Review comment:
ok
##########
File path: be/src/vec/columns/column_decimal.h
##########
@@ -95,6 +95,13 @@ class ColumnDecimal final : public COWHelper<ColumnVectorHelper, ColumnDecimal<T
void insert_from(const IColumn& src, size_t n) override {
data.push_back(static_cast<const Self&>(src).get_data()[n]);
}
+
+ void insert_indices_from(const IColumn& src, const int* indices_begin, const int* indices_end) override {
+ for (auto x = indices_begin; x != indices_end; ++x) {
+ Self::insert_from(src, *x);
Review comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] zuochunwei commented on a change in pull request #7751: [vectorized](improving) (exec) optimize VDataStreamSender's send() performance #7747
Posted by GitBox <gi...@apache.org>.
zuochunwei commented on a change in pull request #7751:
URL: https://github.com/apache/incubator-doris/pull/7751#discussion_r785673245
##########
File path: be/src/vec/sink/vdata_stream_sender.h
##########
@@ -261,5 +274,24 @@ class VDataStreamSender::Channel {
size_t _capacity;
bool _is_local;
};
+
+template <typename Channels, typename HashVals>
+Status VDataStreamSender::channel_add_rows(Channels& channels, int num_channels, const HashVals& hash_vals, int rows, Block* block) {
+ std::vector<int> channel2rows[num_channels];
Review comment:
channel2rows is used for aggregate rows for channels, it's a temporary data served for the function of channel_add_rows, it's not used in other member functions, it's not has getter/setter, it's not visiable to outer, so i don't think make it as member data is a good idea.
for performance, if you can prove it with rewriting code, please tell me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org