impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3342: Adding new timer to accurately measure the TotalCpuTime
Date Wed, 05 Oct 2016 21:18:04 GMT
Henry Robinson has posted comments on this change.

Change subject: IMPALA-3342: Adding new timer to accurately measure the TotalCpuTime
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4633/1//COMMIT_MSG
Commit Message:

PS1, Line 10: correctly
incorrectly


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

Line 28: #include<sys/resource.h>
space after include - please run git-clang-format over your patches before submitting.


PS1, Line 190: CpuTimeStopWatch
Do you think we actually need a stopwatch here? What about a method that just returns the
ns computed from getrusage()? Then the PFE can just get the time at the start of the computation,
and then compute the delta when the fragment is complete.


PS1, Line 190: class
class needs a comment


PS1, Line 225: end
Why does this need to be a member?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa88aa6f3371fa42d11ecc122f43c7d83623c300
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke <aphadke@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Yonghyun Hwang
Gerrit-HasComments: Yes

Mime
View raw message