Skip to content
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

Revert "[improvement](scanner_schedule) reduce memory consumption of … #26772

Closed
wants to merge 1 commit into from

Conversation

dataroaring
Copy link
Contributor

…scanner (#24199)"

This reverts commit 71dcb58.

Proposed changes

Issue Number: close #xxx

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...

@dataroaring
Copy link
Contributor Author

run buildall

Copy link
Contributor

@github-actions github-actions bot left a 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

@@ -74,13 +74,13 @@ class ScannerContext {
ScannerContext(RuntimeState* state_, VScanNode* parent,
const TupleDescriptor* output_tuple_desc,
const std::list<VScannerSPtr>& scanners_, int64_t limit_,
int64_t max_bytes_in_blocks_queue_, const int num_parallel_instances = 1,
int64_t max_bytes_in_blocks_queue_, const int num_parallel_instances = 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: parameter 'num_parallel_instances' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
int64_t max_bytes_in_blocks_queue_, const int num_parallel_instances = 0,
int64_t max_bytes_in_blocks_queue_, int num_parallel_instances = 0,

}

int get_available_thread_slot_num() {
int cal_thread_slot_num_by_free_block_num() {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'cal_thread_slot_num_by_free_block_num' can be made static [readability-convert-member-functions-to-static]

Suggested change
int cal_thread_slot_num_by_free_block_num() {
static int cal_thread_slot_num_by_free_block_num() {

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.77% (8404/22856)
Line Coverage: 29.28% (68138/232681)
Region Coverage: 27.91% (35226/126196)
Branch Coverage: 24.73% (18018/72860)
Coverage Report: http://coverage.selectdb-in.cc/coverage/de8f0959bcbb3caf382e7c9b8f5e2ca0ceeb2127_de8f0959bcbb3caf382e7c9b8f5e2ca0ceeb2127/report/index.html

@doris-robot
Copy link

TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
Tpch sf100 test result on commit de8f0959bcbb3caf382e7c9b8f5e2ca0ceeb2127, data reload: false

run tpch-sf100 query with default conf and session variables
q1	4626	4327	4297	4297
q2	366	190	202	190
q3	2009	1922	1932	1922
q4	1457	1355	1348	1348
q5	4138	4077	4125	4077
q6	240	130	131	130
q7	1924	1521	1522	1521
q8	2713	2685	2699	2685
q9	12948	13528	10088	10088
q10	11397	3424	3444	3424
q11	377	229	229	229
q12	454	284	287	284
q13	10150	3975	3999	3975
q14	328	311	286	286
q15	563	505	518	505
q16	683	609	593	593
q17	1142	972	918	918
q18	7254	6876	6953	6876
q19	1669	1596	1533	1533
q20	585	367	357	357
q21	4334	4078	3989	3989
q22	465	381	381	381
Total cold run time: 69822 ms
Total hot run time: 49608 ms

run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1	4285	4221	4269	4221
q2	322	234	220	220
q3	3777	3767	3765	3765
q4	2537	2546	2586	2546
q5	6583	6588	6529	6529
q6	242	122	125	122
q7	3052	2690	2681	2681
q8	4216	4271	4272	4271
q9	17177	17195	17256	17195
q10	4041	4030	4083	4030
q11	666	634	655	634
q12	946	813	810	810
q13	4218	3800	3771	3771
q14	408	356	391	356
q15	552	517	513	513
q16	724	664	680	664
q17	3968	3922	3946	3922
q18	8944	8901	8891	8891
q19	1709	1645	1695	1645
q20	2298	2049	2016	2016
q21	7829	7675	7710	7675
q22	760	706	733	706
Total cold run time: 79254 ms
Total hot run time: 77183 ms

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants