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

Issue 812784 link

Starred by 3 users

Issue metadata

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

Blocked on:
issue 774301



Sign in to add a comment

rebaseline-cl doesn't optimize properly

Project Member Reported by skobes@chromium.org, Feb 15 2018

Issue description

When a baseline matches on all platforms, it creates platform/mac-mac10.12 and platform/win instead of updating the generic one.
 

Comment 1 by skobes@chromium.org, Feb 15 2018

Cc: robertma@chromium.org qyears...@chromium.org
http://crrev.com/c/922791
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 16 2018

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

commit 47b21786e150427aba0055923b834ccb53b17a62
Author: Steve Kobes <skobes@chromium.org>
Date: Fri Feb 16 02:51:50 2018

Set is_try_builder on mac10.13_blink_rel in builders.json.

To optimize baselines properly, rebaseline-cl needs mac10.13_blink_rel
in its try bot set, because mac10.13 is the only port whose baseline
search path begins with "platform/mac".  If BaselineOptimizer doesn't
see a baseline in platform/mac, it will consolidate matching baselines
from other Mac ports under platform/mac-mac10.12, and fail to merge them
with matching baselines in platform/win into the generic path.

Bug:  812784 ,  774301 
Change-Id: Ifda7a0051bed79a044ab516b40c0795242247b46
Reviewed-on: https://chromium-review.googlesource.com/922791
Commit-Queue: Steve Kobes <skobes@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537146}
[modify] https://crrev.com/47b21786e150427aba0055923b834ccb53b17a62/third_party/WebKit/Tools/Scripts/webkitpy/common/config/builders.json

Comment 3 by skobes@chromium.org, Feb 16 2018

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 16 2018

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

commit f73729069287840409d64ecd95667febf658388e
Author: Robert Ma <robertma@chromium.org>
Date: Fri Feb 16 07:23:10 2018

Revert "Set is_try_builder on mac10.13_blink_rel in builders.json."

This reverts commit 47b21786e150427aba0055923b834ccb53b17a62.

Reason for revert: rebaseline-cl fails, which blocks wpt-import

It's a bit premature to set this flag, which includes mac10.13 in
rebaseline-cl automatically. Baselines haven't been created for mac10.13
so the first engineer/bot that tries to run `rebaseline-cl` with the
flag enabled will need to conduct the huge mac10.13 rebaseline.

Original change's description:
> Set is_try_builder on mac10.13_blink_rel in builders.json.
> 
> To optimize baselines properly, rebaseline-cl needs mac10.13_blink_rel
> in its try bot set, because mac10.13 is the only port whose baseline
> search path begins with "platform/mac".  If BaselineOptimizer doesn't
> see a baseline in platform/mac, it will consolidate matching baselines
> from other Mac ports under platform/mac-mac10.12, and fail to merge them
> with matching baselines in platform/win into the generic path.
> 
> Bug:  812784 ,  774301 
> Change-Id: Ifda7a0051bed79a044ab516b40c0795242247b46
> Reviewed-on: https://chromium-review.googlesource.com/922791
> Commit-Queue: Steve Kobes <skobes@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#537146}

TBR=qyearsley@chromium.org,dpranke@chromium.org,skobes@chromium.org

Change-Id: Id6c23f2bde53ac42fbc49fa5b5211b2f1d90303f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  812784 ,  774301 
Reviewed-on: https://chromium-review.googlesource.com/923781
Reviewed-by: Robert Ma <robertma@chromium.org>
Commit-Queue: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537244}
[modify] https://crrev.com/f73729069287840409d64ecd95667febf658388e/third_party/WebKit/Tools/Scripts/webkitpy/common/config/builders.json

Blockedon: 774301
Cc: -robertma@chromium.org skobes@chromium.org
Owner: robertma@chromium.org
Status: Assigned (was: Fixed)
Sorry, it was my mistake that I didn't leave enough details before OOO. The TODO note in my CL doesn't make much sense on its own.

Once this flag is set, rebaseline-cl will start to try to rebaseline on Mac 10.13 automatically. The first person/bot who runs the command after this switch is flipped will run into the huge rebaseline nightmare on the new port, and will likely fail (due to the scale and potential flakes, etc., the first rebaseline needs multiple passes and manual tweaks).

As expected, the wpt-import bot is the first to hit the problem: https://chromium-review.googlesource.com/c/chromium/src/+/923524

On the other hand, not setting the flag for now won't produce incorrect new baselines. They are just sub-optimal (some of them could be merged to generic). And the first rebaseline on Mac 10.13 will automatically optimize these sub-optimal baselines created in transient, so it's not a big concern.

I was working on the huge rebaseline for Mac 10.13 before OOO, which is close to complete. If this situation is causing inconvenience, I can try to find some time next week to finish it; otherwise, I'll fix it first thing once I'm back. My apology.
P.S. by finishing the work, I mean both landing the rebaseline and relanding Steve's CL to set is_try_builder for mac10.13_blink_rel.

And thanks for the report, Steve.

Comment 7 by skobes@chromium.org, Feb 21 2018

@robertma, thanks for the revert and for clarifying that Mac 10.13 isn't ready yet.

In the meantime, I wonder if there is some change that can be made in BaselineOptimizer to handle this better.  It's bad that we can't update a generic baseline without splitting it into win + mac10.12.  This is the common case and it makes reviewing difficult because they appear in Gerrit as new files instead of diffs.

Comment 8 by skobes@chromium.org, Feb 22 2018

FYI I am using http://crrev.com/c/929886 as a local workaround.
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 2 2018

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

commit 275d7467dae0220bf7942ac064f7065694444923
Author: Steve Kobes <skobes@chromium.org>
Date: Fri Mar 02 18:20:49 2018

Reland "Set is_try_builder on mac10.13_blink_rel in builders.json."

This is a reland of 47b21786e150427aba0055923b834ccb53b17a62

Original change's description:
> Set is_try_builder on mac10.13_blink_rel in builders.json.
> 
> To optimize baselines properly, rebaseline-cl needs mac10.13_blink_rel
> in its try bot set, because mac10.13 is the only port whose baseline
> search path begins with "platform/mac".  If BaselineOptimizer doesn't
> see a baseline in platform/mac, it will consolidate matching baselines
> from other Mac ports under platform/mac-mac10.12, and fail to merge them
> with matching baselines in platform/win into the generic path.
> 
> Bug:  812784 ,  774301 
> Change-Id: Ifda7a0051bed79a044ab516b40c0795242247b46
> Reviewed-on: https://chromium-review.googlesource.com/922791
> Commit-Queue: Steve Kobes <skobes@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#537146}

Bug:  812784 ,  774301 
Change-Id: Icd341db70dc9e3119f69c354729e2e486e7a60a5
Reviewed-on: https://chromium-review.googlesource.com/944201
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Commit-Queue: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540559}
[modify] https://crrev.com/275d7467dae0220bf7942ac064f7065694444923/third_party/WebKit/Tools/Scripts/webkitpy/common/config/builders.json

Status: Fixed (was: Assigned)
Should be fixed now.
Cc: shenghua...@chromium.org linds...@chromium.org sergeybe...@chromium.org dpranke@chromium.org
 Issue 800557  has been merged into this issue.
Cc: robertma@chromium.org martiniss@chromium.org
 Issue 815262  has been merged into this issue.

Sign in to add a comment