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

Issue 731314 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Jun 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Extend the CWP guided optimization to kernel experiment to R60

Project Member Reported by laszio@chromium.org, Jun 8 2017

Issue description

Hi Josafat,

We are running an experiment on quawks in R59 to see what profile guided optimization can brings us in kernel.  Now we would like to make some updates in R60:

1. update the profile for quawks
https://chromium-review.googlesource.com/c/526795/

This should be pretty safe since the experiment has gone through R59 dev and beta, and will be reaching stable soon..


2. extend the experiment to daisy
https://chromium-review.googlesource.com/c/528052/
https://chromium-review.googlesource.com/c/517224/

I've done several trybot tests and they all looked fine.  I'll wait for a canary release before cherry-picking them to R60.


3. fix a potential problem
https://chromium-review.googlesource.com/c/522965/
https://chromium-review.googlesource.com/c/526411/

The experiment was enabled by default for all quawks builds.  This can cause problems to open source developers.  The CLs set the default back and only enable it in chromite.


We already got approval from Doug and Sonny (CrOS kernel team).  However, we didn't make it before R60 was branched so need your approval.  Would you mind to share your concerns, if any?

Please find the details and discussions about this experiment in b/32371911
 
Oops, I've associated the CLs to the items incorrectly.  The right order should be:

1. update the profile for quawks
https://chromium-review.googlesource.com/c/522965/

2. extend the experiment to daisy
https://chromium-review.googlesource.com/c/517224/
https://chromium-review.googlesource.com/c/528052/

3. fix a potential problem
https://chromium-review.googlesource.com/c/526795/
https://chromium-review.googlesource.com/c/526411/
Labels: -Merge-Request-60 M-60 Merge-Approved-60
Owner: laszio@chromium.org
Status: Assigned (was: Untriaged)
Project Member

Comment 3 by sheriffbot@chromium.org, Jun 12 2017

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

Comment 4 by laszio@chromium.org, Jun 13 2017

Labels: -Merge-Approved-60 Merge-Merged
Status: Fixed (was: Assigned)
Oops, I forgot to change the commit messages of cherry-picks to point bugs here.  Manually label this with Merge-Merged.

Comment 5 by laszio@chromium.org, Jun 13 2017

Labels: -Pri-1 Pri-0
Status: Started (was: Fixed)
The 2 builders (daisy and quawks) in R60 are failing.  It looks like that CHROMEOS_KERNEL_SPLITCONFIG is not defined.  However, the same code is working in R59 and master.

chromeos-kernel-3_8-3.8.11-r583: !!! Fetched file: autofdo-chromeos-kernel-3_8--9460.50.0.afdo.xz VERIFY FAILED!
chromeos-kernel-4_4-4.4.64-r838: !!! Fetched file: autofdo-chromeos-kernel-4_4--9460.50.0.afdo.xz VERIFY FAILED!


Project Member

Comment 6 by bugdroid1@chromium.org, Jun 14 2017

Labels: merge-merged-release-R60-9592.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/dba09f3ce771a5dcb5779a3f2a13c7244cd3ba7d

commit dba09f3ce771a5dcb5779a3f2a13c7244cd3ba7d
Author: Ting-Yuan Huang <laszio@chromium.org>
Date: Wed Jun 14 18:06:08 2017

Revert "daisy: Build kernel with AutoFDO"

This reverts commit 557e0a332b2aff6503c5c29e77529be1466e91b4.

Reason for revert: daisy failed to build on R60.
The CL generated an invalid SRC_URI and broke chromeos-kernel.

Original change's description:
> daisy: Build kernel with AutoFDO
> 
> Build kernel with Feedback-Directed-Optimization on daisy.
> 
> BUG=b:32371911
> CQ-DEPEND=CL:517224
> TEST=cbuildbot --hwtest daisy-release
>      chromeos_config_unittest
> 
> Change-Id: Ib115bc46ec60f9b953c6e9d921eb8035319a867b
> Reviewed-on: https://chromium-review.googlesource.com/528052
> Commit-Ready: Ting-Yuan Huang <laszio@chromium.org>
> Tested-by: Ting-Yuan Huang <laszio@chromium.org>
> Reviewed-by: Gabriel Marin <gmx@chromium.org>
> Reviewed-by: Luis Lozano <llozano@chromium.org>
> (cherry picked from commit 5719dac3a88fe9edd26b60a9ddef2eb44882613b)
> Reviewed-on: https://chromium-review.googlesource.com/532237
> Reviewed-by: Ting-Yuan Huang <laszio@chromium.org>
> Commit-Queue: Ting-Yuan Huang <laszio@chromium.org>
> Trybot-Ready: Ting-Yuan Huang <laszio@chromium.org>

Bug:  chromium:731314 
Change-Id: If1b258fc6c360ec40345525defaa8a0eae9d56ec
Reviewed-on: https://chromium-review.googlesource.com/535037
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: Luis Lozano <llozano@chromium.org>

[modify] https://crrev.com/dba09f3ce771a5dcb5779a3f2a13c7244cd3ba7d/cbuildbot/config_dump.json
[modify] https://crrev.com/dba09f3ce771a5dcb5779a3f2a13c7244cd3ba7d/cbuildbot/chromeos_config.py

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/bedb9519547bfcc4027c29f83c17642631b8eefe

