Add tool to use the manifest, fetch and cache dependencies and run the test.

Add tool to use the manifest, fetch and cache dependencies and run the test.

Only keep functions needed on the slave in tree_creator.py. Move the rest to
isolate.py.

R=rogerta@chromium.org
BUG=98834
TEST=Run from the ouput directory:
    GYP_DEFINES="$GYP_DEFINES tests_run=hashtable" ../../build/gyp_chromium; \
    ninja base_unittests_run; \
    ../../tools/isolate/tree_creator.py -m base_unittests.results --remote .

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=129455

[M-A Ruel, 2012-03-24 22:58:05]

In theory, them swarm simply has to do this:
1. Package tree_creator.py and base_unittests.results. Nothing else!

2. Run tree_creator.py -m base_unittests.results --remote <server where the hash
are saved>

That will improve the efficiency since swarm won't have gigs of data to pass
around to run browser_tests for example.

Chris, any opinion on this?

https://chromiumcodereview.appspot.com/9834090/diff/1001/tools/isolate/isolat...
File tools/isolate/isolate.py (right):

https://chromiumcodereview.appspot.com/9834090/diff/1001/tools/isolate/isolat...
tools/isolate/isolate.py:55: def expand_directories(indir, infiles, blacklist):
I just moved these functions from tree_creator.py since they weren't necessary
there.

[csharp, 2012-03-26 13:46:32]

Sounds good to me.

My only issue is that some of the swarm machines have less then 20gigs of total
space, so they wouldn't ever reach the max cache size but they would often fall
below the minimum of 1gig free space. Looking at the code I don't think that
would be too bad, but I wonder if it would make sense to let the max cache size
be a machine specific value?

[Roger Tawa OOO till Jul 10th, 2012-03-26 14:50:40]

a couple of questions below.

http://codereview.chromium.org/9834090/diff/1001/tools/isolate/tree_creator.py
File tools/isolate/tree_creator.py (right):

http://codereview.chromium.org/9834090/diff/1001/tools/isolate/tree_creator.p...
tools/isolate/tree_creator.py:177: self.MIN_FREE_SPACE and
why is this in the condition?

http://codereview.chromium.org/9834090/diff/1001/tools/isolate/tree_creator.p...
tools/isolate/tree_creator.py:183: while self.MAX_SIZE and self.state and
self.total_size() > self.MAX_SIZE:
do you really want to call total_size() each time through the loop?

http://codereview.chromium.org/9834090/diff/1001/tools/isolate/tree_creator.p...
tools/isolate/tree_creator.py:219: def run_tha_test(manifest, cache_dir,
remote):
i assume the typo is intentional :-)

[M-A Ruel, 2012-03-26 15:55:59]

http://codereview.chromium.org/9834090/diff/1001/tools/isolate/tree_creator.py
File tools/isolate/tree_creator.py (right):

http://codereview.chromium.org/9834090/diff/1001/tools/isolate/tree_creator.p...
tools/isolate/tree_creator.py:177: self.MIN_FREE_SPACE and
On 2012/03/26 14:50:40, Roger Tawa wrote:
> why is this in the condition?

I explained in the __init__ docstring. 0 is a legal value.

http://codereview.chromium.org/9834090/diff/1001/tools/isolate/tree_creator.p...
tools/isolate/tree_creator.py:183: while self.MAX_SIZE and self.state and
self.total_size() > self.MAX_SIZE:
On 2012/03/26 14:50:40, Roger Tawa wrote:
> do you really want to call total_size() each time through the loop?

No.

http://codereview.chromium.org/9834090/diff/1001/tools/isolate/tree_creator.p...
tools/isolate/tree_creator.py:219: def run_tha_test(manifest, cache_dir,
remote):
On 2012/03/26 14:50:40, Roger Tawa wrote:
> i assume the typo is intentional :-)

I didn't have enough imagination to figure out a good function name. I'm not
happy with "tree_creator" either. Any suggestion?

http://codereview.chromium.org/9834090/diff/5001/tools/isolate/tree_creator.py
File tools/isolate/tree_creator.py (right):

http://codereview.chromium.org/9834090/diff/5001/tools/isolate/tree_creator.p...
tools/isolate/tree_creator.py:117: def download_or_copy(file_or_url, dest):
Technically, this could be turned into an API so that Cache() would not have to
know if it is an url, a NFS share or some other object file system. In practice,
this didn't seem to be (yet) required so I decided to keep the design simpler
here.

http://codereview.chromium.org/9834090/diff/6003/tools/isolate/tree_creator.py
File tools/isolate/tree_creator.py (right):

http://codereview.chromium.org/9834090/diff/6003/tools/isolate/tree_creator.p...
tools/isolate/tree_creator.py:192: sizes = [os.stat(self.path(f)).st_size for f
in self.state]
This could be saved in self.state if this is found to be too slow.

But I'd rather not do that so that there can't be any cache discrepancy about
what's on disk and what the cache thinks is the size of each object.

[M-A Ruel, 2012-03-27 00:35:23]

On 2012/03/26 13:46:32, csharp wrote:
> Sounds good to me.
> 
> My only issue is that some of the swarm machines have less then 20gigs of
total
> space, so they wouldn't ever reach the max cache size but they would often
fall
> below the minimum of 1gig free space. Looking at the code I don't think that
> would be too bad, but I wonder if it would make sense to let the max cache
size
> be a machine specific value?

I made it an argument but I don't think it's useful to have this value be saved
anywhere. Its sole purpose is to not turn the cache into a leak. In the 20gb
disk space case, the cache will simply be bound by the free disk space, which is
fine.

[Roger Tawa OOO till Jul 10th, 2012-03-27 15:30:08]

lgtm

[commit-bot: I haz the power, 2012-03-28 17:23:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/maruel@chromium.org/9834090/12001