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

Issue 773561 link

Starred by 5 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 0
Type: Bug

Blocked on:
issue 774728
issue 792153



Sign in to add a comment

AFDO: several benchmark degradations including Speedometer2: 4.5% regression on R62 branch

Reported by jie....@intel.com, Oct 11 2017

Issue description

Chrome Version: 62.0.3202.4
Chrome OS Version: 9901.1.0
Chrome OS Platform: reef
Network info: 

Please specify Cr-* of the system to which this bug/feature applies (add
the label below).

Steps To Reproduce:
(1) open chrome
(2) lunch speedometer2 benchmark(the source code of speedometer2 https://github.com/WebKit/webkit/tree/master/Websites/browserbench.org/Speedometer2.0, revision 223162)
(3) record the score

Expected Result:
same as 9901.0.0
Actual Result:
~4.5% regression
How frequently does this problem reproduce?
Always
What is the impact to the user, and is there a workaround? If so, what is
it?

Please provide any additional information below. Attach a screen shot or
log if possible.
It's caused by
https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/0ba4e2399a7ecd1853a5a702e92ab9e6d2a18dc7

chromeos-chrome-amd64-62.0.*.afdo is not effective as chromeos-chrome-amd64-61.0.3163.73_rc-r1.afdo, and the regression still exist on Chrome OS Version:9901.42.0, after roll back to use amd64-61.0.3163.73_rc-r1.afdo, the regression disappear




 

Comment 1 by pan.d...@intel.com, Oct 11 2017

Cc: tianyou...@intel.com pan.d...@intel.com
Labels: -Pri-3 Build-Toolchain Performance Pri-0
Owner: llozano@chromium.org
Speedometer2 is not in crosbolt list of benchmarks and I suspect there is no test for it either on ChromeOS. I doubt it should be a P0 in this case.

Comment 3 by pan.d...@intel.com, Oct 11 2017

AFAIK, "speedometer" in chromeperf has been switched to a speedometer2 version https://chromeperf.appspot.com/report?sid=85d1d2a4e09aa185c94675159d5dfb0c12918571003846e4b5083975161e6fd8

I'm not sure it should be a P0 bug, consider it impacts the performance and R62 is going to be stable in 2 weeks, it should be resolved soon in may opinion.

thanks
Pan
Pan, do you happen to know when was the switch to speedometer2 made?
Looking at crosbolt data, I see a 50% performance regression that started between 62.9844.0.0 and 62.9853.0.0 ChromeOS versions which is far more bothersome.

Also, I don't think we should really use R61 afdo data for R62 builds. It might be improving Speedometer for reasons unknown to me but will most likely cause regressions elsewhere.
Owner: laszio@chromium.org
Status: Assigned (was: Unconfirmed)
initially assigning to laszio. But he may need help. 

Comment 6 by pan.d...@intel.com, Oct 11 2017

@manojgupta, thanks for your quick response,
The switch is according to https://bugs.chromium.org/p/chromium/issues/detail?id=756990

To switch back R61's feedback data is not the intention, just an experiment to check if R62's feedback is problematic.
Cc: krk@chromium.org
Labels: ReleaseBlock-Stable M-62
This came up in the 62 status review where 50% reduction in Speedometer performance was noted on Crosbolt, is this a real regression or is it just that the test changed?

Marking as RB-Stable for now. 
there are 2 separate issues:

1) speedometer benchmark has changed as explained in here: https://chromeperf.appspot.com/report?sid=85d1d2a4e09aa185c94675159d5dfb0c12918571003846e4b5083975161e6fd8

I don't understand what version of speedometer is Chrome OS using. So, one of the owners of crosbolt needs to decide which version we need to track. This is most probably related to the degradation that crosbolt sees in 62:

https://cros-goldeneye.corp.google.com/chromeos/console/listCrosbolt?graphSKU=samus_intel_broadwell_i5_2.2g_4Gb&graphTest=speedometer%2FTotal.Total

I have filed Issue 774195 to track that problem (maybe a duplicate)

2) as reported by Intel, there is a perf degradation in Speedometer2. It turns out that AFDO is broken and this will yield to perf degradations upto 10% on several benchmarks. This bug is about this problem.

Status: We have identified the root cause and there is a fix for it. We are working on testing it on 63 before deploying on 62. 




Summary: AFDO: several benchmark degradations including Speedometer2: 4.5% regression on R62 branch (was: Speedometer2: 4.5% regression on R62 branch)
It is most likely due to the incompatibility of autofdo tools and llvm. We are using autofdo-0.15, which is released 17 months ago. The problem happens after we did a compiler roll on Aug. 6. The newer LLVM also need a new compiler flag to make most of AutoFDO. Pending CLs:

