Bug 94665 – Baseline optimizer should try to optimize per-port if global optimization fails

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Kenneth Russell]

In http://trac.webkit.org/changeset/126225 failing Mountain Lion text results were checked in to platform/mac/ which caused failures on all Chromium ports. dpranke updated webkit-patch to generate correct rebaselines for the Chromium port (i.e., to not destroy the new results in platform/mac/), but the baseline optimizer doesn't handle this situation well and generates fairly redundant baselines. For fast/canvas/canvas-scale-shadowBlur.html, it produces the following rebaselines:

  LayoutTests/platform/chromium-linux-x86/fast/canvas/canvas-scale-shadowBlur-expected.txt
  LayoutTests/platform/chromium-linux/fast/canvas/canvas-scale-shadowBlur-expected.txt
  LayoutTests/platform/chromium-mac-snowleopard/fast/canvas/canvas-scale-shadowBlur-expected.txt
  LayoutTests/platform/chromium-mac/fast/canvas/canvas-scale-shadowBlur-expected.txt
  LayoutTests/platform/chromium-win-xp/fast/canvas/canvas-scale-shadowBlur-expected.txt
  LayoutTests/platform/chromium-win/fast/canvas/canvas-scale-shadowBlur-expected.txt

where ideally it would just produce a result into LayoutTests/platform/chromium/ .

It looks like in order for this to work, the baseline optimizer's algorithm would have to be changed to try to optimize the results just within a given port (i.e., all of the chromium-related directories) if the global optimization heuristics fail.

[Dirk Pranke]

to provide a little more detail, what happens is that mountain lion fails differently from every other port, so we have two results, X, and Y.

The baseline optimizer bins mountain-lion into one bucket, and all of the other ports into the other bucket; so the most_common_specific_directory for all of the other ports is the generic LayoutTests directory; this result is wrong, because it fails to account for the fact that chromium (and mac-lion and mac-snowleopard) won't see that result, it'll use the ML result.

although I spent some time thinking about it, it wasn't obvious to me how to change the algorithm in baselineoptimizer to accomodate this. It seemed like we might have to complete redo the approach, but maybe I'm missing something?

[Kenneth Russell]

Committed r126255: <http://trac.webkit.org/changeset/126255>

[Brady Eidson]

(In reply to comment #0)
> In http://trac.webkit.org/changeset/126225 failing Mountain Lion text results were checked in to platform/mac/ which caused failures on all Chromium ports. 

I'm sorry for the failures.  The explanation is simple - I was working on a rebaselining effort believed to only affect Apple ports.

I (and others at Apple) expected that mac and mac-* were our directories that we owned.  There might be some at Apple that knew that chromium pulled from "our" directory but that knowledge has not been spread here institutionally.

Was there a point in time where chromium started pulling from the mac results, and it hadn't before?

I'm quite curious why this sharing takes place.  While I can believe - for example - that chromium-mountain-lion might differ from the generic results for some tests the exact same way mac-mountain-lion does, I instinctually suspect that chromium would have more differences than commonalities when compared to generic.

I might be completely wrong about this.

Again, apologies for the noise...  Just a warning though.  Now that we have mountain-lion bots chugging away at build.webkit.org there is going to be a concerted effort over the coming days/weeks to continue getting them greener and this type of thing is bound to happen some more.

[Dirk Pranke]

No, chromium has always tried to match the mac baselines where possible. There are two reasons for this: first, the Chromium mac port was explicitly trying to match the Apple Mac port. Second, it's simply the case that we produce a lot of the same baselines (thousands, last I checked), and so there's a significant advantage to sharing.

That said, you're not doing anything wrong. The tools are simply busted; if they were working correctly you'd be able to rebaseline to your heart's content and we'd simply adjust automatically.

It may also be that if fixing the tools turns out to be hard, we should stop falling back to mac and just accept the duplicated baselines, but I don't think we're quite there yet.

[Brady Eidson]

(In reply to comment #4)
> No, chromium has always tried to match the mac baselines where possible. There are two reasons for this: first, the Chromium mac port was explicitly trying to match the Apple Mac port. Second, it's simply the case that we produce a lot of the same baselines (thousands, last I checked), and so there's a significant advantage to sharing.
> 
> That said, you're not doing anything wrong. The tools are simply busted; if they were working correctly you'd be able to rebaseline to your heart's content and we'd simply adjust automatically.
> 
> It may also be that if fixing the tools turns out to be hard, we should stop falling back to mac and just accept the duplicated baselines, but I don't think we're quite there yet.

Thanks for the explanation!

[Adam Barth]

I think we should change chromium to no longer fall back through platform/mac.  We did this originally when bringing up the chromium port because platform/mac had many baselines that worked for Chromium as well, but now I think it causes more trouble than it's worth.

[Dirk Pranke]

I am running some scripts now to figure out what the impact of not falling back through platform/mac would be (how many baselines we'd need to copy down to platform/chromium). It might be that this is the right long-term thing to do regardless of improving the optimizer algorithm or not ...

[Adam Barth]

Another approach Dirk and I discussed was adding a second optimization strategy that iterates using least_common_specific_directory rather than trying to optimize all-at-once with most_common_specific_directory.

[Dirk Pranke]

Created attachment 160052 [details]
Patch

[Adam Barth]

Comment on attachment 160052 [details]
Patch

This seems fine.  Would you be willing to convert _optimize_by_pushing_results_up to use a loop rather than recursion?

[Dirk Pranke]

(In reply to comment #10)
> (From update of attachment 160052 [details])
> This seems fine.  Would you be willing to convert _optimize_by_pushing_results_up to use a loop rather than recursion?

Will do.

[Dirk Pranke]

Created attachment 160054 [details]
patch for landing w/o recursion

[Dirk Pranke]

Committed r126374: <http://trac.webkit.org/changeset/126374>

[Dirk Pranke]

http://trac.webkit.org/changeset/126380 fixes a couple of issues with the r126374.

[Kenneth Russell]

FYI this new algorithm worked great optimizing yesterday's tricky case; see http://trac.webkit.org/changeset/126382 .