Support filling in baselines from other platforms in rebaseline-cl.

Support filling in baselines from other platforms in rebaseline-cl.

BUG=673966
Review-Url: https://codereview.chromium.org/2803143002
Cr-Commit-Position: refs/heads/master@{#463877}
Committed: https://chromium.googlesource.com/chromium/src/+/fb23c8091deec6697b7855fe4f40a6be9f9a604b

[qyearsley, 2017-04-07 23:23:15]

qyearsley@chromium.org changed reviewers:
+ wkorman@chromium.org

[qyearsley, 2017-04-07 23:23:16]

In this initial version:

 - The tool will only try to fill in results if a special flag is passed
(--fill-missing).
 - There is no logic to select the "best" build to fill in for a given port; it
just selects some build. This should be refined later, but this should work fine
for cases where results are the same on all platforms.

[qyearsley, 2017-04-07 23:23:22]

The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run

[qyearsley, 2017-04-07 23:23:31]

The CQ bit was unchecked by qyearsley@chromium.org

[qyearsley, 2017-04-07 23:25:20]

The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-07 23:25:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-08 01:17:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-08 01:17:31]

Dry run: This issue passed the CQ dry run.

[wkorman, 2017-04-10 20:12:27]

lgtm

https://codereview.chromium.org/2803143002/diff/20001/third_party/WebKit/Tool...
File third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py
(right):

https://codereview.chromium.org/2803143002/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py:182:
return self._test_prefix_map[test_prefix]
Should we make a defensive copy so that if caller mutates the result it doesn't
hose internals?

https://codereview.chromium.org/2803143002/diff/20001/third_party/WebKit/Tool...
File third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py
(right):

https://codereview.chromium.org/2803143002/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py:41:
help='If some builds have no results available, use results from other
builds.'),
This reads as if the port will be the same and the only thing we are potentially
doing is falling back to an earlier build if it had suitable baseline. But I
think we could in fact switch ports? Perhaps worth clarifying documentation.

https://codereview.chromium.org/2803143002/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py:242: #
TODO(qyearsley): Decide what build to use for a given port
Just looking to make sure I understand, is this noted work basically revising to
incorporate the same/similar platform (port) fallback logic that we use for
optimizing baselines?

[qyearsley, 2017-04-10 23:52:10]

https://codereview.chromium.org/2803143002/diff/20001/third_party/WebKit/Tool...
File third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py
(right):

https://codereview.chromium.org/2803143002/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py:182:
return self._test_prefix_map[test_prefix]
On 2017/04/10 at 20:12:27, wkorman wrote:
> Should we make a defensive copy so that if caller mutates the result it
doesn't hose internals?

Ah, good point :-) Done.

https://codereview.chromium.org/2803143002/diff/20001/third_party/WebKit/Tool...
File third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py
(right):

https://codereview.chromium.org/2803143002/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py:41:
help='If some builds have no results available, use results from other
builds.'),
On 2017/04/10 at 20:12:27, wkorman wrote:
> This reads as if the port will be the same and the only thing we are
potentially doing is falling back to an earlier build if it had suitable
baseline. But I think we could in fact switch ports? Perhaps worth clarifying
documentation.

Ah, yes, not just "use results from other builds", but "use results from other
platforms' try jobs".

Done.

https://codereview.chromium.org/2803143002/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py:242: #
TODO(qyearsley): Decide what build to use for a given port
On 2017/04/10 at 20:12:27, wkorman wrote:
> Just looking to make sure I understand, is this noted work basically revising
to incorporate the same/similar platform (port) fallback logic that we use for
optimizing baselines?

Something similar, but I believe we can't entirely rely on the fallback graph.

For example, suppose we just have results for win7. The baseline fallback path
for linux-trusty is ["linux", "win"], and "win" corresponds to win10. Similarly,
the baseline fallback path for mac10.10 is ["mac-mac10.10", "mac-mac10.11",
"mac"], which doesn't include any windows platforms.

Fallback graph:
https://drive.google.com/a/google.com/file/d/0Bzxz6fWz5JOIWlVMQkJXc1Bjd2M/vie...

So I guess one approach would be to start with the fallback path, but then if
there are no available builds for any ports there, then just use any build?

One approach would be to construct a dict of port to list of other ports
starting with similar ports just by listing all the supported versions, e.g.:


def similar_ports_dict():
  android = ['android-' + v for v in AndroidPort.SUPPORTED_VERSIONS]
  linux = ['linux-' + v for v in LinuxPort.SUPPORTED_VERSIONS]
  mac = ['mac-' + v for v in MacPort.SUPPORTED_VERSIONS]
  win = ['win-' + v for v in WinPort.SUPPORTED_VERSIONS]
  similar_ports = {}
  for v in android:
    similar_ports[v] = android + linux + mac + win
  for v in linux:
    similar_ports[v] = linux + android + mac + win
  for v in mac:
    similar_ports[v] = mac + linux + android + win
  for v in win:
    similar_ports[v] = win + linux + android + mac
  return similar_ports

My main thought here is just that, for example, if we just need to fill in one
mac port, then we should default to using another mac result first; but if there
are no other mac builds available, then probably trying to use any result is
OK...


Does that sound roughly correct? I'm hoping I can use something a little simpler
than the above.

[wkorman, 2017-04-11 20:36:49]

lgtm

https://codereview.chromium.org/2803143002/diff/20001/third_party/WebKit/Tool...
File third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py
(right):

https://codereview.chromium.org/2803143002/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py:242: #
TODO(qyearsley): Decide what build to use for a given port
On 2017/04/10 23:52:10, qyearsley wrote:
> On 2017/04/10 at 20:12:27, wkorman wrote:
> > Just looking to make sure I understand, is this noted work basically
revising
> to incorporate the same/similar platform (port) fallback logic that we use for
> optimizing baselines?
> 
> Something similar, but I believe we can't entirely rely on the fallback graph.
> 
> For example, suppose we just have results for win7. The baseline fallback path
> for linux-trusty is ["linux", "win"], and "win" corresponds to win10.
Similarly,
> the baseline fallback path for mac10.10 is ["mac-mac10.10", "mac-mac10.11",
> "mac"], which doesn't include any windows platforms.
> 
> Fallback graph:
>
https://drive.google.com/a/google.com/file/d/0Bzxz6fWz5JOIWlVMQkJXc1Bjd2M/vie...
> 
> So I guess one approach would be to start with the fallback path, but then if
> there are no available builds for any ports there, then just use any build?
> 
> One approach would be to construct a dict of port to list of other ports
> starting with similar ports just by listing all the supported versions, e.g.:
> 
> 
> def similar_ports_dict():
>   android = ['android-' + v for v in AndroidPort.SUPPORTED_VERSIONS]
>   linux = ['linux-' + v for v in LinuxPort.SUPPORTED_VERSIONS]
>   mac = ['mac-' + v for v in MacPort.SUPPORTED_VERSIONS]
>   win = ['win-' + v for v in WinPort.SUPPORTED_VERSIONS]
>   similar_ports = {}
>   for v in android:
>     similar_ports[v] = android + linux + mac + win
>   for v in linux:
>     similar_ports[v] = linux + android + mac + win
>   for v in mac:
>     similar_ports[v] = mac + linux + android + win
>   for v in win:
>     similar_ports[v] = win + linux + android + mac
>   return similar_ports
> 
> My main thought here is just that, for example, if we just need to fill in one
> mac port, then we should default to using another mac result first; but if
there
> are no other mac builds available, then probably trying to use any result is
> OK...
> 
> 
> Does that sound roughly correct? I'm hoping I can use something a little
simpler
> than the above.

Yes, something like this sounds correct to me. Perhaps checkpoint with drott@ as
well for another opinion since he spends a lot of time with cross platform font
baselines. Doing this as a separate change makes sense to me.

https://codereview.chromium.org/2803143002/diff/60001/third_party/WebKit/Tool...
File third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py
(right):

https://codereview.chromium.org/2803143002/diff/60001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py:41:
help='If some platforms have no try job results, use results from try job
results of other platforms.'),
Will the script provide understandable log output so that dev can see what we
did fallback-wise where needed for this case? I could imagine folks wanting to
know explicitly what baselines were used to fill in missing for which, etc.

https://codereview.chromium.org/2803143002/diff/60001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py:234:
For a each test prefix, if there is entry missing for some port,
rm 'a' before 'each', add 'an' before 'entry'

[qyearsley, 2017-04-11 23:23:20]

The CQ bit was checked by qyearsley@chromium.org

[qyearsley, 2017-04-11 23:23:21]

https://codereview.chromium.org/2803143002/diff/20001/third_party/WebKit/Tool...
File third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py
(right):

https://codereview.chromium.org/2803143002/diff/20001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py:242: #
TODO(qyearsley): Decide what build to use for a given port
On 2017/04/11 at 20:36:49, wkorman wrote:
> On 2017/04/10 23:52:10, qyearsley wrote:
> > Does that sound roughly correct? I'm hoping I can use something a little
simpler
> > than the above.
> 
> Yes, something like this sounds correct to me. Perhaps checkpoint with drott@
as well for another opinion since he spends a lot of time with cross platform
font baselines. Doing this as a separate change makes sense to me.

Sounds good, will do :-)

https://codereview.chromium.org/2803143002/diff/60001/third_party/WebKit/Tool...
File third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py
(right):

https://codereview.chromium.org/2803143002/diff/60001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py:41:
help='If some platforms have no try job results, use results from try job
results of other platforms.'),
On 2017/04/11 at 20:36:49, wkorman wrote:
> Will the script provide understandable log output so that dev can see what we
did fallback-wise where needed for this case? I could imagine folks wanting to
know explicitly what baselines were used to fill in missing for which, etc.

Yeah, I think it should; I'm not sure if it might be too verbose to log all of
the "filling in" activity at "info" level (rather than "debug" level), since the
same message might be repeated for 100s of tests, but for now I think I'd like
to err on the side of making things too verbose.

https://codereview.chromium.org/2803143002/diff/60001/third_party/WebKit/Tool...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py:234:
For a each test prefix, if there is entry missing for some port,
On 2017/04/11 at 20:36:49, wkorman wrote:
> rm 'a' before 'each', add 'an' before 'entry'

Done.

[qyearsley, 2017-04-11 23:23:22]

The patchset sent to the CQ was uploaded after l-g-t-m from wkorman@chromium.org
Link to the patchset: https://codereview.chromium.org/2803143002/#ps100001
(title: "Undo some changes")

[commit-bot: I haz the power, 2017-04-11 23:24:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-12 01:26:27]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1491953000795400,
"parent_rev": "36300286d016d77f699e5323339d3299b7b61761", "commit_rev":
"fb23c8091deec6697b7855fe4f40a6be9f9a604b"}

[commit-bot: I haz the power, 2017-04-12 01:27:17]

Description was changed from

==========
Support filling in baselines from other platforms in rebaseline-cl.

BUG=673966
==========

to

==========
Support filling in baselines from other platforms in rebaseline-cl.

BUG=673966

Review-Url: https://codereview.chromium.org/2803143002
Cr-Commit-Position: refs/heads/master@{#463877}
Committed:
https://chromium.googlesource.com/chromium/src/+/fb23c8091deec6697b7855fe4f40...
==========

[commit-bot: I haz the power, 2017-04-12 01:27:18]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/fb23c8091deec6697b7855fe4f40...