We'd like to start chunking suites based on the average runtimes of each individual tests such that each chunk takes approximately the same amount of time.

/r/4371 - Bug 1137339 - [manifestparser] implement a chunk_by_runtime filter, r=jmaher Pull down this commit: hg pull review -r b305dc4f2985e74ec9cbdbf51f0026d9c2e7b577

This algorithm works by repeatedly taking the longest running manifest and its tests to the (current) shortest running chunk. On each iteration the shortest chunk is recalculated. This seems to give good results while still keeping tests of the same manifest together.

s/manifest and its tests/manifest and assigning its tests

Comment on attachment 8570016 [details] MozReview Request: bz://1137339/ahal https://reviewboard.mozilla.org/r/4369/#review3579 ::: testing/mozbase/manifestparser/manifestparser/filters.py (Diff revision 1) > + self.runtimes.update(runtimes) why do we do a defaultdict(int), then update(runtimes)? Does this take care of the case where runtimes in None or {} ? ::: testing/mozbase/manifestparser/manifestparser/filters.py (Diff revision 1) > + self.total = total I wish this was called total_chunks ::: testing/mozbase/manifestparser/manifestparser/filters.py (Diff revision 1) > + self.this = this this is confusing when you are familiar with other languages, can we call this current_chunk or this_chunk? ::: testing/mozbase/manifestparser/manifestparser/filters.py (Diff revision 1) > + tests_by_chunk[0][1].extend(batch) why are we only modifying tests_by_chunk[0] ? Is there a chance that the .sort() we do sorts it and we keep putting the next highest values in the chunks with the lowest runtime (at a start it is value of 0)? If I got that, it would be better to make the code a bit more self reading. ::: testing/mozbase/manifestparser/tests/test_chunking.py (Diff revision 1) > + { <dir>: <num tests> this is clear, but it should be a complete json object in the example.

(In reply to Joel Maher (:jmaher) from comment #4) > why do we do a defaultdict(int), then update(runtimes)? Does this take care > of the case where runtimes in None or {} ? No, defaultdict(int) means any keys that don't exist in the dictionary will default to a value of 0 if accessed. E.g: d = defaultdict(int) d['foo'] == 0 # True The idea is that tests that don't exist in the runtimes file are so quick that their runtimes essentially count for nothing. The tool that generates the runtimes files would decide whether to include negligible tests or not. > why are we only modifying tests_by_chunk[0] ? Is there a chance that the > .sort() we do sorts it and we keep putting the next highest values in the > chunks with the lowest runtime (at a start it is value of 0)? > > If I got that, it would be better to make the code a bit more self reading. Yes every iteration tests_by_chunk gets re-sorted so tests_by_chunk[0] is always the fastest chunk. I'm not sure how to make this more self reading, but I can add a comment.

Comment on attachment 8570016 [details] MozReview Request: bz://1137339/ahal /r/4371 - Bug 1137339 - [manifestparser] implement a chunk_by_runtime filter, r=jmaher /r/4719 - Address review comments Pull down these commits: hg pull review -r 6b4fdf41f64eae071a61b1e99169255e334ca959

Comment on attachment 8570016 [details] MozReview Request: bz://1137339/ahal https://reviewboard.mozilla.org/r/4369/#review3863 Ship It!

The test passes on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8a292ce1f520 Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/86e026f0d539