WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED INVALID
141161
Remove slow test unused code from run-jsc-stress-tests
https://bugs.webkit.org/show_bug.cgi?id=141161
Summary
Remove slow test unused code from run-jsc-stress-tests
Csaba Osztrogonác
Reported
2015-02-02 08:23:27 PST
Handling slow tests mechanism was added by
http://trac.webkit.org/changeset/157887
16 months before to prepend the only one slow test to the run list instead of append. LayoutTests/js/regress/script-tests/stepanov_container.js was removed from the trunk 13 months before -
http://trac.webkit.org/changeset/162181
This hacky slow test mechanism wasn't used at all after it. I think we don't need it at all, let's clean up the code.
Attachments
Patch
(3.42 KB, patch)
2015-02-02 08:26 PST
,
Csaba Osztrogonác
no flags
Details
Formatted Diff
Diff
Patch
(2.58 KB, patch)
2015-02-02 08:27 PST
,
Csaba Osztrogonác
fpizlo
: review-
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 1
2015-02-02 08:26:05 PST
Created
attachment 245878
[details]
Patch
Csaba Osztrogonác
Comment 2
2015-02-02 08:26:33 PST
Comment on
attachment 245878
[details]
Patch wrong patch
Csaba Osztrogonác
Comment 3
2015-02-02 08:27:30 PST
Created
attachment 245879
[details]
Patch
Csaba Osztrogonác
Comment 4
2015-02-07 23:05:34 PST
ping?
Filip Pizlo
Comment 5
2015-02-08 06:51:24 PST
Comment on
attachment 245879
[details]
Patch Please don't remove this. :-). We use the --extra-tests flag to pass tests that cannot be made public for whatever reasons, and most of those use the slow mechanism. Plus I'm surprised that you think this is a hack. Of course you want slow-running tests to get pretended rather than appended, to aid with load balancing.
Csaba Osztrogonác
Comment 6
2015-02-09 02:41:22 PST
(In reply to
comment #5
)
> Comment on
attachment 245879
[details]
> Patch > > Please don't remove this. :-). We use the --extra-tests flag to pass tests > that cannot be made public for whatever reasons, and most of those use the > slow mechanism.
OK, now I know it is used, I don't want to remove a used feature. But as I mentioned in the description,
r157887
added it for only one test which was removed by
r162181
and haven't found any reference if it is used. If a feature has only non-public use cases, it would be great to have some comment why is it needed in the code or in the bug report which added it.
> Plus I'm surprised that you think this is a hack. Of course you want > slow-running tests to get pretended rather than appended, to aid with load > balancing.
I still think and consider this code a hack. I understad the goal, but I'm surprised if it worked ever. Let's see the GNU make based runner: all: test_done_0 test_done_1 test_done_2 test_done_3 ... ... The order of the prerequisite doesn't guaranatee anything. If you want to make sure if test_done_0 runs before test_done_1, you have to add normal or order only prerequisite. Let's see the shell based runner. It collects the tests with external find call without any ordering and runner threads always pick up a test from the end of the list to run. I checked this list file, it is absolutely random on Linux and it is more or less random on OSX too: ./test_script_0 ./test_script_1 ./test_script_10 ./test_script_100 ./test_script_1000 ./test_script_10000 ./test_script_10001 ... ./test_script_10009 ./test_script_1001 ... ./test_script_16998 ./test_script_16999 ./test_script_17 ./test_script_170 ./test_script_1700 ./test_script_17000 ... ./test_script_17025 ./test_script_17026 ./test_script_1703 ./test_script_1704 ... ./test_script_1709 ./test_script_171 ... It seems as if it was sorted with sort, but not with numberic sort at all.
Csaba Osztrogonác
Comment 7
2015-02-09 02:42:34 PST
Otherwise close as resolved/invalid, because it is still used.
Csaba Osztrogonác
Comment 8
2015-02-09 03:09:28 PST
I fixed the shell test runner for you in
bug141383
. The make runner runs the tests in good order now on Linux and OSX too, but it isn't guaranteed at all, it is only a luck.
Filip Pizlo
Comment 9
2015-02-09 06:09:22 PST
(In reply to
comment #8
)
> I fixed the shell test runner for you in
bug141383
. > > The make runner runs the tests in good order now > on Linux and OSX too, but it isn't guaranteed at > all, it is only a luck.
Nope, not luck. GNU make has a predictable search that drives execution order. It's done it the same way for at least 15 years. I haven't seen it documented but they probably can't change it because it would break makefiles that accidentally depend on it. We don't care about the parallel test runner achieving great scalability on other make implementations. Also, all we need here is luck. Any test can be run in any order, but we scale better if the slowest tests are fired off first. So if the make we ran on decided to break with tradition and use a different ordering then we would just lose the efficacy of the slow test optimization but this wouldn't affect the validity of the test run.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug