Bug 87538 – webkit-patch optimize-baselines should add/delete files in batches from the vcs

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dirk Pranke]

webkit-patch optimize-baselines should add/delete files in batches from the vcs

[Dirk Pranke]

Created attachment 144153 [details]
Patch

[Ojan Vafai]

Comment on attachment 144153 [details]
Patch

Wow. That was easy. I suppose this is covered by existing tests? If so, the changelog description should say so.

[Ojan Vafai]

Do we pass all the tests from garden-o-matic to a single "webkit-patch optimize-baselines"?

[Dirk Pranke]

(In reply to comment #2)
> (From update of attachment 144153 [details])
> Wow. That was easy. I suppose this is covered by existing tests? If so, the changelog description should say so.

I will double-check the test coverage and update the change log (and add tests) accordingly.

We optimize one test at a time. 

If we start to rebaseline multiple tests at once using garden-o-matic (or whatever) we will probably want to either extend optimize-baselines to take a list of tests or defer the scm modifications and do it all at once at the end like we're gonna do with rebaseline-test.

[Ojan Vafai]

(In reply to comment #4)
> (In reply to comment #2)
> > (From update of attachment 144153 [details] [details])
> > Wow. That was easy. I suppose this is covered by existing tests? If so, the changelog description should say so.
> 
> I will double-check the test coverage and update the change log (and add tests) accordingly.
> 
> We optimize one test at a time. 
> 
> If we start to rebaseline multiple tests at once using garden-o-matic (or whatever) we will probably want to either extend optimize-baselines to take a list of tests or defer the scm modifications and do it all at once at the end like we're gonna do with rebaseline-test.

Makes sense. I suppose it's rare that garden-o-matic does multiple tests at once. Although, I've had times when I've queued up tons of rebaselines that in theory could be batched. But maybe with all these performance improvements it won't matter.

[Adam Barth]

Comment on attachment 144153 [details]
Patch

It's important to do all the deletes before the adds, which you've done.  Thanks.

[Dirk Pranke]

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #2)
> > > (From update of attachment 144153 [details] [details] [details])
> > > Wow. That was easy. I suppose this is covered by existing tests? If so, the changelog description should say so.
> > 
> > I will double-check the test coverage and update the change log (and add tests) accordingly.
> > 
> > We optimize one test at a time. 
> > 
> > If we start to rebaseline multiple tests at once using garden-o-matic (or whatever) we will probably want to either extend optimize-baselines to take a list of tests or defer the scm modifications and do it all at once at the end like we're gonna do with rebaseline-test.
> 
> Makes sense. I suppose it's rare that garden-o-matic does multiple tests at once. Although, I've had times when I've queued up tons of rebaselines that in theory could be batched. But maybe with all these performance improvements it won't matter.

I think there still could be wins from pipelining multiple tests through and batching more things. Since the 'rebaselineall' http entry point does take multiple lists, it won't be hard to extend the backend if we need it to. Even with all of these improvements, Rebaselining still is multiple seconds per test over a fast network. We should have somebody test this from asia or europe to see how it is from there.

[Ojan Vafai]

I nominate Noel once this and https://bugs.webkit.org/show_bug.cgi?id=87451 are committed.

[noel gordon]

Happy to do it.  Let me know what you need when you're ready.

[Dirk Pranke]

Committed r118838: <http://trac.webkit.org/changeset/118838>

[noel gordon]

Created attachment 146485 [details]
webkit-patch-rebaseline-expectations.txt

Timing: rebaseline one test from AEST time zone.

[Ojan Vafai]

(In reply to comment #11)
> Created an attachment (id=146485) [details]
> webkit-patch-rebaseline-expectations.txt
> 
> Timing: rebaseline one test from AEST time zone.

Unfortunately, webkit-patch rebaseline-expectations doesn't yet do the rebaselines in parallel. We only do them in parallel from garden-o-matic. So, this 21 seconds is the time to do it serially. Looking at the output here, doing the rebaselines in parallel should cut the time down to ~5 seconds.

[noel gordon]

5 seconds sounds good.  "Let me know what you need when you're ready." ... and so with no news, I just punted that webkit-patch rebaseline-expectations had the new fancy :)