https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/717057
https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/716718
More explanation on the first CL: I tried with the latest AutoFDO tool and got some (half) of the performance back.

Comment 12 by krk@chromium.org, Oct 12 2017

Cc: cros-perf-detectives@google.com
Cc: bhthompson@chromium.org vapier@chromium.org
Labels: Merge-Request-62
Sorry I chumped the CLs on master because CQ tests all passed and this is urgent. I'll wait for a successful canary on master before merging to R62.
Project Member

Comment 14 by sheriffbot@chromium.org, Oct 13 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: We are only 3 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Blockedon: 774728
Project Member

Comment 16 by bugdroid1@chromium.org, Oct 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/0783335fa011bbead27a852cecd563fcb2e4bb8e

commit 0783335fa011bbead27a852cecd563fcb2e4bb8e
Author: Ting-Yuan Huang <laszio@chromium.org>
Date: Fri Oct 13 19:19:53 2017

chromeos-chrome: update compiler flags for newer LLVM

BUG= chromium:773561 
TEST=emerge-chell chromeos-chrome

Change-Id: I95486f9efeec6737ace5ea2cfede224dcf8dc819
Reviewed-on: https://chromium-review.googlesource.com/716718
Commit-Queue: Ting-Yuan Huang <laszio@chromium.org>
Tested-by: Ting-Yuan Huang <laszio@chromium.org>
Trybot-Ready: Ting-Yuan Huang <laszio@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/0783335fa011bbead27a852cecd563fcb2e4bb8e/chromeos-base/chromeos-chrome/chromeos-chrome-9999.ebuild

Project Member

Comment 17 by bugdroid1@chromium.org, Oct 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/e533e5fcc16a1e4d749a0e04b6909ad6b26152e0

commit e533e5fcc16a1e4d749a0e04b6909ad6b26152e0
Author: Ting-Yuan Huang <laszio@chromium.org>
Date: Fri Oct 13 19:23:35 2017

autofdo: update to autofdo-0.17

BUG= chromium:773561 
TEST=emerge autofdo
     create_llvm_prof produces reasonable profile

Change-Id: I453b166d2097a2d6e2b6fb94df361b25262acfbe
Reviewed-on: https://chromium-review.googlesource.com/717057
Reviewed-by: Ting-Yuan Huang <laszio@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Tested-by: Ting-Yuan Huang <laszio@chromium.org>
Commit-Queue: Ting-Yuan Huang <laszio@chromium.org>
Trybot-Ready: Ting-Yuan Huang <laszio@chromium.org>

[add] https://crrev.com/e533e5fcc16a1e4d749a0e04b6909ad6b26152e0/sys-devel/autofdo/autofdo-0.17-r1.ebuild
[delete] https://crrev.com/0783335fa011bbead27a852cecd563fcb2e4bb8e/sys-devel/autofdo/files/autofdo-0.15-llvm-next.patch
[add] https://crrev.com/e533e5fcc16a1e4d749a0e04b6909ad6b26152e0/sys-devel/autofdo/autofdo-0.17.ebuild
[modify] https://crrev.com/e533e5fcc16a1e4d749a0e04b6909ad6b26152e0/sys-devel/autofdo/Manifest
[delete] https://crrev.com/0783335fa011bbead27a852cecd563fcb2e4bb8e/sys-devel/autofdo/files/autofdo-0.15-rpath.patch
[delete] https://crrev.com/0783335fa011bbead27a852cecd563fcb2e4bb8e/sys-devel/autofdo/autofdo-0.15.ebuild
[delete] https://crrev.com/0783335fa011bbead27a852cecd563fcb2e4bb8e/sys-devel/autofdo/autofdo-0.15-r6.ebuild

Verified with 10031 and 10032, where the later is 4.4% / 5.6% better than the former in speedometer / speedometer2.
A few canaries are also out there. Can we merge this to R62?
Labels: -Hotlist-Merge-Review -Merge-Review-62 Merge-Approved-62
Approved for 62. 
Project Member

Comment 21 by bugdroid1@chromium.org, Oct 16 2017

Labels: merge-merged-release-R62-9901.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/baa93cee8ca4bfbaaff1f1aa6f2dbf71b33cfbb6

commit baa93cee8ca4bfbaaff1f1aa6f2dbf71b33cfbb6
Author: Ting-Yuan Huang <laszio@chromium.org>
Date: Mon Oct 16 18:34:56 2017

autofdo: update to autofdo-0.17

BUG= chromium:773561 
TEST=emerge autofdo
     create_llvm_prof produces reasonable profile

