204154 2019-11-13 06:39:01 -0800 test262-runner: use NUMBER_OF_PROCESSORS and stop multiplying the number of cores. 2019-11-13 17:01:48 -0800 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=204063 InRadar P2 Normal --- 1 clopez clopez bugs-noreply keith_miller leo msaboff ross.kirsling valerie webkit-bug-importer xan.lopez ysuzuki oldest_to_newest 1589841 0 clopez 2019-11-13 06:39:01 -0800 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 1589845 1 383453 clopez 2019-11-13 06:47:29 -0800 Created attachment 383453 Patch 1589911 2 383453 ross.kirsling 2019-11-13 09:58:39 -0800 Comment on attachment 383453 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 1589916 3 clopez 2019-11-13 10:13:49 -0800 (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. 1590012 4 ross.kirsling 2019-11-13 13:24:04 -0800 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. 1590073 5 clopez 2019-11-13 15:49:26 -0800 (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. 1590081 6 383506 clopez 2019-11-13 15:56:42 -0800 Created attachment 383506 Patch 1590091 7 383506 clopez 2019-11-13 16:17:47 -0800 Comment on attachment 383506 Patch Clearing flags on attachment: 383506 Committed r252437: <https://trac.webkit.org/changeset/252437> 1590092 8 clopez 2019-11-13 16:17:52 -0800 All reviewed patches have been landed. Closing bug. 1590093 9 webkit-bug-importer 2019-11-13 16:18:18 -0800 <rdar://problem/57172650> 1590116 10 clopez 2019-11-13 16:59:55 -0800 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 1590117 11 ross.kirsling 2019-11-13 17:01:48 -0800 Excellent! 383453 2019-11-13 06:47:29 -0800 2019-11-13 15:56:30 -0800 Patch bug-204154-20191113154728.patch text/plain 1812 clopez U3VidmVyc2lvbiBSZXZpc2lvbjogMjUyNDA2CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggMDE0YjMxNzM4OGIzNjM2M2EyNDI2OTYxOWYxZTczNTBl NDZjMzcyNC4uNjQwNmZlZWI4ZTdkNjI2ODQ5N2FkOWZiMTBmY2VhYTA1MTYwZjdkMyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI1 IEBACisyMDE5LTExLTEzICBDYXJsb3MgQWxiZXJ0byBMb3BleiBQZXJleiAgPGNsb3BlekBpZ2Fs aWEuY29tPgorCisgICAgICAgIHRlc3QyNjItcnVubmVyOiB1c2UgZW52aXJvbm1lbnQgdmFyaWFi bGUgTlVNQkVSX09GX1BST0NFU1NPUlMgaWYgaXMgc2V0CisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMDQxNTQKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXZSBoYXZlIGVuYWJsZWQgcnVubmluZyB0ZXN0MjYy IG9uIEdUSyBhbmQgV1BFIHRlc3QgYm90cyBvbiByMjUyMzYyCisgICAgICAgIEJ1dCBpdCBzZWVt cyB0aGlzIHRlc3QyNjIgcnVubmVyIGlzIG5vdCByZXNwZWN0aW5nIHRoZSBlbnZpcm9ubWVudAor ICAgICAgICB2YXJpYWJsZSBOVU1CRVJfT0ZfUFJPQ0VTU09SUy4gV2Ugc2V0IHRoYXQgb24gdGhl IGJvdCBlbnZpcm9ubWVudHMKKyAgICAgICAgdG8gdGVsbCBlYWNoIHdvcmtlciBob3cgbXVjaCBw YXJhbGxlbGlzbSBpdCBzaG91bGQgdXNlLgorCisgICAgICAgIEluc3RlYWQgdGhpcyBydW5uZXIg d2FzIHVzaW5nIGFsbCB0aGUgc3lzdGVtIGNvcmVzICh3aGljaCBvbiBzb21lCisgICAgICAgIGNh c2VzIGlzIDI1NikgYW5kIHRoYXQgd2FzIGNhdXNpbmcgaXNzdWVzIG9uIG90aGVyIGNvbnRhaW5l cnMvd29ya2xvYWRzCisgICAgICAgIHJ1bm5pbmcgb24gdGhlIHNhbWUgc3lzdGVtIGF0IHRoZSBz YW1lIHRpbWUuCisKKyAgICAgICAgVGhpcyBlbnZpcm9ubWVudCB2YXJpYWJsZSBpcyB1bmRlcnN0 b29kIGJ5IGFsbCB3ZWJraXQtcmVsYXRlZCB0ZXN0IHNjcmlwdHMuCisgICAgICAgIEl0IHNob3Vs ZCBiZSBhY2NlcHRlZCBhbHNvIGJ5IHRoaXMgdGVzdDI2MiBydW5uZXIgc2NyaXB0LgorCisgICAg ICAgICogU2NyaXB0cy90ZXN0MjYyL1J1bm5lci5wbToKKyAgICAgICAgKGdldFByb2Nlc3Nlcyk6 CisKIDIwMTktMTEtMTMgIEFha2FzaCBKYWluICA8YWFrYXNoX2phaW5AYXBwbGUuY29tPgogCiAg ICAgICAgIEZsYWt5IEFQSSBUZXN0IFRlc3RXZWJLaXRBUEkuU2VydmljZVdvcmtlcnMuVGhyb3R0 bGVDcmFzaApkaWZmIC0tZ2l0IGEvVG9vbHMvU2NyaXB0cy90ZXN0MjYyL1J1bm5lci5wbSBiL1Rv b2xzL1NjcmlwdHMvdGVzdDI2Mi9SdW5uZXIucG0KaW5kZXggNWNmNDczZjExMWNlYzU1YmUxNTY4 MDRlMzJmNWRhNzY3ZDU4MDc3OS4uMjM3NzJhNzgwYTU1NDUyYjRjOTQ0OTQ5ODkzZWJhOWNmYzY2 NDUyOSAxMDA3NTUKLS0tIGEvVG9vbHMvU2NyaXB0cy90ZXN0MjYyL1J1bm5lci5wbQorKysgYi9U b29scy9TY3JpcHRzL3Rlc3QyNjIvUnVubmVyLnBtCkBAIC01NTMsNiArNTUzLDkgQEAgc3ViIGxv YWRJbXBvcnRGaWxlIHsKIH0KIAogc3ViIGdldFByb2Nlc3NlcyB7CisKKyAgICByZXR1cm4gJEVO VntOVU1CRVJfT0ZfUFJPQ0VTU09SU30gaWYgKGRlZmluZWQoJEVOVntOVU1CRVJfT0ZfUFJPQ0VT U09SU30pKTsKKwogICAgIG15ICRjb3JlczsKICAgICBteSAkdW5hbWUgPSBxeCh3aGljaCB1bmFt ZSA+PiAvZGV2L251bGwgJiYgdW5hbWUpOwogICAgIGNob21wICR1bmFtZTsK 383506 2019-11-13 15:56:42 -0800 2019-11-13 16:17:47 -0800 Patch bug-204154-20191114005640.patch text/plain 2418 clopez U3VidmVyc2lvbiBSZXZpc2lvbjogMjUyNDA2CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggMDE0YjMxNzM4OGIzNjM2M2EyNDI2OTYxOWYxZTczNTBl NDZjMzcyNC4uY2Q3ZmY5MmIzZDE1MGUyOGUyOGQ3MTAxOWU0YjlhMWRhZTYyZTc2YyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDMx IEBACisyMDE5LTExLTEzICBDYXJsb3MgQWxiZXJ0byBMb3BleiBQZXJleiAgPGNsb3BlekBpZ2Fs aWEuY29tPgorCisgICAgICAgIHRlc3QyNjItcnVubmVyOiB1c2UgTlVNQkVSX09GX1BST0NFU1NP UlMgYW5kIHN0b3AgbXVsdGlwbHlpbmcgdGhlIG51bWJlciBvZiBjb3Jlcy4KKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIwNDE1NAorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFdlIGhhdmUgZW5hYmxlZCBydW5u aW5nIHRlc3QyNjIgb24gR1RLIGFuZCBXUEUgdGVzdCBib3RzIG9uIHIyNTIzNjIKKyAgICAgICAg QnV0IGl0IHNlZW1zIHRoaXMgdGVzdDI2MiBydW5uZXIgaXMgbm90IHJlc3BlY3RpbmcgdGhlIGVu dmlyb25tZW50CisgICAgICAgIHZhcmlhYmxlIE5VTUJFUl9PRl9QUk9DRVNTT1JTLiBXZSBzZXQg dGhhdCBvbiB0aGUgYm90IGVudmlyb25tZW50cworICAgICAgICB0byB0ZWxsIGVhY2ggd29ya2Vy IGhvdyBtdWNoIHBhcmFsbGVsaXNtIGl0IHNob3VsZCB1c2UuCisKKyAgICAgICAgSW5zdGVhZCB0 aGlzIHJ1bm5lciB3YXMgdXNpbmcgYWxsIHRoZSBzeXN0ZW0gY29yZXMgKHdoaWNoIG9uIHNvbWUK KyAgICAgICAgY2FzZXMgaXMgMjU2KSBhbmQgdGhhdCB3YXMgY2F1c2luZyBpc3N1ZXMgb24gb3Ro ZXIgY29udGFpbmVycy93b3JrbG9hZHMKKyAgICAgICAgcnVubmluZyBvbiB0aGUgc2FtZSBzeXN0 ZW0gYXQgdGhlIHNhbWUgdGltZS4KKworICAgICAgICBUaGlzIGVudmlyb25tZW50IHZhcmlhYmxl IGlzIHVuZGVyc3Rvb2QgYnkgYWxsIHdlYmtpdC1yZWxhdGVkIHRlc3Qgc2NyaXB0cy4KKyAgICAg ICAgSXQgc2hvdWxkIGJlIGFjY2VwdGVkIGFsc28gYnkgdGhpcyB0ZXN0MjYyIHJ1bm5lciBzY3Jp cHQuCisKKyAgICAgICAgT24gdG9wIG9mIHRoYXQsIHRoaXMgcGF0Y2ggcmVtb3ZlcyB0aGUgbXVs dGlwbGljYXRpb24gdGhhdCBpcyBkb25lCisgICAgICAgIHdoZW4gdGhlIG51bWJlciBvZiBwYXJh bGxlbCBqb2JzIGlzIG5vdCBzcGVjaWZpZWQgYW5kIGl0cyBjYWxjdWxhdGVkCisgICAgICAgIGZy b20gdGhlIGF2YWlsYWJsZSBjb3JlcyBvZiB0aGUgc3lzdGVtLiBPbiBvbmUgaGFuZCB0aGVyZSBp cyBubworICAgICAgICB3cml0dGVuIG1vdGl2YXRpb24gZm9yIHRoaXMgbXVsdGlwbGljYXRpb24g YmVoYXZpb3VyLCBhbmQgb24gdGhlIG90aGVyCisgICAgICAgIGhhbmQgdGhlcmUgaXMgZXZpZGVu Y2UgdGhhdCBpdCBkb2Vzbid0IGltcHJvdmUgcGVyZm9ybWFuY2UuCisKKyAgICAgICAgKiBTY3Jp cHRzL3Rlc3QyNjIvUnVubmVyLnBtOgorICAgICAgICAoZ2V0UHJvY2Vzc2VzKToKKwogMjAxOS0x MS0xMyAgQWFrYXNoIEphaW4gIDxhYWthc2hfamFpbkBhcHBsZS5jb20+CiAKICAgICAgICAgRmxh a3kgQVBJIFRlc3QgVGVzdFdlYktpdEFQSS5TZXJ2aWNlV29ya2Vycy5UaHJvdHRsZUNyYXNoCmRp ZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3Rlc3QyNjIvUnVubmVyLnBtIGIvVG9vbHMvU2NyaXB0 cy90ZXN0MjYyL1J1bm5lci5wbQppbmRleCA1Y2Y0NzNmMTExY2VjNTViZTE1NjgwNGUzMmY1ZGE3 NjdkNTgwNzc5Li44MWFkYjE1YWJkNGQ5ZDI5Zjg0MjBmNjEwNDAzZjE5M2Y1YjJlMmFmIDEwMDc1 NQotLS0gYS9Ub29scy9TY3JpcHRzL3Rlc3QyNjIvUnVubmVyLnBtCisrKyBiL1Rvb2xzL1Njcmlw dHMvdGVzdDI2Mi9SdW5uZXIucG0KQEAgLTU1Myw2ICs1NTMsOCBAQCBzdWIgbG9hZEltcG9ydEZp bGUgewogfQogCiBzdWIgZ2V0UHJvY2Vzc2VzIHsKKyAgICByZXR1cm4gJEVOVntOVU1CRVJfT0Zf UFJPQ0VTU09SU30gaWYgKGRlZmluZWQoJEVOVntOVU1CRVJfT0ZfUFJPQ0VTU09SU30pKTsKKwog ICAgIG15ICRjb3JlczsKICAgICBteSAkdW5hbWUgPSBxeCh3aGljaCB1bmFtZSA+PiAvZGV2L251 bGwgJiYgdW5hbWUpOwogICAgIGNob21wICR1bmFtZTsKQEAgLTU3MywxMiArNTc1LDcgQEAgc3Vi IGdldFByb2Nlc3NlcyB7CiAgICAgICAgICRjb3JlcyA9IDE7CiAgICAgfQogCi0gICAgaWYgKCRj b3JlcyA8PSA4KSB7Ci0gICAgICAgIHJldHVybiAkY29yZXMgKiA0OwotICAgIH0KLSAgICBlbHNl IHsKLSAgICAgICAgcmV0dXJuICRjb3JlcyAqIDI7Ci0gICAgfQorICAgIHJldHVybiAkY29yZXM7 CiB9CiAKIHN1YiBwYXJzZUVycm9yIHsK