185918 2018-05-23 12:08:35 -0700 test262/Runner.pm: randomize tests for performance 2020-06-12 19:07:00 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified NEW InRadar P2 Normal --- 1 valerie webkit-unassigned ap commit-queue darin ews-watchlist leo msaboff valerie webkit-bug-importer webkit-unassigned ysuzuki oldest_to_newest 1426668 0 valerie 2018-05-23 12:08:35 -0700 If there are more than 5*max_process number of test, then shuffle the tests before allocating to child process to split up groups of small tests. 1426682 1 341115 valerie 2018-05-23 12:24:02 -0700 Created attachment 341115 Patch 1426739 2 341115 ap 2018-05-23 14:45:28 -0700 Comment on attachment 341115 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341115&action=review > Tools/Scripts/test262/Runner.pm:296 > + @files = shuffle @files; Anything random in tests makes me nervous because it can make reproducing results hard. Can the same result be achieved without randomness? 1426750 3 valerie 2018-05-23 15:17:55 -0700 (In reply to Alexey Proskuryakov from comment #2) > Comment on attachment 341115 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=341115&action=review > > > Tools/Scripts/test262/Runner.pm:296 > > + @files = shuffle @files; > > Anything random in tests makes me nervous because it can make reproducing > results hard. Can the same result be achieved without randomness? Hmm -- so this randomize the order in which the tests are run to even the load across the child processes (there are some groups of slow tests). The tests all stand alone. I thought of alphabetizing the tests (instead of shuffle), as the "slow" groupings are groupings within directories, but unfortunately the naming convention is also consistent in such a way that slow tests would be grouped. This is not a very great performance increase -- and I'm not married to seeing it committed, it's just here to get feedback from Michael :) 1427998 4 darin 2018-05-28 20:36:31 -0700 I think that using randomness is OK to achieve this. But it just needs to be one-time randomness with the results stored in the repository, rather than "every time the tests are run" randomness. Choosing a different set every time we run tests can make problems with one test affecting another into hard-to-reproduce flakiness. And we’d like to avoid that even if it’s unlikely. 1428147 5 leo 2018-05-29 11:36:23 -0700 Darin, I'm taking over this patch work as Valerie is in a vacation time this week. I'm not show how we randomize and keep it for consecutive runs. The results are sorted anyway. > tests can make problems with one test affecting another into hard-to-reproduce flakiness Every test run is isolated from each other. The only thing that could be a problem in this case is memory usage from each call to JSC (for each test). This not prevented from the current alternative - tests order as it is by their file path - anyway. > And we’d like to avoid that even if it’s unlikely. The only guaranteed way to prevent any conflict here is to run the tests in a single process queue. That's not what being solved here, neither this patch will compromise it. Running in a single thread makes the run way slower but gives you a save ride to prevent shared memory usage from multiple JSC calls. You should also consider Test262 has only unit tests - as a maintainer I can guarantee that - and we don't run anything such as stress tests or anything relying on performance. 1662296 6 341115 ysuzuki 2020-06-12 19:07:00 -0700 Comment on attachment 341115 Patch Putting r- based on Darin's comment about one-time randomness. 341115 2018-05-23 12:24:02 -0700 2020-06-12 19:07:00 -0700 Patch bug-185918-20180523152400.patch text/plain 1298 valerie U3VidmVyc2lvbiBSZXZpc2lvbjogMjMyMTE5CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggYjkxM2EyMmIyNjZmYjE3ZWJmNmM5N2QyNmEzY2Y5NmRj MTcxMTdkMi4uNzZiNGFjNGIzYWFjMGRjMWYzMzcyNmM5NjUzYWJkMzAwNzMyNzExYyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEz IEBACisyMDE4LTA1LTIzICBWYWxlcmllIFIgWW91bmcgIDx2YWxlcmllQGJvY291cC5jb20+CisK KyAgICAgICAgdGVzdDI2Mi9SdW5uZXIucG06IHJhbmRvbWl6ZSB0ZXN0cyBmb3IgcGVyZm9ybWFu Y2UKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE4NTkx OAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogU2Ny aXB0cy90ZXN0MjYyL1J1bm5lci5wbToKKyAgICAgICAgKG1haW4pOgorCiAyMDE4LTA1LTIzICBN aWNoYWVsIENhdGFuemFybyAgPG1jYXRhbnphcm9AaWdhbGlhLmNvbT4KIAogICAgICAgICBbR1RL XSBTaWxlbmNlIEdDQyA4IHdhcm5pbmdzCmRpZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3Rlc3Qy NjIvUnVubmVyLnBtIGIvVG9vbHMvU2NyaXB0cy90ZXN0MjYyL1J1bm5lci5wbQppbmRleCBlYzMw ZDAxZmQ0Y2Q1ODM2NWQyYmEwY2FjMmUzMTU2ZDI5NTliNjE5Li4wNDcyOTkwMzA2MTFlYWVlNTIx MDM5ZjVmYjA4NmMwYTU1M2ViZWQ3IDEwMDc1NQotLS0gYS9Ub29scy9TY3JpcHRzL3Rlc3QyNjIv UnVubmVyLnBtCisrKyBiL1Rvb2xzL1NjcmlwdHMvdGVzdDI2Mi9SdW5uZXIucG0KQEAgLTQyLDYg KzQyLDcgQEAgdXNlIEZpbmRCaW47CiB1c2UgRW52IHF3KERZTERfRlJBTUVXT1JLX1BBVEgpOwog dXNlIENvbmZpZzsKIHVzZSBUaW1lOjpIaVJlcyBxdyh0aW1lKTsKK3VzZSBMaXN0OjpVdGlsIHF3 KHNodWZmbGUpOwogCiBteSAkQmluOwogQkVHSU4gewpAQCAtMjkyLDYgKzI5Myw4IEBAIHN1YiBt YWluIHsKICAgICAjIElmIHdlIGFyZSBwcm9jZXNzaW5nIG1hbnkgZmlsZXMsIGZvcmsgcHJvY2Vz cwogICAgIGlmIChzY2FsYXIgQGZpbGVzID4gJG1heF9wcm9jZXNzICogNSkgewogCisgICAgICAg IEBmaWxlcyA9IHNodWZmbGUgQGZpbGVzOworCiAgICAgICAgICMgTWFrZSB0ZW1wb3JhcnkgZmls ZXMgdG8gcmVjb3JkIHJlc3VsdHMKICAgICAgICAgbXkgQHJlc3VsdHNmaHM7CiAgICAgICAgIGZv ciAobXkgJGkgPSAwOyAkaSA8PSAkbWF4X3Byb2Nlc3MtMTsgJGkrKykgewo=