Michael Ho has posted comments on this change. Change subject: IMPALA-4026: Implement double-buffering for BlockingQueue. ......................................................................
Patch Set 1: (3 comments) It appears to me that primitive_conjunct_odering_5 and primitive_conjunct_ordering_1 have quite a bit of variance. I compared the result at the following two baseline runs: http://sandbox.jenkins.cloudera.com/job/impala-private-perf-16node/234/ at commit 1a83f8bbe871df0527923e6f131a295270e54d33 http://sandbox.jenkins.cloudera.com/job/impala-private-perf-16node/233/ at commit 4fd25114d2ca23205396af1c16dcde3d3bec69fb The result for conjunt_ordering_5 ranges from 27.16 to 44.45 while the reference baseline is 38.93. Similarly, conjunct_ordering_1 ranges from 10.95 to 11.08 while the reference baseline is 9.84. My baseline should be based on run 233: +---------------------+--------------------------------------------------------+-----------------------+--------+-------------+------------+-----------+----------------+-------------+-------+ | Workload | Query | File Format | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%) | Base StdDev(%) | Num Clients | Iters | +---------------------+--------------------------------------------------------+-----------------------+--------+-------------+------------+-----------+----------------+-------------+-------+ | TARGETED-PERF(_300) | primitive_filter_string_selective | parquet / none / none | 1.09 | 0.99 | +10.31% | 2.59% | 2.49% | 1 | 3 | | TARGETED-PERF(_300) | primitive_conjunct_ordering_2 | parquet / none / none | 73.94 | 70.22 | +5.29% | 3.50% | 0.29% | 1 | 3 | | TARGETED-PERF(_300) | primitive_broadcast_join_2 | parquet / none / none | 6.01 | 5.73 | +5.04% | 0.07% | 0.44% | 1 | 3 | | TARGETED-PERF(_300) | primitive_broadcast_join_1 | parquet / none / none | 1.70 | 1.62 | +4.83% | 1.26% | 3.04% | 1 | 3 | | TARGETED-PERF(_300) | primitive_exchange_broadcast | parquet / none / none | 84.30 | 81.13 | +3.90% | 0.67% | 0.35% | 1 | 3 | | TARGETED-PERF(_300) | primitive_filter_string_non_selective | parquet / none / none | 1.04 | 1.01 | +2.31% | 2.44% | 0.21% | 1 | 3 | | TARGETED-PERF(_300) | primitive_groupby_bigint_lowndv | parquet / none / none | 3.62 | 3.57 | +1.44% | 2.10% | 0.71% | 1 | 3 | | TARGETED-PERF(_300) | primitive_groupby_bigint_pk | parquet / none / none | 89.36 | 88.84 | +0.59% | 2.64% | 1.25% | 1 | 3 | | TARGETED-PERF(_300) | primitive_broadcast_join_3 | parquet / none / none | 58.96 | 58.61 | +0.59% | 0.43% | 0.08% | 1 | 3 | | TARGETED-PERF(_300) | primitive_orderby_bigint | parquet / none / none | 30.72 | 30.64 | +0.26% | 0.00% | 0.00% | 1 | 3 | | TARGETED-PERF(_300) | primitive_filter_decimal_selective | parquet / none / none | 0.92 | 0.92 | +0.14% | 0.06% | 0.53% | 1 | 3 | | TARGETED-PERF(_300) | primitive_groupby_bigint_highndv | parquet / none / none | 23.84 | 23.82 | +0.08% | 0.67% | 0.31% | 1 | 3 | | TARGETED-PERF(_300) | primitive_empty_build_join_1 | parquet / none / none | 13.31 | 13.32 | -0.10% | 1.04% | 0.54% | 1 | 3 | | TARGETED-PERF(_300) | primitive_filter_string_like | parquet / none / none | 5.77 | 5.84 | -1.28% | 3.07% | 2.55% | 1 | 3 | | TARGETED-PERF(_300) | primitive_conjunct_ordering_4 | parquet / none / none | 1.17 | 1.18 | -1.33% | 1.30% | 0.01% | 1 | 3 | | TARGETED-PERF(_300) | primitive_shuffle_join_one_to_many_string_with_groupby | parquet / none / none | 228.99 | 232.12 | -1.35% | 0.29% | 1.01% | 1 | 3 | | TARGETED-PERF(_300) | primitive_conjunct_ordering_3 | parquet / none / none | 1.53 | 1.55 | -1.87% | 0.37% | 4.97% | 1 | 3 | | TARGETED-PERF(_300) | primitive_orderby_all | parquet / none / none | 108.38 | 110.56 | -1.97% | 0.73% | 1.21% | 1 | 3 | | TARGETED-PERF(_300) | primitive_conjunct_ordering_1 | parquet / none / none | 10.72 | 10.95 | -2.10% | 2.57% | 1.82% | 1 | 3 | | TARGETED-PERF(_300) | primitive_groupby_decimal_lowndv.test | parquet / none / none | 3.49 | 3.62 | -3.44% | 1.45% | 0.63% | 1 | 3 | | TARGETED-PERF(_300) | primitive_filter_bigint_selective | parquet / none / none | 0.64 | 0.67 | -3.64% | 4.04% | 0.16% | 1 | 3 | | TARGETED-PERF(_300) | primitive_exchange_shuffle | parquet / none / none | 76.82 | 80.29 | -4.32% | 0.28% | 0.41% | 1 | 3 | | TARGETED-PERF(_300) | primitive_shuffle_join_union_all_with_groupby | parquet / none / none | 74.98 | 79.89 | -6.14% | 3.71% | 0.82% | 1 | 3 | | TARGETED-PERF(_300) | primitive_top-n_all | parquet / none / none | 34.24 | 36.63 | -6.52% | 1.85% | 0.78% | 1 | 3 | | TARGETED-PERF(_300) | primitive_filter_decimal_non_selective | parquet / none / none | 0.88 | 0.94 | -7.04% | 1.14% | 2.49% | 1 | 3 | | TARGETED-PERF(_300) | primitive_groupby_decimal_highndv | parquet / none / none | 24.78 | 26.66 | -7.07% | 0.08% | 0.22% | 1 | 3 | | TARGETED-PERF(_300) | primitive_topn_bigint | parquet / none / none | 5.02 | 5.43 | -7.42% | 1.52% | 0.20% | 1 | 3 | | TARGETED-PERF(_300) | primitive_conjunct_ordering_5 | parquet / none / none | 40.60 | 44.45 | -8.64% | 3.50% | 3.56% | 1 | 3 | | TARGETED-PERF(_300) | primitive_filter_bigint_non_selective | parquet / none / none | 0.74 | 1.62 | I -54.22% | 3.50% | 0.01% | 1 | 3 | +---------------------+--------------------------------------------------------+-----------------------+--------+-------------+------------+-----------+----------------+-------------+-------+ http://gerrit.cloudera.org:8080/#/c/4350/1//COMMIT_MSG Commit Message: Line 29: BlockingQueue to using POSIX pthread primitives instead of boost > Maybe we should just implement our own wrapper class around pthread's condi That sounds like a reasonable middle ground. Will look into it. http://gerrit.cloudera.org:8080/#/c/4350/1/be/src/util/blocking-queue.h File be/src/util/blocking-queue.h: Line 201: /// Guards against concurrent access to 'write_list_'. > Maybe document lock order here too? Done PS1, Line 192: /// 'get' callers wait on this. : mutable pthread_cond_t get_cv_; : : /// 'put' callers wait on this. : mutable pthread_cond_t put_cv_; : : /// Guards against concurrent access to 'read_list_'. : mutable pthread_mutex_t read_lock_; : : /// Guards against concurrent access to 'write_list_'. : mutable pthread_mutex_t write_lock_; > why do we add 'mutable' here? they are used in non-const member functions a Yes, they are probably not needed. -- To view, visit http://gerrit.cloudera.org:8080/4350 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib9f4cf351455efefb0f3bb791cf9bc82d1421d54 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Chen Huang <paulhuan...@utexas.edu> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Mostafa Mokhtar <mmokh...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
