Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2871843003:
[translate] Defer start of translate ranker model load to first use. (Closed)
|
Created:
3 years, 7 months ago by Roger McFarlane (Chromium) Modified:
3 years, 7 months ago CC:
chromium-reviews Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[translate] Defer start of translate ranker model load to first use.
This CL is in response to perf regression (see referenced bugs).
BUG=697947, 697665
Review-Url: https://codereview.chromium.org/2871843003
Cr-Commit-Position: refs/heads/master@{#471086}
Committed: https://chromium.googlesource.com/chromium/src/+/d2876f4ee1c1a01ee262ca55752503ba3a30a6d5
Patch Set 1 : Perf Experiment: Disable field trial config #Patch Set 2 : Perf Experiment: defer load to first use #Messages
Total messages: 26 (14 generated)
Patchset #2 (id:20001) has been deleted
Description was changed from ========== DO NOT SUBMIT - Perf test Remove translate ranker enforcement from configured fieldtrials for testing. BUG=697947 ========== to ========== [translate] Defer start of translate ranker model load to first use. This CL is in response to perf regression (see referenced bugs). BUG=697947, 697665 ==========
rogerm@chromium.org changed reviewers: + benhenry@chromium.org, hamelphi@chromium.org, sullivan@chromium.org
PTAL? In particular, at the telemetry results from the trybot runs.
rogerm@chromium.org changed reviewers: + droger@chromium.org
+droger
The CQ bit was checked by rogerm@chromium.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.chromium.or...
sullivan@chromium.org changed reviewers: + vmiura@chromium.org
Replacing myself with vmiura on the review. Victor, can you review or delegate to someone with familiarity with smoothness metrics?
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
vmiura@chromium.org changed reviewers: + tdresser@chromium.org
+tdresser@ fyi, this feature was affecting first_gesture_scroll_update_latency on https://chromeperf.appspot.com/group_report?bug_id=697947. Roger, how can we see the trybot results?
On 2017/05/10 22:46:14, vmiura wrote: > +tdresser@ fyi, this feature was affecting first_gesture_scroll_update_latency > on https://chromeperf.appspot.com/group_report?bug_id=697947. > > Roger, how can we see the trybot results? click on the winx64_zen_perf_bisect bot's results under each patchset and look at the "HTML Results" link. For your convenience: PS1: https://console.developers.google.com/m/cloudstorage/b/chromium-telemetry/o/h... PS2: https://console.developers.google.com/m/cloudstorage/b/chromium-telemetry/o/h...
On 2017/05/10 22:49:59, Roger McFarlane (Chromium) wrote: > On 2017/05/10 22:46:14, vmiura wrote: > > +tdresser@ fyi, this feature was affecting first_gesture_scroll_update_latency > > on https://chromeperf.appspot.com/group_report?bug_id=697947. > > > > Roger, how can we see the trybot results? > > click on the winx64_zen_perf_bisect bot's results under each patchset and look > at the "HTML Results" link. > > For your convenience: > > PS1: > https://console.developers.google.com/m/cloudstorage/b/chromium-telemetry/o/h... > PS2: > https://console.developers.google.com/m/cloudstorage/b/chromium-telemetry/o/h... The change seems beneficial to first_gesture_scroll_update_latency But there's a 33X difference between the TOT average of the two runs, so it's not clear to me how comparable the results are.
rogerm@chromium.org changed reviewers: + groby@chromium.org - droger@chromium.org
+groby (droger is OOO)
lgtm i haz a sad :(
The CQ bit was checked by rogerm@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494531522266460,
"parent_rev": "78dd7afe685c9f37a5d0c1e48f1d2f2fc78a1469", "commit_rev":
"d2876f4ee1c1a01ee262ca55752503ba3a30a6d5"}
Message was sent while issue was closed.
Description was changed from ========== [translate] Defer start of translate ranker model load to first use. This CL is in response to perf regression (see referenced bugs). BUG=697947, 697665 ========== to ========== [translate] Defer start of translate ranker model load to first use. This CL is in response to perf regression (see referenced bugs). BUG=697947, 697665 Review-Url: https://codereview.chromium.org/2871843003 Cr-Commit-Position: refs/heads/master@{#471086} Committed: https://chromium.googlesource.com/chromium/src/+/d2876f4ee1c1a01ee262ca557525... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001) as https://chromium.googlesource.com/chromium/src/+/d2876f4ee1c1a01ee262ca557525...
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:40001) has been created in https://codereview.chromium.org/2918173002/ by hamelphi@chromium.org. The reason for reverting is: The perf regression was not caused by this piece of code. Reverting to the original behavior. See https://docs.google.com/document/d/1eeTSJg18rJ4v_oxxSJU8MK20UywnEhocWk_WrWv8k... for details of the regression investigation.. |
