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

Issue 806831 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug-Regression



Sign in to add a comment

10%-10.7% regression in blink_perf.layout at 531992:532076

Project Member Reported by primiano@chromium.org, Jan 29 2018

Issue description

See the link to graphs below.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Jan 29 2018

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=806831

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


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

chromium-rel-win7-gpu-ati
chromium-rel-win7-gpu-nvidia
Owner: hidehiko@chromium.org
Status: Assigned (was: Untriaged)
Re-running the bisect job since it didn't complete, but the results in #2 look pretty clear that there is a regression at r532004, "Implement conditional copy/move ctors/assign-operators."

hidehiko, can you take a look?
Cc: danakj@chromium.org
Labels: OS-Windows
sullivan@, I don't have win dev env set up. Is it possible to run perf test on bots for a CL which is not yet landed for testing?

crrev.com/c/951024 is the potential fix if r532004 is actual cause, so I'd like to give it a try to run the test.

danakj@, BTW, regardless of this issue, I think we should have such a change anyway, so I have sent a CL to review. 
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 7 2018

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

commit c4aa16b9dfd1acc518bad647b5f551eaa72ed0b0
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Wed Mar 07 03:45:37 2018

Enable Empty-Base-Class optimization for Win.

On windows build, empty base class optimization does not work
by default, thus empty struct trick to implement conditional
{copy,move}{constructor,assign-operator} causes unexpected
Optional class size increasing. This CL fixes it.

BUG= 806831 
TEST=Trybot.

Change-Id: I712b949b55a75d1ce23f032d2223258f744350c5
Reviewed-on: https://chromium-review.googlesource.com/951024
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541313}
[modify] https://crrev.com/c4aa16b9dfd1acc518bad647b5f551eaa72ed0b0/base/optional.h
[modify] https://crrev.com/c4aa16b9dfd1acc518bad647b5f551eaa72ed0b0/base/optional_unittest.cc

Project Member

Comment 8 by 42576172...@developer.gserviceaccount.com, Mar 10 2018

Cc: cbiesin...@chromium.org bcwh...@chromium.org chrishtr@chromium.org skobes@chromium.org bokan@chromium.org rch@chromium.org zhongyi@chromium.org asvitk...@chromium.org hidehiko@chromium.org rkaplow@chromium.org
Owner: cbiesin...@chromium.org
📍 Found significant differences after each of 5 commits.
https://pinpoint-dot-chromeperf.appspot.com/job/16b8c9b4440000

Implement conditional copy/move ctors/assign-operators. by hidehiko@chromium.org
https://chromium.googlesource.com/chromium/src/+/5cae9645215d02cb1f986a181a208f8a4817fc86

Add test for purge of UKM logs when consent is revoked. by bcwhite@chromium.org
https://chromium.googlesource.com/chromium/src/+/da9945e142b70f951f933fc32009740fc39cd2b0

Landing Recent QUIC changes until 1:12 PM, Jan 19, 2018 UTC-8 by zhongyi@chromium.org
https://chromium.googlesource.com/chromium/src/+/d372280b432c74f278faeddfcd5ec59f65fe17cf

Update WebFrame::VisibleContentRect for RLS. by skobes@chromium.org
https://chromium.googlesource.com/chromium/src/+/a1dceecc377acf195a2794851a9340279faf0cf1

Add UKM metrics for rendering timing measurements by cbiesinger@chromium.org
https://chromium.googlesource.com/chromium/src/+/a324bac1171a6d35eb9fe367529300aabc3ae1f2

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Status: Fixed (was: Assigned)
My change has been reverted in https://chromium-review.googlesource.com/c/chromium/src/+/892482

Sign in to add a comment