Merge lp:~vila/bzr/selftest-fixes into lp:~bzr/bzr/trunk-old

This patch implements a better balancing algorithm for selftest --parallel=fork.

It leads to ~25% smaller elapsed time on 4-core (8 threads) host.
Or said otherwise, running the full test suite goes down from 4 minutes to 3 minutes.

I also added a '-Eslices' that helps understand how the tests are distributed.

I'm fairly sure that this breaks compatibility with --parallel=ec2.

I think the concept is useful but please consider how to fit in with that plugin, and on windows, where external processes are much more expensive to start.

>>>>> "robert" == Robert Collins <email address hidden> writes:

    robert> Review: Needs Fixing

    robert> I'm fairly sure that this breaks compatibility with
    robert> --parallel=ec2.

Right, care to give it a try ? You know it's far easier for you
than for me.

I presume your suspicion comes from the fact that the plugin uses
fork_for_tests right ?

    robert> I think the concept is useful but please consider how
    robert> to fit in with that plugin, and on windows, where
    robert> external processes are much more expensive to start.

Using a process for each slice was simpler than starting one
process by processor and then implementing a way to send the
slices via another socket/pipe, so I tried that approach a bit
and punted.

If you played a bit with -Eslices, you noticed that, indeed, many
processes are spawned (one by slice) and avoiding them can give
even better performances.

Bug given that the test suite is *not* fully running so far on
windows, being able to run even part of it, faster is still a win IMHO.

And even if starting new processes is slow, I doubt it can be slower
to run 8 parallel newly started processes than a single started
once process :).

Hmm, I don't doubt it, I know in fact, the overhead of starting
new processes and handling the results via subunit is high, even
on linux, yet --parallel=fork reduces the *elapsed* time
enormously. Unless that can't reproduced on windows, I far prefer
being able to reduce the elapsed time than delay using -parallel=fork there.

Would you be ok if instead of modifying fork_for_tests I create a
new fork_balanced_for_tests until we can reconcile both ?

On Mon, 2009-08-24 at 06:18 +0000, Vincent Ladeuil wrote:
>
> robert> I think the concept is useful but please consider how
> robert> to fit in with that plugin, and on windows, where
> robert> external processes are much more expensive to start.
>
> Using a process for each slice was simpler than starting one
> process by processor and then implementing a way to send the
> slices via another socket/pipe, so I tried that approach a bit
> and punted.

A chatty protocol has its own issues indeed, including not being
friendly to xmlrpc apis and so on.

> If you played a bit with -Eslices, you noticed that, indeed, many
> processes are spawned (one by slice) and avoiding them can give
> even better performances.

> Bug given that the test suite is *not* fully running so far on
> windows, being able to run even part of it, faster is still a win
> IMHO.

Sure, I'm not debating that. Its only slightly faster though ;). Running
with --randomize should give approximately the same performance boost I
think.

> And even if starting new processes is slow, I doubt it can be slower
> to run 8 parallel newly started processes than a single started
> once process :).

On EC2 bringing up a warm machine takes about 60 seconds. Bringing up a
fresh machine takes about 8-10 minutes. Each machine then has 8 CPU's at
its disposal.

> Hmm, I don't doubt it, I know in fact, the overhead of starting
> new processes and handling the results via subunit is high, even
> on linux, yet --parallel=fork reduces the *elapsed* time
> enormously. Unless that can't reproduced on windows, I far prefer
> being able to reduce the elapsed time than delay using -parallel=fork
> there.
>
> Would you be ok if instead of modifying fork_for_tests I create a
> new fork_balanced_for_tests until we can reconcile both ?

Its not fork_for_tests, its the helper classes.

ec2test is easy to setup, docs are in the developer tree:
install the plugin.
. ~/.aws
install boto [support library]
./bzr push; ./bzr --selftest --parallel=ec2

-Rob

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

...
>
> Bug given that the test suite is *not* fully running so far on
> windows, being able to run even part of it, faster is still a win IMHO.
>
> And even if starting new processes is slow, I doubt it can be slower
> to run 8 parallel newly started processes than a single started
> once process :).
>
> Hmm, I don't doubt it, I know in fact, the overhead of starting
> new processes and handling the results via subunit is high, even
> on linux, yet --parallel=fork reduces the *elapsed* time
> enormously. Unless that can't reproduced on windows, I far prefer
> being able to reduce the elapsed time than delay using -parallel=fork there.
>
> Would you be ok if instead of modifying fork_for_tests I create a
> new fork_balanced_for_tests until we can reconcile both ?

I'll mention that "os.fork()" doesn't exist on Windows, you have to
spawn a subprocess and pass it data.

I don't know if this patch effects those cases or not. But if the
problem is that you're worried about *fork* performance on Windows, it
is, indeed awful :).

If you are spawning a subprocess, and passing it data, then it seems
like most of the work is already done, and you just need that subprocess
to check its input when it is finished, to see if there is more to do....

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkqSqnsACgkQJdeBCYSNAAMjsgCfYLMIv7i/G2iPRBL8pnAikHoB
oUYAn2a9sqHXlSVw4oNmat+TWhz19G5y
=Ydab
-----END PGP SIGNATURE-----