Add Subzero to scons tests on the bots. x86-32 only.

Add Subzero to scons tests on the bots.  x86-32 only.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4092
R=jfb@chromium.org, jvoung@chromium.org

Committed: https://chromium.googlesource.com/native_client/src/native_client/+/6ebcf753eb77446acd8767fee4ef72b8b8cc01a9

[Jim Stichnoth, 2015-03-29 03:12:49]

stichnot@chromium.org changed reviewers:
+ dschuff@chromium.org, jfb@chromium.org, jvoung@chromium.org

[Jim Stichnoth, 2015-03-29 03:12:56]

https://codereview.chromium.org/1037183002/diff/60001/tests/benchmark/nacl.scons
File tests/benchmark/nacl.scons (right):

https://codereview.chromium.org/1037183002/diff/60001/tests/benchmark/nacl.sc...
tests/benchmark/nacl.scons:16: is_broken = is_broken or env.Bit('use_sz')
Would be nice if we could bump the timeout for this test only so that we could
still test correctness.  time_error?  scale_timeout?

[JF, 2015-03-29 05:46:09]

lgtm otherwise.

https://codereview.chromium.org/1037183002/diff/60001/buildbot/buildbot_pnacl.py
File buildbot/buildbot_pnacl.py (right):

https://codereview.chromium.org/1037183002/diff/60001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:63: if arch == 'x86-32':
Is this the duplication dschuff was mentioning? Leave a TODO to remove it?

[jvoung (off chromium), 2015-03-30 17:56:15]

lgtm

https://codereview.chromium.org/1037183002/diff/60001/buildbot/buildbot_pnacl.py
File buildbot/buildbot_pnacl.py (right):

https://codereview.chromium.org/1037183002/diff/60001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:76: # along with the translate_fast flag once
pnacl-sz.nexe is ready.
pnacl-sz.nexe should work now too, but could toggle that separately

https://codereview.chromium.org/1037183002/diff/60001/tests/benchmark/nacl.scons
File tests/benchmark/nacl.scons (right):

https://codereview.chromium.org/1037183002/diff/60001/tests/benchmark/nacl.sc...
tests/benchmark/nacl.scons:16: is_broken = is_broken or env.Bit('use_sz')
On 2015/03/29 03:12:56, stichnot wrote:
> Would be nice if we could bump the timeout for this test only so that we could
> still test correctness.  time_error?  scale_timeout?

Could also set "size=huge" (for this case).

size=large gives you 60 seconds * scale_timeout, and huge gives you 1800 *
scale_timeout

Despite the fact that this is under 'large_tests', size still defaults to
'small' which is 2 seconds?

This was under large_tests primarily to avoid running the test in parallel with
other stuff (since this is a benchmark).

[Jim Stichnoth, 2015-03-30 20:36:53]

https://codereview.chromium.org/1037183002/diff/60001/buildbot/buildbot_pnacl.py
File buildbot/buildbot_pnacl.py (right):

https://codereview.chromium.org/1037183002/diff/60001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:63: if arch == 'x86-32':
On 2015/03/29 05:46:09, JF wrote:
> Is this the duplication dschuff was mentioning? Leave a TODO to remove it?

I think the duplication was in a different set of tests...

The trybot says that about 11 minutes is spent on the original build_all step,
followed by about 3 minutes on this build_all subzero step, so I don't think
there's duplication here (i.e., the pexes are being reused).

https://codereview.chromium.org/1037183002/diff/60001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:76: # along with the translate_fast flag once
pnacl-sz.nexe is ready.
On 2015/03/30 17:56:15, jvoung wrote:
> pnacl-sz.nexe should work now too, but could toggle that separately

OK.  I think I'd like to deal with pnacl-sz.nexe in a separate CL.  (for this
and the other relevant tests)

https://codereview.chromium.org/1037183002/diff/60001/tests/benchmark/nacl.scons
File tests/benchmark/nacl.scons (right):

