[Impala-ASF-CR] IMPALA-2312: simplify stopwatch ElapsedTime()

2019-09-26 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/14293 )

Change subject: IMPALA-2312: simplify stopwatch ElapsedTime()
..

IMPALA-2312: simplify stopwatch ElapsedTime()

The stopwatches had two subtly different ElapsedTime()
interfaces, one that included previous running time
and one that didn't. They do not appear to be both
needed. I looked through all the uses and in all
cases using the old TotalElapsedTime() in place of
ElapsedTime() is equivalent, either because the
stopwatch is stopped when ElapsedTime() is called
or because Stop() is never called.

Change-Id: I004833681248de65ce709fc746269686507aba54
Reviewed-on: http://gerrit.cloudera.org:8080/14293
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins 
---
M be/src/exec/blocking-join-node.cc
M be/src/util/stopwatch.h
2 files changed, 21 insertions(+), 29 deletions(-)

Approvals:
  Tim Armstrong: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I004833681248de65ce709fc746269686507aba54
Gerrit-Change-Number: 14293
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-2312: simplify stopwatch ElapsedTime()

2019-09-26 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14293 )

Change subject: IMPALA-2312: simplify stopwatch ElapsedTime()
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I004833681248de65ce709fc746269686507aba54
Gerrit-Change-Number: 14293
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 26 Sep 2019 21:43:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2312: simplify stopwatch ElapsedTime()

2019-09-26 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14293 )

Change subject: IMPALA-2312: simplify stopwatch ElapsedTime()
..


Patch Set 2:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4650/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I004833681248de65ce709fc746269686507aba54
Gerrit-Change-Number: 14293
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 26 Sep 2019 18:12:45 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2312: simplify stopwatch ElapsedTime()

2019-09-26 Thread Tim Armstrong (Code Review) 
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14293 )

Change subject: IMPALA-2312: simplify stopwatch ElapsedTime()
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I004833681248de65ce709fc746269686507aba54
Gerrit-Change-Number: 14293
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 26 Sep 2019 17:34:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2312: simplify stopwatch ElapsedTime()

2019-09-26 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14293 )

Change subject: IMPALA-2312: simplify stopwatch ElapsedTime()
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5006/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I004833681248de65ce709fc746269686507aba54
Gerrit-Change-Number: 14293
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 26 Sep 2019 17:29:33 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2312: simplify stopwatch ElapsedTime()

2019-09-26 Thread Tim Armstrong (Code Review) 
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14293 )

Change subject: IMPALA-2312: simplify stopwatch ElapsedTime()
..


Patch Set 2:

Carry Csaba's +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I004833681248de65ce709fc746269686507aba54
Gerrit-Change-Number: 14293
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 26 Sep 2019 17:29:19 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2312: simplify stopwatch ElapsedTime()

2019-09-26 Thread Tim Armstrong (Code Review) 
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14293 )

Change subject: IMPALA-2312: simplify stopwatch ElapsedTime()
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14293/1/be/src/util/stopwatch.h
File be/src/util/stopwatch.h:

http://gerrit.cloudera.org:8080/#/c/14293/1/be/src/util/stopwatch.h@116
PS1, Line 116:  void Stop() {
 : total_time_ += RunningTime();
 : running_ = false;
 :   }
> nit, consistancy: this and StopWatch's stop seem to to exactly the same, as
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I004833681248de65ce709fc746269686507aba54
Gerrit-Change-Number: 14293
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 26 Sep 2019 17:29:12 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2312: simplify stopwatch ElapsedTime()

2019-09-26 Thread Tim Armstrong (Code Review) 
Hello Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/14293

to look at the new patch set (#2).

Change subject: IMPALA-2312: simplify stopwatch ElapsedTime()
..

IMPALA-2312: simplify stopwatch ElapsedTime()

The stopwatches had two subtly different ElapsedTime()
interfaces, one that included previous running time
and one that didn't. They do not appear to be both
needed. I looked through all the uses and in all
cases using the old TotalElapsedTime() in place of
ElapsedTime() is equivalent, either because the
stopwatch is stopped when ElapsedTime() is called
or because Stop() is never called.

Change-Id: I004833681248de65ce709fc746269686507aba54
---
M be/src/exec/blocking-join-node.cc
M be/src/util/stopwatch.h
2 files changed, 21 insertions(+), 29 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/14293/2
--
To view, visit http://gerrit.cloudera.org:8080/14293
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I004833681248de65ce709fc746269686507aba54
Gerrit-Change-Number: 14293
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-2312: simplify stopwatch ElapsedTime()

2019-09-26 Thread Csaba Ringhofer (Code Review) 
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14293 )

Change subject: IMPALA-2312: simplify stopwatch ElapsedTime()
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14293/1/be/src/util/stopwatch.h
File be/src/util/stopwatch.h:

http://gerrit.cloudera.org:8080/#/c/14293/1/be/src/util/stopwatch.h@116
PS1, Line 116:  void Stop() {
 : total_time_ += RunningTime();
 : running_ = false;
 :   }
nit, consistancy: this and StopWatch's stop seem to to exactly the same, as 
RunningTime() returns 0 is running_ is false. I think that it would be better 
if both would be the same, as people would not start looking for differences :)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I004833681248de65ce709fc746269686507aba54
Gerrit-Change-Number: 14293
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 26 Sep 2019 14:15:06 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2312: simplify stopwatch ElapsedTime()

2019-09-24 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14293 )

Change subject: IMPALA-2312: simplify stopwatch ElapsedTime()
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4632/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I004833681248de65ce709fc746269686507aba54
Gerrit-Change-Number: 14293
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 24 Sep 2019 21:59:19 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2312: simplify stopwatch ElapsedTime()

2019-09-24 Thread Tim Armstrong (Code Review) 
Tim Armstrong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/14293


Change subject: IMPALA-2312: simplify stopwatch ElapsedTime()
..

IMPALA-2312: simplify stopwatch ElapsedTime()

The stopwatches had two subtly different ElapsedTime()
interfaces, one that included previous running time
and one that didn't. They do not appear to be both
needed. I looked through all the uses and in all
cases using the old TotalElapsedTime() in place of
ElapsedTime() is equivalent, either because the
stopwatch is stopped when ElapsedTime() is called
or because Stop() is never called.

Change-Id: I004833681248de65ce709fc746269686507aba54
---
M be/src/exec/blocking-join-node.cc
M be/src/util/stopwatch.h
2 files changed, 19 insertions(+), 25 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/14293/1
--
To view, visit http://gerrit.cloudera.org:8080/14293
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I004833681248de65ce709fc746269686507aba54
Gerrit-Change-Number: 14293
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong