[swarming] Isolate webkit and optimize_for_size.

[swarming] Isolate webkit and optimize_for_size.

BUG=chromium:535160
LOG=n

Committed: https://crrev.com/e6d45f1a78ce6281c260cbb619682de3a7e367af
Cr-Commit-Position: refs/heads/master@{#31267}

[Michael Achenbach, 2015-10-14 14:51:25]

machenbach@chromium.org changed reviewers:
+ kjellander@chromium.org, tandrii@chromium.org

[Michael Achenbach, 2015-10-14 14:52:28]

PTAL

https://codereview.chromium.org/1402953004/diff/1/test/optimize_for_size.gyp
File test/optimize_for_size.gyp (right):

https://codereview.chromium.org/1402953004/diff/1/test/optimize_for_size.gyp#...
test/optimize_for_size.gyp:10: 'target_name': 'optimize_for_size_run',
This is a collection of four test suites.

[Michael Achenbach, 2015-10-14 14:52:32]

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

[commit-bot: I haz the power, 2015-10-14 14:52:37]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1402953004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1402953004/1

[tandrii(chromium), 2015-10-14 14:56:42]

lgtm

[Michael Achenbach, 2015-10-14 15:01:35]

The CQ bit was unchecked by machenbach@chromium.org

[Michael Achenbach, 2015-10-14 15:01:39]

The CQ bit was checked by machenbach@chromium.org

[commit-bot: I haz the power, 2015-10-14 15:01:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1402953004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1402953004/1

[kjellander_chromium, 2015-10-14 15:06:42]

https://codereview.chromium.org/1402953004/diff/1/test/webkit/webkit.isolate
File test/webkit/webkit.isolate (right):

https://codereview.chromium.org/1402953004/diff/1/test/webkit/webkit.isolate#...
test/webkit/webkit.isolate:1: # Copyright 2015 the V8 project authors. All
rights reserved.
Hm, I don't quite follow. This is identical to intl.isolate and which means
webkit_run in webkit.gyp is identical to intl_run in intl.gyp.

What's the difference?

[commit-bot: I haz the power, 2015-10-14 17:35:04]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-10-14 17:35:20]

Patchset 1 (id:??) landed as
https://crrev.com/e6d45f1a78ce6281c260cbb619682de3a7e367af
Cr-Commit-Position: refs/heads/master@{#31267}

[Michael Achenbach, 2015-10-15 08:41:25]

https://codereview.chromium.org/1402953004/diff/1/test/webkit/webkit.isolate
File test/webkit/webkit.isolate (right):

https://codereview.chromium.org/1402953004/diff/1/test/webkit/webkit.isolate#...
test/webkit/webkit.isolate:1: # Copyright 2015 the V8 project authors. All
rights reserved.
On 2015/10/14 15:06:42, kjellander (chromium) wrote:
> Hm, I don't quite follow. This is identical to intl.isolate and which means
> webkit_run in webkit.gyp is identical to intl_run in intl.gyp.
> 
> What's the difference?

The location of the file is the difference. The 'files' variable pointing to
'./' includes a different set of files, dependent on where this isolate file is
placed. I didn't manage to remove more boilerplate and shrink the files a bit
more. Each isolated test suite could in theory be sent to a different swarming
bot, where it needs the test files, the executable and our test runner.

It might be possible to generate the isolate files per test suite as part of the
compilation, but I think inventing that logic is more expensive than copying the
file 8 times.

[kjellander_chromium, 2015-10-15 10:31:25]

https://codereview.chromium.org/1402953004/diff/1/test/webkit/webkit.isolate
File test/webkit/webkit.isolate (right):

https://codereview.chromium.org/1402953004/diff/1/test/webkit/webkit.isolate#...
test/webkit/webkit.isolate:1: # Copyright 2015 the V8 project authors. All
rights reserved.
On 2015/10/15 08:41:25, Michael Achenbach wrote:
> On 2015/10/14 15:06:42, kjellander (chromium) wrote:
> > Hm, I don't quite follow. This is identical to intl.isolate and which means
> > webkit_run in webkit.gyp is identical to intl_run in intl.gyp.
> > 
> > What's the difference?
> 
> The location of the file is the difference. The 'files' variable pointing to
> './' includes a different set of files, dependent on where this isolate file
is
> placed. I didn't manage to remove more boilerplate and shrink the files a bit
> more. Each isolated test suite could in theory be sent to a different swarming
> bot, where it needs the test files, the executable and our test runner.
> 
> It might be possible to generate the isolate files per test suite as part of
the
> compilation, but I think inventing that logic is more expensive than copying
the
> file 8 times.

I thought the files listing was supposed to contain all the files needed to run
the test (i.e. the test executable and any other files it may load for the
test). I recall there's a way to auto-generate it by running the test wrapped in
a script. Adding all of a directory seems potentially wasteful (and it's less
clear to read too).

[Michael Achenbach, 2015-10-15 11:33:33]

On 2015/10/15 10:31:25, kjellander (chromium) wrote:
> https://codereview.chromium.org/1402953004/diff/1/test/webkit/webkit.isolate
> File test/webkit/webkit.isolate (right):
> 
>
https://codereview.chromium.org/1402953004/diff/1/test/webkit/webkit.isolate#...
> test/webkit/webkit.isolate:1: # Copyright 2015 the V8 project authors. All
> rights reserved.
> On 2015/10/15 08:41:25, Michael Achenbach wrote:
> > On 2015/10/14 15:06:42, kjellander (chromium) wrote:
> > > Hm, I don't quite follow. This is identical to intl.isolate and which
means
> > > webkit_run in webkit.gyp is identical to intl_run in intl.gyp.
> > > 
> > > What's the difference?
> > 
> > The location of the file is the difference. The 'files' variable pointing to
> > './' includes a different set of files, dependent on where this isolate file
> is
> > placed. I didn't manage to remove more boilerplate and shrink the files a
bit
> > more. Each isolated test suite could in theory be sent to a different
swarming
> > bot, where it needs the test files, the executable and our test runner.
> > 
> > It might be possible to generate the isolate files per test suite as part of
> the
> > compilation, but I think inventing that logic is more expensive than copying
> the
> > file 8 times.
> 
> I thought the files listing was supposed to contain all the files needed to
run
> the test (i.e. the test executable and any other files it may load for the
> test). I recall there's a way to auto-generate it by running the test wrapped
in
> a script. Adding all of a directory seems potentially wasteful (and it's less
> clear to read too).

We need 99% of the contents of this directory. Our isolate files are always at
the root of each test suite. Our test suites only contain data files, which are
all used. The executable (it's the same for all test suites) and the runner
scripts are referred to as dependencies.

[kjellander_chromium, 2015-10-15 12:04:09]

On 2015/10/15 11:33:33, Michael Achenbach wrote:
> On 2015/10/15 10:31:25, kjellander (chromium) wrote:
> > https://codereview.chromium.org/1402953004/diff/1/test/webkit/webkit.isolate
> > File test/webkit/webkit.isolate (right):
> > 
> >
>
https://codereview.chromium.org/1402953004/diff/1/test/webkit/webkit.isolate#...
> > test/webkit/webkit.isolate:1: # Copyright 2015 the V8 project authors. All
> > rights reserved.
> > On 2015/10/15 08:41:25, Michael Achenbach wrote:
> > > On 2015/10/14 15:06:42, kjellander (chromium) wrote:
> > > > Hm, I don't quite follow. This is identical to intl.isolate and which
> means
> > > > webkit_run in webkit.gyp is identical to intl_run in intl.gyp.
> > > > 
> > > > What's the difference?
> > > 
> > > The location of the file is the difference. The 'files' variable pointing
to
> > > './' includes a different set of files, dependent on where this isolate
file
> > is
> > > placed. I didn't manage to remove more boilerplate and shrink the files a
> bit
> > > more. Each isolated test suite could in theory be sent to a different
> swarming
> > > bot, where it needs the test files, the executable and our test runner.
> > > 
> > > It might be possible to generate the isolate files per test suite as part
of
> > the
> > > compilation, but I think inventing that logic is more expensive than
copying
> > the
> > > file 8 times.
> > 
> > I thought the files listing was supposed to contain all the files needed to
> run
> > the test (i.e. the test executable and any other files it may load for the
> > test). I recall there's a way to auto-generate it by running the test
wrapped
> in
> > a script. Adding all of a directory seems potentially wasteful (and it's
less
> > clear to read too).
> 
> We need 99% of the contents of this directory. Our isolate files are always at
> the root of each test suite. Our test suites only contain data files, which
are
> all used. The executable (it's the same for all test suites) and the runner
> scripts are referred to as dependencies.

Fair enough, I mostly wanted to check that I hadn't misunderstood this.
lgtm!