Change-Id: I453b166d2097a2d6e2b6fb94df361b25262acfbe
Reviewed-on: https://chromium-review.googlesource.com/717057
Reviewed-by: Ting-Yuan Huang <laszio@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Tested-by: Ting-Yuan Huang <laszio@chromium.org>
Commit-Queue: Ting-Yuan Huang <laszio@chromium.org>
Trybot-Ready: Ting-Yuan Huang <laszio@chromium.org>
(cherry picked from commit e533e5fcc16a1e4d749a0e04b6909ad6b26152e0)
Reviewed-on: https://chromium-review.googlesource.com/719441
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Luis Lozano <llozano@chromium.org>

[add] https://crrev.com/baa93cee8ca4bfbaaff1f1aa6f2dbf71b33cfbb6/sys-devel/autofdo/autofdo-0.17-r1.ebuild
[delete] https://crrev.com/4a80cde2536def3a3bfd3d4e8394bd15a48fd990/sys-devel/autofdo/files/autofdo-0.15-llvm-next.patch
[add] https://crrev.com/baa93cee8ca4bfbaaff1f1aa6f2dbf71b33cfbb6/sys-devel/autofdo/autofdo-0.17.ebuild
[modify] https://crrev.com/baa93cee8ca4bfbaaff1f1aa6f2dbf71b33cfbb6/sys-devel/autofdo/Manifest
[delete] https://crrev.com/4a80cde2536def3a3bfd3d4e8394bd15a48fd990/sys-devel/autofdo/files/autofdo-0.15-rpath.patch
[delete] https://crrev.com/4a80cde2536def3a3bfd3d4e8394bd15a48fd990/sys-devel/autofdo/autofdo-0.15.ebuild
[delete] https://crrev.com/4a80cde2536def3a3bfd3d4e8394bd15a48fd990/sys-devel/autofdo/autofdo-0.15-r6.ebuild

Project Member

Comment 22 by bugdroid1@chromium.org, Oct 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/613af4e16f8d7ea374a0e8518731063b5b273b5c

commit 613af4e16f8d7ea374a0e8518731063b5b273b5c
Author: Ting-Yuan Huang <laszio@chromium.org>
Date: Mon Oct 16 18:35:02 2017

chromeos-chrome: update compiler flags for newer LLVM

BUG= chromium:773561 
TEST=emerge-chell chromeos-chrome

Change-Id: I95486f9efeec6737ace5ea2cfede224dcf8dc819
Reviewed-on: https://chromium-review.googlesource.com/716718
Commit-Queue: Ting-Yuan Huang <laszio@chromium.org>
Tested-by: Ting-Yuan Huang <laszio@chromium.org>
Trybot-Ready: Ting-Yuan Huang <laszio@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
(cherry picked from commit 0783335fa011bbead27a852cecd563fcb2e4bb8e)
Reviewed-on: https://chromium-review.googlesource.com/719440
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Luis Lozano <llozano@chromium.org>

[modify] https://crrev.com/613af4e16f8d7ea374a0e8518731063b5b273b5c/chromeos-base/chromeos-chrome/chromeos-chrome-9999.ebuild

Status: Fixed (was: Assigned)
Verified on R63; Need to verify on R62 as well.
Project Member

Comment 24 by sheriffbot@chromium.org, Oct 20 2017

Cc: bhthompson@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
laszio: can we mark this as verified for both R63 and R62? 
Labels: -Merge-Approved-62
Will close after verifying on R62.
Status: Verified (was: Fixed)
Issue 777298 has been merged into this issue.
Project Member

Comment 29 by bugdroid1@chromium.org, Nov 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/e73cc61a15bca2738abae6e2733e60c81095744c

commit e73cc61a15bca2738abae6e2733e60c81095744c
Author: Ting-Yuan Huang <laszio@chromium.org>
Date: Tue Nov 07 10:32:35 2017

kernel_afdo: update kernel flags for newer LLVM

BUG= chromium:773561 
TEST=cros tryjob --hwtest kip-release

Change-Id: I686e043565381ff6d70622dffa0b5e930b751532
Reviewed-on: https://chromium-review.googlesource.com/755473
Commit-Ready: Ting-Yuan Huang <laszio@chromium.org>
Tested-by: Ting-Yuan Huang <laszio@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/e73cc61a15bca2738abae6e2733e60c81095744c/eclass/cros-kernel2.eclass

Blockedon: 792153
There was one more thing we missed and fixed in the following bug. The expected AFDO performance are recovered now (R64/10176.36.0 and master/10210.0.0): There is 6-8% performance gain over non-afdo builds. 

https://bugs.chromium.org/p/chromium/issues/detail?id=792153

Sign in to add a comment