https://codereview.chromium.org/1037183002/diff/60001/tests/benchmark/nacl.sc...
tests/benchmark/nacl.scons:16: is_broken = is_broken or env.Bit('use_sz')
On 2015/03/30 17:56:15, jvoung wrote:
> On 2015/03/29 03:12:56, stichnot wrote:
> > Would be nice if we could bump the timeout for this test only so that we
could
> > still test correctness.  time_error?  scale_timeout?
> 
> Could also set "size=huge" (for this case).
> 
> size=large gives you 60 seconds * scale_timeout, and huge gives you 1800 *
> scale_timeout
> 
> Despite the fact that this is under 'large_tests', size still defaults to
> 'small' which is 2 seconds?
> 
> This was under large_tests primarily to avoid running the test in parallel
with
> other stuff (since this is a benchmark).

Managed to find a way to bump the timeout for this one test (thanks Derek!).

[Derek Schuff, 2015-03-30 21:11:16]

https://codereview.chromium.org/1037183002/diff/60001/buildbot/buildbot_pnacl.py
File buildbot/buildbot_pnacl.py (right):

https://codereview.chromium.org/1037183002/diff/60001/buildbot/buildbot_pnacl...
buildbot/buildbot_pnacl.py:63: if arch == 'x86-32':
On 2015/03/30 20:36:53, stichnot wrote:
> The trybot says that about 11 minutes is spent on the original build_all step,
> followed by about 3 minutes on this build_all subzero step, so I don't think
> there's duplication here (i.e., the pexes are being reused).

I don't see how that's possible when the scons-out directory is totally separate
(see e.g.
scons-out/nacl-x86-32-pnacl-subzero-pexe-clang/obj/tests/hello_world/hello_world.final.pexe
vs
scons-out/nacl-x86-32-pnacl-pexe-clang/obj/tests/hello_world/hello_world.final.pexe).
3 vs 11 mins seems like more than you would expect for just faster translation
too... maybe there are more (or slower) tests than we would expect that require
non-finalized pexes?

[jvoung (off chromium), 2015-03-30 21:37:25]

On 2015/03/30 21:11:16, Derek Schuff wrote:
>
https://codereview.chromium.org/1037183002/diff/60001/buildbot/buildbot_pnacl.py
> File buildbot/buildbot_pnacl.py (right):
> 
>
https://codereview.chromium.org/1037183002/diff/60001/buildbot/buildbot_pnacl...
> buildbot/buildbot_pnacl.py:63: if arch == 'x86-32':
> On 2015/03/30 20:36:53, stichnot wrote:
> > The trybot says that about 11 minutes is spent on the original build_all
step,
> > followed by about 3 minutes on this build_all subzero step, so I don't think
> > there's duplication here (i.e., the pexes are being reused).
> 
> I don't see how that's possible when the scons-out directory is totally
separate
> (see e.g.
>
scons-out/nacl-x86-32-pnacl-subzero-pexe-clang/obj/tests/hello_world/hello_world.final.pexe
> vs
>
scons-out/nacl-x86-32-pnacl-pexe-clang/obj/tests/hello_world/hello_world.final.pexe).
> 3 vs 11 mins seems like more than you would expect for just faster translation
> too... maybe there are more (or slower) tests than we would expect that
require
> non-finalized pexes?

Right, I don't think the pexes are shared.

There is some sharing of trusted sel_ldr/validator stuff though:


**********************************************************************
COMMAND EXECUTION REPORT
**********************************************************************
// ...
 262 g++
 409 gcc
   1 nacl_bootstrap_munge_phdr.py
// ...
 196 pnacl-finalize
   2 pnacl-ld
  40 pnacl-ranlib
 247 pnacl-translate
//...

 vs

**********************************************************************
COMMAND EXECUTION REPORT
**********************************************************************
//...
   7 g++
   9 gcc
//...
 195 pnacl-finalize
   1 pnacl-ld
  26 pnacl-ranlib
 246 pnacl-translate
//...

So the first "build_all" step is building all the trusted code and the IRT,
which is shared.

Also, looks like every "build_all $arch" builds ARM and MIPS validator + tests:

