Summary: | nrwt should use multiple http shards | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kristóf Kosztyó <kkristof> | ||||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, dpranke, ojan, ossy, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 63116 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Kristóf Kosztyó
2012-01-10 06:40:24 PST
Sure, that seems fine. Feel free to add a patch, or I will get around to it sooner or later. Created attachment 121992 [details]
add --max-locked-shard option
I added the option to set it manual so now we can test it easier.
Comment on attachment 121992 [details] add --max-locked-shard option View in context: https://bugs.webkit.org/attachment.cgi?id=121992&action=review LGTM. Any objection? > Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:383 > + help="Set the maximum of locked shards"), only a little nit: "Maximum number of locked shards" Comment on attachment 121992 [details] add --max-locked-shard option View in context: https://bugs.webkit.org/attachment.cgi?id=121992&action=review > Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:679 > + if self._options.max_locked_shards: > + num_of_locked_shards = self._options.max_locked_shards > + else: > + num_of_locked_shards = 1 How about adding a boolean option instead (e.g. --shard-http-tests)? Then we can return "max(math.ceil(num_workers / 4.0), 1)" and tweak that math until we get reasonable stability. (In reply to comment #4) > How about adding a boolean option instead (e.g. --shard-http-tests)? Then we can return "max(math.ceil(num_workers / 4.0), 1)" and tweak that math until we get reasonable stability. I prefer the integer parameter, which let us use what we want. max(math.ceil(num_workers / 4.0) wouldn't be good for 4 or 8 workers, because 1 or 2 http shards isn't so useful ... I think num_of_locked_shards should be an integer option now, and we can move forward to increase the default value of it. Created attachment 122223 [details]
add --max-locked-shard option
I fixed the grammar issues :)
Comment on attachment 122223 [details]
add --max-locked-shard option
Let's go forward, r=me.
Comment on attachment 122223 [details]
add --max-locked-shard option
r-, because it broke a unittest. :-/
ERROR: test_shard_by_dir (webkitpy.layout_tests.controllers.manager_unittest.ShardingTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/oszi/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py", line 82, in test_shard_by_dir
locked, unlocked = self.get_shards(num_workers=2, fully_parallel=False)
File "/home/oszi/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py", line 79, in get_shards
return self.manager._shard_tests(test_list, num_workers, fully_parallel)
File "/home/oszi/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 593, in _shard_tests
return self._shard_by_directory(test_files, num_workers)
File "/home/oszi/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 664, in _shard_by_directory
'locked_shard'),
File "/home/oszi/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 706, in _resize_shards
num_old_per_new = divide_and_round_up(len(old_shards), max_new_shards)
File "/home/oszi/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 695, in divide_and_round_up
return int(math.ceil(float(numerator) / divisor))
TypeError: unsupported operand type(s) for /: 'float' and 'Mock'
Could you check it and fix?
Created attachment 122412 [details]
add --max-locked-shard option
I fixed the unit test sorry about that, and I also write a new unit test for test it works properly.
Comment on attachment 122412 [details] add --max-locked-shard option Clearing flags on attachment: 122412 Committed r104967: <http://trac.webkit.org/changeset/104967> All reviewed patches have been landed. Closing bug. |