RESOLVED FIXED 204154
test262-runner: use NUMBER_OF_PROCESSORS and stop multiplying the number of cores.
https://bugs.webkit.org/show_bug.cgi?id=204154
Summary test262-runner: use NUMBER_OF_PROCESSORS and stop multiplying the number of c...
Carlos Alberto Lopez Perez
Reported 2019-11-13 06:39:01 PST
We have enabled running test262 on GTK and WPE bots on r252362 But it seems this test is not respecting the environment variable NUMBER_OF_PROCESSORS that is set on the bot environment to tell each worker how much parallelism it should use. Instead it is getting the system number of cores (which on some cases is 256) and that causes issues on other containers running on the same system. This script should use the environment variable NUMBER_OF_PROCESSORS if is set, like all the other scripts for running webkit-related tests do
Attachments
Patch (1.77 KB, patch)
2019-11-13 06:47 PST, Carlos Alberto Lopez Perez
no flags
Patch (2.36 KB, patch)
2019-11-13 15:56 PST, Carlos Alberto Lopez Perez
no flags
Carlos Alberto Lopez Perez
Comment 1 2019-11-13 06:47:29 PST
Ross Kirsling
Comment 2 2019-11-13 09:58:39 PST
Comment on attachment 383453 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383453&action=review This basically seems good, but are we concerned about preserving the fact that the number of *processes* returned here is a multiple of the number of *processors*? (Honest question, since I was surprised to see that this is how it current works.) > Tools/Scripts/test262/Runner.pm:556 > + style nit: this initial newline could be removed
Carlos Alberto Lopez Perez
Comment 3 2019-11-13 10:13:49 PST
(In reply to Ross Kirsling from comment #2) > Comment on attachment 383453 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=383453&action=review > > This basically seems good, but are we concerned about preserving the fact > that the number of *processes* returned here is a multiple of the number of > *processors*? (Honest question, since I was surprised to see that this is > how it current works.) > I also got surprised by that, and I have no idea why is it. This multiplier was originally set to 8 when the script was added in r230680 and then modified depending on the number of cores in r231525 but none of the commits or bugs associated provide any info about why is that. In any case I think that the environment variable NUMBER_OF_PROCESSORS=n should have the same effect that passing the parameter --child-processes=n or -p=n And that is what this patch does.
Ross Kirsling
Comment 4 2019-11-13 13:24:04 PST
Hmm, I just tried doing a full release run with different child process counts on an 8-core iMac and saw the following: 8 processes: > Done in 485.89 seconds! > Tools/Scripts/test262-runner --release 1024.56s user 1517.22s system 522% cpu 8:06.06 total 16 processes: > Done in 476.87 seconds! > Tools/Scripts/test262-runner --release 1023.00s user 1525.18s system 534% cpu 7:57.10 total 32 processes: > Done in 468.89 seconds! > Tools/Scripts/test262-runner --release 1024.17s user 1536.59s system 545% cpu 7:49.11 total So the total time is hardly affected, but my ability to interact with other applications on the same system is hugely affected (i.e. it's very hard just to browse webpages while the 32-process run is active). Given this and a lack of written motivation for the existing behavior, perhaps we should remove the multiples even from the normal path here.
Carlos Alberto Lopez Perez
Comment 5 2019-11-13 15:49:26 PST
(In reply to Ross Kirsling from comment #4) > Hmm, I just tried doing a full release run with different child process > counts on an 8-core iMac and saw the following: > > 8 processes: > > Done in 485.89 seconds! > > Tools/Scripts/test262-runner --release 1024.56s user 1517.22s system 522% cpu 8:06.06 total > > 16 processes: > > Done in 476.87 seconds! > > Tools/Scripts/test262-runner --release 1023.00s user 1525.18s system 534% cpu 7:57.10 total > > 32 processes: > > Done in 468.89 seconds! > > Tools/Scripts/test262-runner --release 1024.17s user 1536.59s system 545% cpu 7:49.11 total > > So the total time is hardly affected, but my ability to interact with other > applications on the same system is hugely affected (i.e. it's very hard just > to browse webpages while the 32-process run is active). > > Given this and a lack of written motivation for the existing behavior, > perhaps we should remove the multiples even from the normal path here. It looks reasonable. I will upload a new patch removing that multiplication.
Carlos Alberto Lopez Perez
Comment 6 2019-11-13 15:56:42 PST
Carlos Alberto Lopez Perez
Comment 7 2019-11-13 16:17:47 PST
Comment on attachment 383506 [details] Patch Clearing flags on attachment: 383506 Committed r252437: <https://trac.webkit.org/changeset/252437>
Carlos Alberto Lopez Perez
Comment 8 2019-11-13 16:17:52 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 9 2019-11-13 16:18:18 PST
Carlos Alberto Lopez Perez
Comment 10 2019-11-13 16:59:55 PST
Another confirmation that the previous behaviour of multiplying cores didn't improved performance: Before r252437, the Mac Test262 bots where taking to complete the step the following: Release - 19:12 mins - Child Processes: 48 - https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20Test262%20%28Tests%29/builds/7127 Debug - 20:11 mins - Child Processes: 32 - https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20Test262%20%28Tests%29/builds/6218 And after r252437 they take even less time to complete Release - 18:28 mins - Child Processes: 24 - https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20Test262%20%28Tests%29/builds/7128 Debug - 19:40 mins - Child Processes: 16 - https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20Test262%20%28Tests%29/builds/6219
Ross Kirsling
Comment 11 2019-11-13 17:01:48 PST
Excellent!
Note You need to log in before you can comment on or make changes to this bug.