Code review comment for lp:~vila/bzr/selftest-fixes

Revision history for this message
Robert Collins (lifeless) wrote :

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

« Back to merge proposal