New issue
Advanced search Search tips

Issue 901751 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

122.5% regression in v8.browsing_desktop at 604307:604358

Project Member Reported by jgruber@chromium.org, Nov 5

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=901751

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=7b07db1a3dac7fd38ac7da1c607dffa0704dab23600eebc6f34105211e5175ec


Bot(s) for this bug's original alert(s):

mac-10_13_laptop_high_end-perf

v8.browsing_desktop - Benchmark documentation link:
  None
📍 Couldn't reproduce a difference.
https://pinpoint-dot-chromeperf.appspot.com/job/163548dee40000
Cc: pbos@chromium.org
Owner: pbos@chromium.org
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/14edcdcde40000

Create new tab-loading animation by pbos@chromium.org
https://chromium.googlesource.com/chromium/src/+/18a40fa1b32e4daccf4ea0d9824b38c13fcc9ac3
total:500ms_window:renderer_eqt: 453 → 230.1 (-222.9)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

Benchmark documentation link:
  None
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 12

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

commit 98d67224c1af355a7578341060a591d2b6505578
Author: Peter Boström <pbos@chromium.org>
Date: Mon Nov 12 20:19:16 2018

Interpolate loading progress in tab animation

Instead of skipping to the new load-progress value as it arrives,
interpolate to it. This results in a smoother animation as the
load-progress callbacks can be fairly sparse (and jump from 20% to 80%
for instance).

Drives animations off of the same timer that is used for the throbbing
state. This will hopefully address a performance regression as the
throbbing timer, progress fade-out animation and favicon fade-in
animation were all scheduling separate high-frequency repainting of the
tab icon.

Bug:  chromium:903827 ,  chromium:903147 , chromium:902475,  chromium:901751 

progress

Change-Id: Iad2a24d94206d5871a5cacc5e56d072161407940
Reviewed-on: https://chromium-review.googlesource.com/c/1330345
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607316}
[modify] https://crrev.com/98d67224c1af355a7578341060a591d2b6505578/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/98d67224c1af355a7578341060a591d2b6505578/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/98d67224c1af355a7578341060a591d2b6505578/chrome/browser/ui/views/tabs/tab.h
[modify] https://crrev.com/98d67224c1af355a7578341060a591d2b6505578/chrome/browser/ui/views/tabs/tab_icon.cc
[modify] https://crrev.com/98d67224c1af355a7578341060a591d2b6505578/chrome/browser/ui/views/tabs/tab_icon.h
[modify] https://crrev.com/98d67224c1af355a7578341060a591d2b6505578/chrome/browser/ui/views/tabs/tab_strip.cc
[modify] https://crrev.com/98d67224c1af355a7578341060a591d2b6505578/chrome/browser/ui/views/tabs/tab_strip.h
[modify] https://crrev.com/98d67224c1af355a7578341060a591d2b6505578/chrome/browser/ui/views/tabs/tab_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 14

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

commit c65b08a1d44375e034ee693649bc923b179799e9
Author: Peter Boström <pbos@chromium.org>
Date: Wed Nov 14 16:10:17 2018

Don't schedule UI updates on load progress

The load-progress animation is driven off a timer that starts in
LoadingStateChanged and progresses without scheduling another update.

This is a likely cause for performance regressions as all load progress
updates (even ones without UI changes) will hammer this function.

Bug:  chromium:905189 ,  chromium:903147 ,  chromium:901751 
Change-Id: I92828bbaee6f15fc497ae50ec0d4c3975e638a86
Reviewed-on: https://chromium-review.googlesource.com/c/1335948
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607996}
[modify] https://crrev.com/c65b08a1d44375e034ee693649bc923b179799e9/chrome/browser/ui/browser.cc
[modify] https://crrev.com/c65b08a1d44375e034ee693649bc923b179799e9/chrome/browser/ui/browser.h

Status: Fixed (was: Assigned)
The graphs for this bug have recovered (even before the last CL), though neither of my fixes look like they are in the range that seems to have fixed it though. *shrug*

Sign in to add a comment