impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Ho (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4026: Implement double-buffering for BlockingQueue.
Date Fri, 09 Sep 2016 23:37:06 GMT
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 <kwho@cloudera.com>
Gerrit-Reviewer: Chen Huang <paulhuang34@utexas.edu>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mmokhtar@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message