zuochunwei commented on a change in pull request #7972:
URL: https://github.com/apache/incubator-doris/pull/7972#discussion_r805629745
##########
File path: be/src/vec/exec/join/join_op.h
##########
@@ -26,15 +26,15 @@ namespace doris::vectorized {
struct RowRef {
using SizeT = uint32_t; /// Do not use size_t cause of memory economy
- const Block* block = nullptr;
+ // const Block* block = nullptr;
SizeT row_num = 0;
// Use in right join to mark row is visited
// TODO: opt the varaible to use it only need
bool visited = false;
Review comment:
use bitfield to make visited as the highest bit of row_num? maybe useful.
##########
File path: be/src/vec/exec/join/vhash_join_node.cpp
##########
@@ -292,8 +318,9 @@ struct ProcessHashTableProbe {
std::vector<bool> same_to_prev;
same_to_prev.reserve(1.2 * _batch_size);
- int right_col_idx = _left_table_data_types.size();
- int right_col_len = _right_table_data_types.size();
+ std::vector<uint32_t> _build_index;
+ _build_index.reserve(1.2 * _batch_size);
Review comment:
consider using stack array instead?
##########
File path: be/src/vec/exec/join/vhash_join_node.cpp
##########
@@ -217,49 +249,44 @@ struct ProcessHashTableProbe {
// after probe data eof
if (!_join_node->_is_right_semi_anti) {
++repeat_count;
- for (size_t j = 0; j < right_col_len; ++j) {
- auto& column =
*mapped.block->get_by_position(j).column;
- mcol[j + right_col_idx]->insert_from(column,
mapped.row_num);
- }
+ _build_index.emplace_back(mapped.row_num);
}
} else {
for (auto it = mapped.begin(); it.ok(); ++it) {
// right semi/anti join should dispose the data in
hash table
// after probe data eof
if (!_join_node->_is_right_semi_anti) {
++repeat_count;
- for (size_t j = 0; j < right_col_len; ++j) {
- auto& column =
*it->block->get_by_position(j).column;
- // TODO: interface insert from cause
serious performance problems
- // when column is nullable. Try to make
more effective way
- mcol[j +
right_col_idx]->insert_from(column, it->row_num);
- }
+ _build_index.emplace_back(it->row_num);
}
it->visited = true;
}
}
- }
- } else if (_join_node->_match_all_probe ||
- _join_node->_join_op == TJoinOp::LEFT_ANTI_JOIN) {
- ++repeat_count;
- // only full outer / left outer need insert the data of right
table
- if (_join_node->_match_all_probe) {
- for (size_t j = 0; j < right_col_len; ++j) {
- DCHECK(mcol[j + right_col_idx]->is_nullable());
- mcol[j + right_col_idx]->insert_data(nullptr, 0);
+ } else if (_join_node->_match_all_probe) {
+ // only full outer / left outer need insert the data of
right table
+ ++repeat_count;
+ for (size_t j = 0; j < _right_col_len; ++j) {
+ DCHECK(mcol[j + _right_col_idx]->is_nullable());
+ mcol[j + _right_col_idx]->insert_data(nullptr, 0);
}
}
}
-
items_counts[_probe_index++] = repeat_count;
current_offset += repeat_count;
-
if (current_offset >= _batch_size) {
break;
}
}
-
- for (int i = 0; i < right_col_idx; ++i) {
+
+ // insert all match build rows
+ for (int i = 0; i < _right_col_len; i++) {
+ auto &column = *_build_block.get_by_position(i).column;
+ for (int j = 0; j < _build_index.size(); j++) {
Review comment:
use insert_indices_from() instead
##########
File path: be/src/vec/exec/join/vhash_join_node.cpp
##########
@@ -804,6 +838,10 @@ Status HashJoinNode::_hash_table_build(RuntimeState*
state) {
RETURN_IF_ERROR(child(1)->open(state));
SCOPED_TIMER(_build_timer);
Block block;
+ MutableColumns columns(_right_table_data_types.size());
+ for (int i = 0; i < _right_table_data_types.size(); i++) {
+ columns[i] = _right_table_data_types[i]->create_column();
Review comment:
call reserve() here to avoid exlarging multi-times?
##########
File path: be/src/vec/exec/join/join_op.h
##########
@@ -115,7 +115,7 @@ struct RowRefList : RowRef {
};
RowRefList() {}
- RowRefList(const Block* block_, size_t row_num_) : RowRef(block_,
row_num_) {}
+ RowRefList(size_t row_num_) : RowRef(row_num_) {}
ForwardIterator begin() { return ForwardIterator(this); }
Review comment:
iterating RowRefList through ForwardIterator is slow, consider providing
a interface to get all row_num list
##########
File path: be/src/vec/exec/join/vhash_join_node.cpp
##########
@@ -177,37 +180,66 @@ struct ProcessHashTableProbe {
std::vector<uint32_t> items_counts(_probe_rows);
auto& mcol = mutable_block.mutable_columns();
-
- int right_col_idx = _join_node->_is_right_semi_anti ? 0 :
_left_table_data_types.size();
- int right_col_len = _right_table_data_types.size();
int current_offset = 0;
+ std::vector<uint32_t> _build_index;
+ _build_index.reserve(1.2 * _batch_size);
Review comment:
why 1.2?
##########
File path: be/src/vec/exec/join/vhash_join_node.cpp
##########
@@ -177,37 +180,66 @@ struct ProcessHashTableProbe {
std::vector<uint32_t> items_counts(_probe_rows);
auto& mcol = mutable_block.mutable_columns();
-
- int right_col_idx = _join_node->_is_right_semi_anti ? 0 :
_left_table_data_types.size();
- int right_col_len = _right_table_data_types.size();
int current_offset = 0;
+ std::vector<uint32_t> _build_index;
+ _build_index.reserve(1.2 * _batch_size);
for (; _probe_index < _probe_rows;) {
- // ignore null rows
if constexpr (ignore_null) {
if ((*null_map)[_probe_index]) {
items_counts[_probe_index++] = 0;
continue;
}
}
-
int repeat_count = 0;
- auto find_result =
- (*null_map)[_probe_index]
+ if constexpr (is_inner_join) {
+ if (!(*null_map)[_probe_index]) {
+ auto find_result =
key_getter.find_key(hash_table_ctx.hash_table, _probe_index, _arena);
+
+ if (find_result.is_found()) {
+ auto& mapped = find_result.get_mapped();
+
+ if (mapped.get_row_count() == 1) {
+ ++repeat_count;
+ _build_index.emplace_back(mapped.row_num);
+ } else {
+ if (_probe_index + 2 < _probe_rows)
+ key_getter.prefetch(hash_table_ctx.hash_table,
_probe_index + 2, _arena);
+ for (auto it = mapped.begin(); it.ok(); ++it) {
+ ++repeat_count;
+ _build_index.emplace_back(it->row_num);
Review comment:
if row_num stored in mapped as continuous array, we can just remember
the row_num array’s address here.
it's no need to copy one by one.
##########
File path: be/src/vec/exec/join/vhash_join_node.cpp
##########
@@ -316,10 +343,7 @@ struct ProcessHashTableProbe {
for (auto it = mapped.begin(); it.ok(); ++it) {
++current_offset;
- for (size_t j = 0; j < right_col_len; ++j) {
- auto& column = *it->block->get_by_position(j).column;
- mcol[j + right_col_idx]->insert_from(column,
it->row_num);
- }
+ _build_index.emplace_back(it->row_num);
Review comment:
increase current_offset += it.size() out of the loop
--
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-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
