Created attachment 410455 [details] Safari 13.1.1 Hi. I am the author of the js-dos project (dosbox in js). After updating to Safari 14 I see a dramatic performance loss in js-dos dhry2 test. It reproduced on all devices: iphone 6, iphone 7, iphone 8, iphone x, macos catalina. All Safari starting from 14 affected (I tested 14.0, 14.0.1 and Technology Preview). The test is a variation of dhry 2 test for dos. It runs inside js-dosbox and reports operation count. Time is measured with Date.now(). So it only does math operations and estimate time. You can run this test using link. https://dos.zone/en/play/https%3A%2F%2Fdoszone-uploads.s3.dualstack.eu-central-1.amazonaws.com%2Foriginal%2F2X%2Fb%2Fb4b5275904d86a4ab8a20917b2b7e34f0df47bf7.jsdos My results are in attached screenshots. Results from (MacBook Pro (13-inch, 2017, Two Thunderbolt 3 ports, Catalina 10.15.17): Safari 13.1.1 - (640_000, VAX: 48) Safari 14.0.0 - (80_000, VAX: 5.5) Safari 14.0.1 - (80_000, VAX: 5.5) Safari TP - (80_000, VAX: 5.9) Firefox - (640_000, VAX: 36.9) -- I also tried to test in recent builds of webkit (267874, 267919, 267940). But, always have this error: MacBook-Pro-Alexander:~ caiiiycuk$ /Users/caiiiycuk/Downloads/267940/run-webkit-archive ; exit; Setting DYLD FRAMEWORK and LIBRARY paths to /Users/caiiiycuk/Downloads/267940/Release 2020-10-04 15:21:50.936 SafariForWebKitDevelopment[4346:301066] -[BrowserWKView _setInspectorDelegate:]: unrecognized selector sent to instance 0x7fef794c9c30 2020-10-04 15:21:50.936 SafariForWebKitDevelopment[4346:301066] -[BrowserWKView _setInspectorDelegate:]: unrecognized selector sent to instance 0x7fef794c9c30 2020-10-04 15:21:50.947 SafariForWebKitDevelopment[4346:301066] .sdef warning for argument 'FileType' of command 'save' in suite 'Standard Suite': 'saveable file format' is not a valid type name. 2020-10-04 15:21:50.995 SafariForWebKitDevelopment[4346:301134] +[NSXPCSharedListener endpointForReply:withListenerName:]: an error occurred while attempting to obtain endpoint for listener 'SandboxBroker': Connection invalid 2020-10-04 15:21:51.083 com.apple.WebKit.WebContent.Development[4348:301125] Error loading /Library/Apple/System/Library/StagedFrameworks/Safari/Safari.framework/Safari: dlopen(/Library/Apple/System/Library/StagedFrameworks/Safari/Safari.framework/Safari, 265): Symbol not found: _OBJC_CLASS_$_WBSContentBlockerStatisticsSQLiteStore Referenced from: /Library/Apple/System/Library/StagedFrameworks/Safari/SafariSharedUI.framework/Versions/A/SafariSharedUI Expected in: /System/Library/PrivateFrameworks/SafariShared.framework/Versions/A/SafariShared in /Library/Apple/System/Library/StagedFrameworks/Safari/SafariSharedUI.framework/Versions/A/SafariSharedUI InjectedBundle::load failed - Could not load the executable from the bundle. 2020-10-04 15:22:01.457 SafariForWebKitDevelopment[4346:301066] -[BrowserWKView _setInspectorDelegate:]: unrecognized selector sent to instance 0x7fef7d529cc0
Created attachment 410456 [details] Safari 14.0.0
Created attachment 410457 [details] Safari 14.0.1
Created attachment 410458 [details] Safari TP
Minimal test: https://js-dos.com/v7/build/examples/dhry2.html There are 2 buttons: 1) Start worker - run test in worker 2) Start direct - run test without worker Results are nearly same.
<rdar://problem/69934870>
Thanks for your report. Will try to reproduce
My guess is that this is related to: https://bugs.webkit.org/show_bug.cgi?id=212105
(In reply to Keith Miller from comment #7) > My guess is that this is related to: > https://bugs.webkit.org/show_bug.cgi?id=212105 I was able to bisect this to d8a0fb4bf380350af94c8cf7c399471e1186b93c..4fbc651e99458aeb44610e57d25d88fe7c1c498a and that bug looks like the most likely culprit.
Created attachment 412207 [details] Patch
Comment on attachment 412207 [details] Patch r=me
Committed r268942: <https://trac.webkit.org/changeset/268942> All reviewed patches have been landed. Closing bug and clearing flags on attachment 412207 [details].
Comment on attachment 412207 [details] Patch Can we get a microbenchmark for this?
(In reply to Saam Barati from comment #12) > Comment on attachment 412207 [details] > Patch > > Can we get a microbenchmark for this? I could try to create one but it might not be worth the time. Alekandr, would you be willing to let us copy your Wasm module into our repo as a test case?
(In reply to Keith Miller from comment #13) > (In reply to Saam Barati from comment #12) > > Comment on attachment 412207 [details] > > Patch > > > > Can we get a microbenchmark for this? > > I could try to create one but it might not be worth the time. Alekandr, > would you be willing to let us copy your Wasm module into our repo as a test > case? Sure! Do you need only a wasm module? If you want I can create an all-in-one wasm file with dhry2 test embedded, that you need to simply run.
(In reply to Alekandr Guryanov from comment #14) > (In reply to Keith Miller from comment #13) > > (In reply to Saam Barati from comment #12) > > > Comment on attachment 412207 [details] > > > Patch > > > > > > Can we get a microbenchmark for this? > > > > I could try to create one but it might not be worth the time. Alekandr, > > would you be willing to let us copy your Wasm module into our repo as a test > > case? > > Sure! Do you need only a wasm module? If you want I can create an all-in-one > wasm file with dhry2 test embedded, that you need to simply run. Sure, if it's not too much work, an all-in-one is probably a bit easier to work with.
> Sure, if it's not too much work, an all-in-one is probably a bit easier to > work with. Thanks!
Created attachment 412294 [details] all-in-one-test This is a wasm file with an embedded dhry2 test (function names are not stripped).
Even on Safari 14.4 performance is still very bad.
Created attachment 421600 [details] Performance on Safari 14.4
Sorry for taking so long to come back to this bug. The problem is that 25k for the option --maximumTmpsForGraphColoring is too small for some benchmarks (including the Dhry2 in js-dosbox example in the original comment), causing the performance regressions; but anything significantly above 25k caused jetsams on some other webpages (which was the original reason for this limit to exist). The jetsams were coming from two data-structures: the interference graph used by AirAllocateRegistersByGraphColoring.cpp and the interference graph used by AirAllocateStackByGraphColoring.cpp. I massively reduced their respective memory consumption in: - https://trac.webkit.org/changeset/277714/webkit - https://trac.webkit.org/changeset/278186/webkit One of the pages that used to jetsam with a larger value of --maximumTmpsForGraphColoring was mruby-wasm.aotoki.dev: - the first of the above patches reduced the memory consumption of the register allocator from 2 x 262MB to 2 x 20MB (2x because of trying to compile two functions in parallel) - the second patch reduced the memory consumption of the stack allocator on the same functions from 221MB + 399MB to 6.5MB + 10.5MB Since I saw similar reductions on every benchmark I tried, I believed it is now safe to try to increase --maximumTmpsForGraphColoring. I verified on the example you provided, there are exactly two functions with > 25k temporaries (54845 and 54856). Increasing --maximumTmpsForGraphColoring from 25k to 60k increased the "VAX" score on my machine (ToT minibrowser on an M1 MBP 2020) from about 27 to about 70.
Created attachment 443607 [details] Patch
Comment on attachment 443607 [details] Patch r=me
(In reply to Robin Morisset from comment #20) > I verified on the example you provided, there are exactly two functions with > > 25k temporaries (54845 and 54856). Increasing > --maximumTmpsForGraphColoring from 25k to 60k increased the "VAX" score on > my machine (ToT minibrowser on an M1 MBP 2020) from about 27 to about 70. So this also means if I split this two functions into smaller somehow (like split them) it will increase performance on same value, correct?
Comment on attachment 443607 [details] Patch Thanks Yusuke for the review.
(In reply to Alekandr Guryanov from comment #23) > (In reply to Robin Morisset from comment #20) > > > I verified on the example you provided, there are exactly two functions with > > > 25k temporaries (54845 and 54856). Increasing > > --maximumTmpsForGraphColoring from 25k to 60k increased the "VAX" score on > > my machine (ToT minibrowser on an M1 MBP 2020) from about 27 to about 70. > > So this also means if I split this two functions into smaller somehow (like > split them) it will increase performance on same value, correct? Correct. If you want to improve the performance on the currently shipping version of Safari, keeping these functions split should help a lot. I believe these giant functions typically occur because of very aggressive inlining by LLVM/Emscripten, so preventing some of it somehow might be the easiest way to achieve this.
Committed r285533 (244050@main): <https://commits.webkit.org/244050@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 443607 [details].
This change should be present in STP 139, iOS 15.4 Beta, and macOS 12.3 Beta.