Skip to content

Conversation

@jacktengg
Copy link
Contributor

@jacktengg jacktengg commented Jul 3, 2023

and remove prefetch when insert data into columns, because it's effect is not stable in production env.

Proposed changes

Issue Number: close #xxx

For the following test, which simulate hash join outputing 435699854 rows from 5131 buiding rows:

    {
        auto col = doris::vectorized::ColumnString::create();
        constexpr int build_rows = 5131;
        constexpr int output_rows = 435699854;
        std::string str("01234567");
        for (int i = 0; i < build_rows; ++i) {
            col->insert_data(str.data(), str.size());
        }
        int indices[output_rows];
        for (int i = 0; i < output_rows; ++i) {
            indices[i] = i % build_rows;
        }
        auto col2 = doris::vectorized::ColumnString::create();
        doris::MonotonicStopWatch watch;
        watch.start();
        col2->insert_indices_from(*col, indices, indices + output_rows);
        watch.stop();
        LOG(WARNING) << "string column insert_indices_from, rows: " << output_rows << ", time: " << doris::PrettyPrinter::print(watch.elapsed_time(), doris::TUnit::TIME_NS);
    }

The ColumnString::insert_indices_from inserting time improve from 6s665ms to 3s158ms:

W0702 23:08:39.672044 1277989 doris_main.cpp:545] string column insert_indices_from, rows: 435699854, time: 3s153ms
W0702 23:09:36.368853 1282061 doris_main.cpp:545] string column insert_indices_from, rows: 435699854, time: 3s158ms


W0703 00:30:26.093307 1468640 doris_main.cpp:545] string column insert_indices_from, rows: 435699854, time: 6s761ms
W0703 00:31:21.043638 1472937 doris_main.cpp:545] string column insert_indices_from, rows: 435699854, time: 6s665ms

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

clang-tidy review says "All clean, LGTM! 👍"

@yiguolei
Copy link
Contributor

yiguolei commented Jul 3, 2023

run buildall

Gabriel39
Gabriel39 previously approved these changes Jul 3, 2023
Copy link
Contributor

@Gabriel39 Gabriel39 left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Jul 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

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

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

PR approved by anyone and no changes requested.

@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Jul 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Member

@mrhhsg mrhhsg left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

clang-tidy review says "All clean, LGTM! 👍"

@jacktengg
Copy link
Contributor Author

run buildall

jacktengg added 2 commits July 3, 2023 14:09
and remove prefetch when insert data into columns, because it's effect
is not stable in production env.
@jacktengg
Copy link
Contributor Author

run p0

@jacktengg
Copy link
Contributor Author

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Contributor

@yiguolei yiguolei left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Jul 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

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

@yiguolei yiguolei merged commit 938c076 into apache:master Jul 4, 2023
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.

4 participants