rebaseline-cl doesn't optimize properly |
|||||
Issue descriptionWhen a baseline matches on all platforms, it creates platform/mac-mac10.12 and platform/win instead of updating the generic one.
,
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
,
Feb 16 2018
,
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
,
Feb 16 2018
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.
,
Feb 16 2018
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.
,
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.
,
Feb 22 2018
FYI I am using http://crrev.com/c/929886 as a local workaround.
,
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
,
Mar 2 2018
Should be fixed now.
,
Mar 2 2018
Issue 800557 has been merged into this issue.
,
Mar 5 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by skobes@chromium.org
, Feb 15 2018