Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(739)

Issue 272393004: Make cygprofile use clock_gettime() instead of gettimeofday(). (Closed)

Created:
6 years, 7 months ago by Philippe
Modified:
6 years, 7 months ago
Reviewers:
pasko, aberent, glotov
CC:
chromium-reviews
Visibility:
Public.

Description

Make cygprofile use clock_gettime() instead of gettimeofday(). Some of the generated orderfiles looked suspicious. I could observe that one of them had the renderer PID show up before the browser one. The mergetraces.py file was also occasionally complaining about unsorted timestamps. There are not many situations where gettimeofday() should be used instead of clock_gettime(). The former is not monotonic in particular. The original code also had the issue that it was using both gettimeofday() and time(). This also replaces the occurrences of 'ms(ec)' with 'us(ec)' to make it clear that microseconds are used as opposed to milliseconds. BUG=372323 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270090

Patch Set 1 : #

Total comments: 2

Patch Set 2 : Address Egor's comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -21 lines) Patch
M tools/cygprofile/cygprofile.cc View 2 chunks +4 lines, -4 lines 0 comments Download
M tools/cygprofile/mergetraces.py View 1 4 chunks +13 lines, -13 lines 0 comments Download
M tools/cygprofile/symbolize.py View 3 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 11 (0 generated)
Philippe
6 years, 7 months ago (2014-05-12 11:46:05 UTC) #1
pasko
non-owner lgtm! great catch! https://codereview.chromium.org/272393004/diff/40001/tools/cygprofile/mergetraces.py File tools/cygprofile/mergetraces.py (right): https://codereview.chromium.org/272393004/diff/40001/tools/cygprofile/mergetraces.py#newcode88 tools/cygprofile/mergetraces.py:88: raise Exception("WARNING: last_timestamp: " + ...
6 years, 7 months ago (2014-05-12 12:09:34 UTC) #2
Philippe
Thanks Egor! Committer approval would actually be enough since there is no specific owner for ...
6 years, 7 months ago (2014-05-13 09:48:10 UTC) #3
Philippe
The CQ bit was checked by pliard@chromium.org
6 years, 7 months ago (2014-05-13 09:48:34 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pliard@chromium.org/272393004/60001
6 years, 7 months ago (2014-05-13 09:48:56 UTC) #5
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are ...
6 years, 7 months ago (2014-05-13 11:11:46 UTC) #6
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-05-13 11:41:35 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: android_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/183357)
6 years, 7 months ago (2014-05-13 11:41:35 UTC) #8
Philippe
The CQ bit was checked by pliard@chromium.org
6 years, 7 months ago (2014-05-13 12:46:26 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pliard@chromium.org/272393004/60001
6 years, 7 months ago (2014-05-13 12:46:32 UTC) #10
commit-bot: I haz the power
6 years, 7 months ago (2014-05-13 14:06:11 UTC) #11
Message was sent while issue was closed.
Change committed as 270090

Powered by Google App Engine
This is Rietveld 408576698