WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(2.36 KB, patch)
2019-11-13 15:56 PST
,
Carlos Alberto Lopez Perez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Carlos Alberto Lopez Perez
Comment 1
2019-11-13 06:47:29 PST
Created
attachment 383453
[details]
Patch
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
Created
attachment 383506
[details]
Patch
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
<
rdar://problem/57172650
>
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.
Top of Page
Format For Printing
XML
Clone This Bug