commit bedb9519547bfcc4027c29f83c17642631b8eefe
Author: Ting-Yuan Huang <laszio@chromium.org>
Date: Wed Jun 14 18:08:47 2017

Revert "quawks: Build kernel with AutoFDO"

This reverts commit a66c025978cf28a0f03ad1ffabcc110b2016d4f1.

Reason for revert: quawks failed to build on R60.
The CL generated an invalid SRC_URI and broke chromeos-kernel.

Original change's description:
> quawks: Build kernel with AutoFDO
> 
> Build kernel with Feedback-Directed-Optimization on quawks.
> 
> BUG=b:32371911
> TEST=cbuildbot --hwtest quawks-release
>      chromeos_config_unittest
> 
> Change-Id: Ic90ce73ce4cf2e5ad3321cadd8bbb0f058704eed
> Reviewed-on: https://chromium-review.googlesource.com/526411
> Commit-Ready: Ting-Yuan Huang <laszio@chromium.org>
> Tested-by: Ting-Yuan Huang <laszio@chromium.org>
> Reviewed-by: Luis Lozano <llozano@chromium.org>
> (cherry picked from commit 5ffdcc246d3950eba520484e328ff40c872a615c)
> Reviewed-on: https://chromium-review.googlesource.com/532236
> Commit-Queue: Ting-Yuan Huang <laszio@chromium.org>
> Trybot-Ready: Ting-Yuan Huang <laszio@chromium.org>
> Reviewed-by: Gabriel Marin <gmx@chromium.org>
> Reviewed-by: Ting-Yuan Huang <laszio@chromium.org>

Bug:  chromium:731314 
CQ-DEPEND=CL:535037
Change-Id: I2ddb6d84b4e7837292f105bd6237c33fbf1bf84f
Reviewed-on: https://chromium-review.googlesource.com/535038
Tested-by: Ting-Yuan Huang <laszio@chromium.org>
Reviewed-by: Luis Lozano <llozano@chromium.org>

[modify] https://crrev.com/bedb9519547bfcc4027c29f83c17642631b8eefe/cbuildbot/config_dump.json
[modify] https://crrev.com/bedb9519547bfcc4027c29f83c17642631b8eefe/cbuildbot/chromeos_config.py

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 16 2017

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

commit 56010b0a9011cc9f86f8bd2645af3857fab4603e
Author: Ting-Yuan Huang <laszio@chromium.org>
Date: Fri Jun 16 06:54:03 2017

cros-kernel2: use single autofdo profile for different boards

Information that is not predefined (such as PV) nor defined in the
ebuild cannot be embedded in variables that are cached by portage.
In this particular case, CHROMEOS_KERNEL_SPLITCONFIG cannot be embedded
in SRC_URI.

BUG= chromium:731314 
TEST=USE=kernel_afdo emerge-daisy chromeos-kernel-3_8
     USE=kernel_afdo emerge-quawks chromeos-kernel-4_4

Change-Id: Ic900f5d192474413b9a6533a7e9ddee770a9b165
Reviewed-on: https://chromium-review.googlesource.com/536194
Commit-Ready: Ting-Yuan Huang <laszio@chromium.org>
Tested-by: Ting-Yuan Huang <laszio@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Gabriel Marin <gmx@chromium.org>
Reviewed-by: Luis Lozano <llozano@chromium.org>

[modify] https://crrev.com/56010b0a9011cc9f86f8bd2645af3857fab4603e/sys-kernel/chromeos-kernel-4_4/Manifest
[modify] https://crrev.com/56010b0a9011cc9f86f8bd2645af3857fab4603e/sys-kernel/chromeos-kernel-3_8/Manifest
[modify] https://crrev.com/56010b0a9011cc9f86f8bd2645af3857fab4603e/eclass/cros-kernel2.eclass

Comment 9 by laszio@chromium.org, Jun 16 2017

Labels: -Pri-0 -Merge-Merged -M-60 -merge-merged-release-R60-9592.B Merge-Request-60 Pri-1
Hi Josafat, the fix is already in. Could we cherry-pick the fix and re-enable the experiment in R60?

fix:
https://chromium-review.googlesource.com/c/538559

re-land:
https://chromium-review.googlesource.com/c/532236
https://chromium-review.googlesource.com/c/532237
Project Member

Comment 10 by sheriffbot@chromium.org, Jun 16 2017

Labels: -Merge-Request-60 Hotlist-Merge-Review Merge-Review-60
This bug requires manual review: M60 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
ping?

Comment 12 by josa...@google.com, Jun 20 2017

Labels: -Merge-Review-60 M-60 Merge-Approved-60
Labels: -Hotlist-Merge-Review -Merge-Approved-60 Merge-Merged merge-merged-release-R60-9592.B
Status: Fixed (was: Started)
Thanks, the cherry-pick is in:
https://chromium-review.googlesource.com/c/538559/

and CLs enabling kernel_afdo on quawks and daisy relanded:
https://chromium-review.googlesource.com/c/541676/
https://chromium-review.googlesource.com/c/541678

I'll monitor the builders closely.

Comment 14 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment