New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 763110 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: 2017-09-12
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

2189.8% regression in system_health.common_desktop/clock_sync_latency_battor_to_telemetry_avg/load_search/load_search_taobao on chromium-rel-mac11-pro at 499692:499797

Project Member Reported by nedngu...@google.com, Sep 7 2017

Issue description

Performance dashboard identified a 2189.8% regression in system_health.common_desktop/clock_sync_latency_battor_to_telemetry_avg/load_search/load_search_taobao on chromium-rel-mac11-pro at revision range 499692:499797. Graph: https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg9e_etwoM



 
Battor clock sync now is in 50ms range. Infra work involving hardware is just hard :-/
It's clear that this is due to https://codereview.chromium.org/3004403002/, which falls in the revision range that this regression happened at. That new release of the BattOr agent contained several CLs worth of changes, though, so I'll need to do some digging for the exact change responsible.
They call it *hard*ware for a reason. Let me know if you need any help, I'm happy to help as needed.
Boooooooooo, that joke was awful! I loved it. Aaron, I'll have a fix headed your way in a few minutes.
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d6142d2665b94d3472f5ff1bd23c837c2f76539d

commit d6142d2665b94d3472f5ff1bd23c837c2f76539d
Author: Charlie Andrews <charliea@chromium.org>
Date: Fri Sep 08 21:24:38 2017

Only flush the BattOr connection the first time that it's opened

Previously, we were flushing it every time that
BattOrConnection::Open is called, which happens once per command,
including RecordClockSyncMarker. This added an unnecessary 50ms delay to
RecordClockSyncMarker.

TBR=rnephew@chromium.org

Bug:  763110 
Change-Id: Ibea37eb1cabd7d154a101c498234ca5b6e596e4e
Reviewed-on: https://chromium-review.googlesource.com/657198
Commit-Queue: Charlie Andrews <charliea@chromium.org>
Reviewed-by: Charlie Andrews <charliea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500680}
[modify] https://crrev.com/d6142d2665b94d3472f5ff1bd23c837c2f76539d/tools/battor_agent/battor_agent.cc
[modify] https://crrev.com/d6142d2665b94d3472f5ff1bd23c837c2f76539d/tools/battor_agent/battor_agent_unittest.cc
[modify] https://crrev.com/d6142d2665b94d3472f5ff1bd23c837c2f76539d/tools/battor_agent/battor_connection_impl.cc
[modify] https://crrev.com/d6142d2665b94d3472f5ff1bd23c837c2f76539d/tools/battor_agent/battor_connection_impl_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b808d5e225a0ba21d3eb720173a27f8feac2f563

commit b808d5e225a0ba21d3eb720173a27f8feac2f563
Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org>
Date: Mon Sep 11 20:09:55 2017

Roll src/third_party/catapult/ f465506fe..99ec81878 (6 commits)

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/f465506fe2a7..99ec8187805e

$ git log f465506fe..99ec81878 --date=short --no-merges --format='%ad %ae %s'
2017-09-11 dtu [pinpoint] Rename some variables on the front-end.
2017-09-11 loloangela Fix errors related to invalid-name pt. 8
2017-09-11 nednguyen Use --ignore-certificate-errors for webview
2017-09-11 benjhayden Upgrade <dom-module name> to <dom-module id> for Polymer 2.0.
2017-09-11 charliea Release a new version of the BattOr agent binary
2017-09-11 yuzus Add PrepareForLeakDetection method in telemetry

Created with:
  roll-dep src/third_party/catapult
BUG= 753948 , 763110 , 763280 


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=sullivan@chromium.org

Change-Id: I3bd453c50dd53950415724cc3b68c180fa50e56f
Reviewed-on: https://chromium-review.googlesource.com/661179
Reviewed-by: <catapult-deps-roller@chromium.org>
Commit-Queue: <catapult-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501012}
[modify] https://crrev.com/b808d5e225a0ba21d3eb720173a27f8feac2f563/DEPS

NextAction: 2017-09-12
Status: Fixed (was: Untriaged)
I'll take a look at this tomorrow once the new numbers are available.
The NextAction date has arrived: 2017-09-12
Status: Verified (was: Fixed)
Yep, the graph is back down.

https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg9e_etwoM

Sign in to add a comment