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

Issue 903147 link

Starred by 3 users

Issue metadata

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



Sign in to add a comment

61.3% regression in loading.desktop at 604306:604359

Project Member Reported by alexclarke@chromium.org, Nov 8

Issue description

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

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


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

linux-perf

loading.desktop - Benchmark documentation link:
  https://bit.ly/loading-benchmarks
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/10bfe399e40000

Create new tab-loading animation by pbos@chromium.org
https://chromium.googlesource.com/chromium/src/+/18a40fa1b32e4daccf4ea0d9824b38c13fcc9ac3
timeToFirstContentfulPaint: 374.7 → 613.6 (+238.9)

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

Benchmark documentation link:
  https://bit.ly/loading-benchmarks
Project Member

Comment 4 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 5 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 linked above have recovered with the last change.

Sign in to add a comment