Bug 56760

Created attachment 86345 [details] Patch

Committed r81597: <http://trac.webkit.org/changeset/81597>

Pavel is rolling this out because on the Chromium bots the http/ tests are being run backwards. From http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Linux/builds/4600/steps/webkit_tests/logs/stdio: worker-0 http/tests/loading/preload-img-test.html passed worker-0 http/tests/loading/onload-vs-immediate-refresh.pl worker-0 http/tests/loading/location-hash-reload-cycle.html worker-0 http/tests/loading/load-javascript-after-many-xhrs.html passed

Fascinating. I wonder how that is possible. If that is the case, it probably would've been clearer to say "tests are being run backwards" in the other bug rather than "tests are unexpectedly flaky".

Reopening, because it was rolled out in http://trac.webkit.org/changeset/81675. So, this code has been broken for quite a while, at least since http://trac.webkit.org/changeset/68903 and http://trac.webkit.org/changeset/69074. There are four paths through the "sharding" code, depending on whether we're running through the single-threaded path (which triggers if you're running with --worker-model=inline or --child-processes=1, or, less commonly, if you're running with --experimental-fully-parallel), or through the multi-threaded path. If you run through the single-threaded path, then test are either put into the http_lock bucket (in order) or to their own bucket. Then the tests in the http_lock bucket are reversed. The net result is that http tests are in a single batch backwards but all the other tests are individual batches of one file each. The --worker-model={old-threads,old-inline} would then get the list of reversed tests and *pop* them one at a time and run them, hence effectively reversing the http tests again and having no effect on the other batches. The net result would be that everything would run in order. In the multithreaded case, we would use real batches for the non-http tests, but we would reverse each batch, so that again, popping them would work and they'd be run in order. The new worker model code, however, doesn't pop items from the batches, it iterates over them in order. The net result is that in the single-threaded case, we run the http tests in reverse and the other tests in order, and in the multi-threaded case, we run everything in reverse per directory (which is what the bots are currently doing). One solution could be to change the sharding logic to leave the reverses in for the old-* worker models, and take the reverses out for the new worker models. Another would be to take the reverses out, and change the old-* implementations to pop from the front of the batch rather than pop from the end. A third would be to change the new-* implementations to pop the tests rather than iterate over them. Since my long term plan is to delete the old implementations, I suggest that we take the reverses out and change the old-* implementation to pop from the front of the batch. Anyone disagree? Note that the real implication of all of this is that currently Chromium is running the tests backwards by default, and the test_expectations are set to deal with that. When we "fix" this to run them forwards, we will get a bunch of diffs and have to adjust for them. This will probably require some coordinating with the gardeners.

(In reply to comment #5) > Since my long term plan is to delete the old implementations, I suggest that we take the reverses out and change the old-* implementation to pop from the front of the batch. > > Anyone disagree? Sounds good to me. > Note that the real implication of all of this is that currently Chromium is running the tests backwards by default, and the test_expectations are set to deal with that. When we "fix" this to run them forwards, we will get a bunch of diffs and have to adjust for them. This will probably require some coordinating with the gardeners. Sigh. This is just a pain we'll have to deal with. I don't understand though. Looking at the bot stdio, they seem to be running them in alphabetical order per worker.

(In reply to comment #6) > Sigh. This is just a pain we'll have to deal with. I don't understand though. Looking at the bot stdio, they seem to be running them in alphabetical order per worker. Which bot? Mac 10.5 is running them in the correct order because it is single threaded (we hard-code to --worker-model=old-threads and --child-processes=1 because we don't have multiprocessing module on 10.5) Mac 10.6 is running them in reverse order, because it is running --worker-model=processes and multiple children. The Win bots are still running old-threads and hence running them in order, because the last time I tried to switch them over to processes we had a bunch of issues. I need to fix a bug or two and flip the switch again this week. The Linux Hardy bots are still running with --old-threads, presumably because Hardy also only has Python 2.5; the 64-bit bots are running with processes and hence, in reverse order. Fun, huh?

I see. I had looked at a Windows bot.

Created attachment 86697 [details] Patch

Comment on attachment 86697 [details] Patch Clearing flags on attachment: 86697 Committed r81815: <http://trac.webkit.org/changeset/81815>

All reviewed patches have been landed. Closing bug.