Skip to content

[vectorized] (olap) optimize BlockReader's performance - #7642

Merged
HappenLee merged 4 commits into
apache:vectorizedfrom
zuochunwei:BlockReaderRefactor
Jan 10, 2022
Merged

HappenLee merged 4 commits into
apache:vectorizedfrom
zuochunwei:BlockReaderRefactor

Conversation

@zuochunwei

@zuochunwei zuochunwei commented Jan 6, 2022 •

Copy link
Copy Markdown
Contributor

Proposed changes

make _collect_iter as normal object instead of pointer object
replace _init_agg_state to avoid unnecessary condition branch
prefer to use reference instead of object copy
prefer to use emplace_back rather than push_back
pick the evaluation out from loop
ssb test indicate that this change have 5% improvement nearly(41 seconds reduce to 40.5 seconds)
test SQL: select sum(lo_revenue) as revenue from lineorder join dates on lo_orderdate = d_datekey where d_year = 1993 and lo_discount between 1 and 3 and lo_quantity < 25;

Close related #issue (replace it with issue number if it exists).

Describe the overview of changes, and introduce why we need it.

Types of changes

What types of changes does your code introduce to Doris?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)
  • [ x] Code refactor (Modify the code structure, format the code, etc...)
  • [ x] Optimization. Including functional usability improvements and performance improvements.
  • Dependency. Such as changes related to third-party components.
  • Other.

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I have created an issue on (Fix #ISSUE) and described the bug/feature there in detail
  • Compiling and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • If these changes need document changes, I have updated the document
  • Any dependent changes have been merged

Further comments

If this is a relatively large or complex change, kick off the discussion at [email protected] by explaining why you chose the solution you did and what alternatives you considered, etc...

@zuochunwei zuochunwei changed the title [vectorized] (olap) optimize BlockReader performance [vectorized] (olap) optimize BlockReader's performance Jan 6, 2022
Comment thread be/src/vec/olap/block_reader.cpp Outdated
if (read_params.return_columns[j] == cid) {
if (j < _tablet->num_key_columns() || _tablet->keys_type() != AGG_KEYS) {
_normal_columns_idx.emplace_back(j);
_normal_columns.emplace_back(i);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_normal_columns.emplace_back(i); 这里原来是j,是不是手误写成i了?

@zuochunwei zuochunwei Jan 6, 2022 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_normal_columns.emplace_back(i); 这里原来是j,是不是手误写成i了?

i got it wrong

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here should use the j not i. because the origin_return_columns may different with the return_columns. here need a map to insert right data in insert_data_normal

@zuochunwei zuochunwei Jan 8, 2022 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

@HappenLee HappenLee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposed No 1 will cause error

Comment thread be/src/vec/olap/block_reader.cpp Outdated
if (read_params.return_columns[j] == cid) {
if (j < _tablet->num_key_columns() || _tablet->keys_type() != AGG_KEYS) {
_normal_columns_idx.emplace_back(j);
_normal_columns.emplace_back(i);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here should use the j not i. because the origin_return_columns may different with the return_columns. here need a map to insert right data in insert_data_normal

@HappenLee

Copy link
Copy Markdown
Contributor

LGTM

@github-actions

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Jan 10, 2022
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@HappenLee
HappenLee merged commit 5b0997b into apache:vectorized Jan 10, 2022
HappenLee pushed a commit to HappenLee/incubator-doris that referenced this pull request Jan 13, 2022
HappenLee pushed a commit to HappenLee/incubator-doris that referenced this pull request Jan 13, 2022
HappenLee pushed a commit that referenced this pull request Jan 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants