Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2728663008:
Increase tick precision in TaskQueue on Windows 64. (Closed)
|
Created:
3 years, 9 months ago by tommi Modified:
3 years, 9 months ago Reviewers:
henrika_webrtc CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionIncrease tick precision in TaskQueue on Windows 64.
Hopefully this will reduce the flakiness of PostDelayedTask.
BUG=none
Review-Url: https://codereview.webrtc.org/2728663008
Cr-Commit-Position: refs/heads/master@{#17001}
Committed: https://chromium.googlesource.com/external/webrtc/+/5bdee47ede2f21768db9f1d79504c8713b121331
Patch Set 1 #
Total comments: 6
Patch Set 2 : Only define GetTick for win64 #
Messages
Total messages: 23 (12 generated)
tommi@webrtc.org changed reviewers: + henrika@webrtc.org
The CQ bit was checked by tommi@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_clang_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_clang_dbg/builds/11646)
https://codereview.webrtc.org/2728663008/diff/1/webrtc/base/task_queue_win.cc File webrtc/base/task_queue_win.cc (right): https://codereview.webrtc.org/2728663008/diff/1/webrtc/base/task_queue_win.cc... webrtc/base/task_queue_win.cc:73: DWORD ret = timeGetTime(); What happens if high_res is false?
https://codereview.webrtc.org/2728663008/diff/1/webrtc/base/task_queue_win.cc File webrtc/base/task_queue_win.cc (right): https://codereview.webrtc.org/2728663008/diff/1/webrtc/base/task_queue_win.cc... webrtc/base/task_queue_win.cc:73: DWORD ret = timeGetTime(); On 2017/03/03 12:24:55, henrika_webrtc wrote: > What happens if high_res is false? Default behavior of timeGetTime(): https://msdn.microsoft.com/en-us/library/windows/desktop/dd757629(v=vs.85).aspx In practice this only affects tests at the moment btw, since in Chrome we use Chrome's task scheduler on Windows.
Only define GetTick for win64
The CQ bit was checked by tommi@webrtc.org to run a CQ dry run
LGTM with one question. https://codereview.webrtc.org/2728663008/diff/1/webrtc/base/task_queue_win.cc File webrtc/base/task_queue_win.cc (right): https://codereview.webrtc.org/2728663008/diff/1/webrtc/base/task_queue_win.cc... webrtc/base/task_queue_win.cc:73: DWORD ret = timeGetTime(); Got it. I was more thinking about what will be returned if high_res is false.
LGTM with one question. https://codereview.webrtc.org/2728663008/diff/1/webrtc/base/task_queue_win.cc File webrtc/base/task_queue_win.cc (right): https://codereview.webrtc.org/2728663008/diff/1/webrtc/base/task_queue_win.cc... webrtc/base/task_queue_win.cc:73: DWORD ret = timeGetTime(); Got it. I was more thinking about what will be returned if high_res is false.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
https://codereview.webrtc.org/2728663008/diff/1/webrtc/base/task_queue_win.cc File webrtc/base/task_queue_win.cc (right): https://codereview.webrtc.org/2728663008/diff/1/webrtc/base/task_queue_win.cc... webrtc/base/task_queue_win.cc:73: DWORD ret = timeGetTime(); On 2017/03/03 12:42:32, henrika_webrtc wrote: > Got it. I was more thinking about what will be returned if high_res is false. TIMERR_NOCANDO is one possibility. I don't think that's likely, but I don't want to have a mismatched call to timeEndPeriod if it does, since these calls affect all processes.
https://codereview.webrtc.org/2728663008/diff/1/webrtc/base/task_queue_win.cc File webrtc/base/task_queue_win.cc (right): https://codereview.webrtc.org/2728663008/diff/1/webrtc/base/task_queue_win.cc... webrtc/base/task_queue_win.cc:73: DWORD ret = timeGetTime(); Sorry. My brain added a {} below and I therefore figured that you did not do return in case of false. My bad. Ignore.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_tsan2 on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_tsan2/builds/20477)
https://codereview.webrtc.org/2728663008/diff/1/webrtc/base/task_queue_win.cc File webrtc/base/task_queue_win.cc (right): https://codereview.webrtc.org/2728663008/diff/1/webrtc/base/task_queue_win.cc... webrtc/base/task_queue_win.cc:73: DWORD ret = timeGetTime(); On 2017/03/03 13:00:53, henrika_webrtc wrote: > Sorry. My brain added a {} below and I therefore figured that you did not do > return in case of false. > > My bad. Ignore. ack :)
The CQ bit was checked by tommi@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1488546666299760,
"parent_rev": "9a59473f2c403827a3560944c7ffe76d37ab77b9", "commit_rev":
"5bdee47ede2f21768db9f1d79504c8713b121331"}
Message was sent while issue was closed.
Description was changed from ========== Increase tick precision in TaskQueue on Windows 64. Hopefully this will reduce the flakiness of PostDelayedTask. BUG=none ========== to ========== Increase tick precision in TaskQueue on Windows 64. Hopefully this will reduce the flakiness of PostDelayedTask. BUG=none Review-Url: https://codereview.webrtc.org/2728663008 Cr-Commit-Position: refs/heads/master@{#17001} Committed: https://chromium.googlesource.com/external/webrtc/+/5bdee47ede2f21768db9f1d79... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/external/webrtc/+/5bdee47ede2f21768db9f1d79... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