Install file:
"scons-out/opt-linux-x86-32/obj/src/trusted/validator_mips/libncvalidate_mips.a"
as "scons-out/opt-linux-x86-32/lib/libncvalidate_mips.a"

Install file:
"scons-out/opt-linux-x86-32/obj/src/trusted/validator_arm/libncvalidate_arm_v2.a"
as "scons-out/opt-linux-x86-32/lib/libncvalidate_arm_v2.a"


Which seems a bit wasteful... especially since the x86-64-toolchain bot does
that for three times.

There is "skip_trusted_tests" which at least skips running the tests, but it
would be nice to be able to only build those validators when platform=arm or
platform=mips is specified.

[Jim Stichnoth, 2015-03-30 21:48:44]

On 2015/03/30 21:37:25, jvoung wrote:
> On 2015/03/30 21:11:16, Derek Schuff wrote:
> >
>
https://codereview.chromium.org/1037183002/diff/60001/buildbot/buildbot_pnacl.py
> > File buildbot/buildbot_pnacl.py (right):
> > 
> >
>
https://codereview.chromium.org/1037183002/diff/60001/buildbot/buildbot_pnacl...
> > buildbot/buildbot_pnacl.py:63: if arch == 'x86-32':
> > On 2015/03/30 20:36:53, stichnot wrote:
> > > The trybot says that about 11 minutes is spent on the original build_all
> step,
> > > followed by about 3 minutes on this build_all subzero step, so I don't
think
> > > there's duplication here (i.e., the pexes are being reused).
> > 
> > I don't see how that's possible when the scons-out directory is totally
> separate
> > (see e.g.
> >
>
scons-out/nacl-x86-32-pnacl-subzero-pexe-clang/obj/tests/hello_world/hello_world.final.pexe
> > vs
> >
>
scons-out/nacl-x86-32-pnacl-pexe-clang/obj/tests/hello_world/hello_world.final.pexe).
> > 3 vs 11 mins seems like more than you would expect for just faster
translation
> > too... maybe there are more (or slower) tests than we would expect that
> require
> > non-finalized pexes?
> 
> Right, I don't think the pexes are shared.
> 
> There is some sharing of trusted sel_ldr/validator stuff though:
> 
> 
> **********************************************************************
> COMMAND EXECUTION REPORT
> **********************************************************************
> // ...
>  262 g++
>  409 gcc
>    1 nacl_bootstrap_munge_phdr.py
> // ...
>  196 pnacl-finalize
>    2 pnacl-ld
>   40 pnacl-ranlib
>  247 pnacl-translate
> //...
> 
>  vs
> 
> **********************************************************************
> COMMAND EXECUTION REPORT
> **********************************************************************
> //...
>    7 g++
>    9 gcc
> //...
>  195 pnacl-finalize
>    1 pnacl-ld
>   26 pnacl-ranlib
>  246 pnacl-translate
> //...
> 
> So the first "build_all" step is building all the trusted code and the IRT,
> which is shared.
> 
> Also, looks like every "build_all $arch" builds ARM and MIPS validator +
tests:
> 
> Install file:
>
"scons-out/opt-linux-x86-32/obj/src/trusted/validator_mips/libncvalidate_mips.a"
> as "scons-out/opt-linux-x86-32/lib/libncvalidate_mips.a"
> 
> Install file:
>
"scons-out/opt-linux-x86-32/obj/src/trusted/validator_arm/libncvalidate_arm_v2.a"
> as "scons-out/opt-linux-x86-32/lib/libncvalidate_arm_v2.a"
> 
> 
> Which seems a bit wasteful... especially since the x86-64-toolchain bot does
> that for three times.
> 
> There is "skip_trusted_tests" which at least skips running the tests, but it
> would be nice to be able to only build those validators when platform=arm or
> platform=mips is specified.

OK yeah, that makes sense.

[Jim Stichnoth, 2015-03-30 22:08:08]

Committed patchset #5 (id:80001) manually as
6ebcf753eb77446acd8767fee4ef72b8b8cc01a9 (presubmit successful).