Abhishek Rawat has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13178 )

Change subject: IMPALA-4658: Potential race if compiler reorders ReachedLimit() 
usage.
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13178/2/be/src/exec/exec-node.h
File be/src/exec/exec-node.h:

http://gerrit.cloudera.org:8080/#/c/13178/2/be/src/exec/exec-node.h@391
PS2, Line 391:   AtomicInt64 num_rows_returned_shared_;
> I made the change so that we only have a single variable. In the multi-thre
It seems the tool did not send my response from yesterday to the reviewers. I 
also did some basic perf tests and I see that using LongAdder we regress the 
single threaded code-paths such as ExchangeNode::GetNext (evident from 
following result see 11:EXCHANGE stats below)

(R) Regression: TPCH(50) TPCH-Q22 [parquet / none / none] (3.78s -> 3.98s 
[+5.29%])
+---------------------+------------+----------+----------+------------+------------+----------+----------+------------+--------+---------+-----------+
| Operator            | % of Query | Avg      | Base Avg | Delta(Avg) | 
StdDev(%)  | Max      | Base Max | Delta(Max) | #Hosts | #Rows   | Est #Rows |
+---------------------+------------+----------+----------+------------+------------+----------+----------+------------+--------+---------+-----------+
| 05:HASH JOIN        | 44.01%     | 2.51s    | 2.52s    | -0.36%     |   3.56% 
   | 2.75s    | 2.67s    | +2.93%     | 3      | 318.22K | 750.00K   |
| 01:SCAN HDFS        | 3.62%      | 206.70ms | 198.37ms | +4.20%     |   8.68% 
   | 268.71ms | 286.23ms | -6.12%     | 3      | 1.91M   | 750.00K   |
| 11:EXCHANGE         | 8.45%      | 482.53ms | 275.75ms | +74.99%    |   8.77% 
   | 620.61ms | 355.22ms | +74.71%    | 3      | 75.00M  | 75.00M    |
| F00:EXCHANGE SENDER | 35.94%     | 2.05s    | 2.03s    | +1.01%     | * 
12.67% * | 3.18s    | 2.65s    | +20.01%    | 3      | -1      | -1        |
| 03:SCAN HDFS        | 3.20%      | 183.07ms | 173.45ms | +5.55%     | * 
11.75% * | 254.60ms | 235.88ms | +7.94%     | 3      | 75.00M  | 75.00M    |
+---------------------+------------+----------+----------+------------+------------+----------+----------+------------+--------+---------+-----------+

The pre-review tests on the latest code with patch #4 is clean: 
https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/5450/testReport/



--
To view, visit http://gerrit.cloudera.org:8080/13178
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4cbbfad80f7ab87dd6f192a24e2c68f7c66b047e
Gerrit-Change-Number: 13178
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Rawat <[email protected]>
Gerrit-Reviewer: Abhishek Rawat <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Wed, 01 May 2019 22:23:28 +0000
Gerrit-HasComments: Yes

Reply via email to