169510 2017-03-11 00:46:59 -0800 REGRESSION(r213645): It made JSC tests super slow and timeout on AArch64 Linux 2017-04-11 01:27:36 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build All Linux RESOLVED FIXED P1 Blocker --- 108645 169300 1 ossy zan benjamin buildbot cdumez clopez cmarcelo dbates fpizlo mark.lam ossy saam zan oldest_to_newest 1286105 0 ossy 2017-03-11 00:46:59 -0800 before r213645: - https://build.webkit.org/builders/JSCOnly%20Linux%20AArch64%20Release/builds/4895 - runtime: 1h 15mins - only 7 failures after r213645: - https://build.webkit.org/builders/JSCOnly%20Linux%20AArch64%20Release/builds/4950 - runtime: 10h 30 mins (stopped after it, full test runtime would have taken days) - zillion timeouts Note, that https://trac.webkit.org/changeset/213645 broke the build too which was fixed in https://trac.webkit.org/changeset/213705. But I tested r213645 with the buildfix directly and it showed that r213645 caused this serious regression. Is there any way to disable this super slow new feature on Linux? I don't and won't have to debug what caused this regeression in your patch. 1286136 1 ossy 2017-03-11 05:58:11 -0800 I disabled it on Linux in https://trac.webkit.org/changeset/213758. Please enable it again once somebody fixed this regression. 1286137 2 fpizlo 2017-03-11 07:44:50 -0800 We should figure out what's going on here. This is probably not a Linux versus Darwin thing but a CPU model thing. Here's a possible hint: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20150921/300953.html It looks like in LLVM, they have to emit clrex when exiting ldx/stx (LL/SC) loops because "some uarchs" need it for "performance". I tried emitting clrex, and on my hardware, it made no difference. I think in our jargon, "clrex" should be called "WTF::clearLink()", since we use the old computer science terminology for this (load link, store conditional) rather than the ARM terminology (reservations). The way this would look like is that you'd want Atomic<>::transaction() to call clearLink() whenever it returns after having done a loadLink() but before the storeCond() - basically anytime that we would otherwise be leaving the link (the reservation) in place. If this works then just as transaction() abstracts LL/SC and CAS behind prepare/attempt, you probably want a giveUp() method that is a wrapper for clearLink() on LL_SC platforms and a no-op on CAS platforms. You probably want clearLink() to only be declared when HAVE(LL_SC) just as loadLink()/storeCond() are only declared when HAVE(LL_SC). I think it's OK to do clearLink() on all ARM64s, if it helps your perf. If this works, then can you also put a FIXME in B3LowerToAir's code that emits loadlink/storecond, and MacroAssembler's loadLink/storeCond code, and link it to a bug? Once I get around to implementing SharedArrayBuffer atomics, I'll definitely need to use MacroAssembler's and B3's LL/SC. It won't be possible to easily get around this. 1287145 3 ossy 2017-03-13 05:21:56 -0700 (In reply to comment #2) > We should figure out what's going on here. This is probably not a Linux > versus Darwin thing but a CPU model thing. ... Of course, yes and thanks for the detailed info. But as I said I don't and won't have to do it myself in the (near) future. If somebody else is interested in debugging this issue, feel free to pick it up. 1293729 4 306092 zan 2017-04-03 12:03:42 -0700 Created attachment 306092 Patch 1293730 5 buildbot 2017-04-03 12:05:00 -0700 Attachment 306092 did not pass style-queue: ERROR: Source/WTF/wtf/Atomics.h:193: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/Atomics.h:213: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style. 1296253 6 306092 saam 2017-04-10 19:23:35 -0700 Comment on attachment 306092 Patch r=me 1296315 7 306092 zan 2017-04-11 01:27:32 -0700 Comment on attachment 306092 Patch Clearing flags on attachment: 306092 Committed r215220: <http://trac.webkit.org/changeset/215220> 1296316 8 zan 2017-04-11 01:27:36 -0700 All reviewed patches have been landed. Closing bug. 306092 2017-04-03 12:03:42 -0700 2017-04-11 01:27:32 -0700 Patch bug-169510-20170403210339.patch text/plain 2370 zan U3VidmVyc2lvbiBSZXZpc2lvbjogMjE0Nzg3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IDY0OWRkZGNmZDcyOGM3YmI1ZTYxY2Fh NzMyZmY3NGUzZmU1N2E0MDEuLjA4YzFhMjU2NDY2N2Y3N2QwYzgwOGVhOGVmZDAwNDBjNmY2Njc5 ZDkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMjEgQEAKKzIwMTctMDQtMDMgIFphbiBEb2JlcnNlayAgPHpkb2Jl cnNla0BpZ2FsaWEuY29tPgorCisgICAgICAgIFJFR1JFU1NJT04ocjIxMzY0NSk6IEl0IG1hZGUg SlNDIHRlc3RzIHN1cGVyIHNsb3cgYW5kIHRpbWVvdXQgb24gQUFyY2g2NCBMaW51eAorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTY5NTEwCisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQWRkIHRoZSBgdm9sYXRp bGVgIHF1YWxpZmllcnMgZm9yIEFTTSBzdGF0ZW1lbnRzIHRoYXQgc2V0IHVwIGxkeHIgYW5kIHN0 eHIKKyAgICAgICAgaW5zdHJ1Y3Rpb24gY2FsbHMgd2hlbiBIQVZFKExMX1NDKSBpcyBlbmFibGVk IGZvciBhYXJjaDY0IHBsYXRmb3Jtcy4KKworICAgICAgICBUaGlzIGF2b2lkcyBpbmRlZmluaXRl IGV4ZWN1dGlvbiB3aGVuIHJ1bm5pbmcgR0NDLWNvbXBpbGVkIEpTQyBvbiBMaW51eC4KKyAgICAg ICAgU3BlY2lmaWMgYnVnIG9yIGV4cGVjdGVkIGJlaGF2aW9yIHRoYXQgZGlmZmVycyBmcm9tIENs YW5nIHdhc24ndCBkZXRlcm1pbmVkLgorCisgICAgICAgIEhBVkUoTExfU0MpIGlzIHJlLWVuYWJs ZWQgb24gbm9uLURhcndpbiBhYXJjaDY0IHBsYXRmb3Jtcy4KKworICAgICAgICAqIHd0Zi9BdG9t aWNzLmg6CisgICAgICAgICogd3RmL1BsYXRmb3JtLmg6CisKIDIwMTctMDQtMDEgIENzYWJhIE9z enRyb2dvbsOhYyAgPG9zc3lAd2Via2l0Lm9yZz4KIAogICAgICAgICBNYWMgY21ha2UgYnVpbGRm aXggYWZ0ZXIgMjE0NTg2LgpkaWZmIC0tZ2l0IGEvU291cmNlL1dURi93dGYvQXRvbWljcy5oIGIv U291cmNlL1dURi93dGYvQXRvbWljcy5oCmluZGV4IDYxZmRmZDZkZWQzNzhiZWRkOGEyNWQyYmY5 NDM5OWM2MjY0M2Q2NzkuLjc3MmU2ZmViYmQyZTcxNjlkNDNkNTA3OTVjMWVhZmM1OGVhNjhkMTgg MTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvd3RmL0F0b21pY3MuaAorKysgYi9Tb3VyY2UvV1RGL3d0 Zi9BdG9taWNzLmgKQEAgLTE5MCw3ICsxOTAsNyBAQCBzdHJ1Y3QgQXRvbWljIHsKICAgICAgICAg ICAgICAgICA6ICJyIih0aGlzKSBcCiAgICAgICAgICAgICAgICAgOiAibWVtb3J5Iik7IFwKICAg ICAgICAgfSBlbHNlIHsgXAotICAgICAgICAgICAgYXNtICggXAorICAgICAgICAgICAgYXNtIHZv bGF0aWxlICggXAogICAgICAgICAgICAgICAgICJsZHhyIiBzdWZmaXggIiAlIiBtb2RpZmllciAi MCwgWyUxXSIgXAogICAgICAgICAgICAgICAgIDogIj1yIihyZXN1bHQpIFwKICAgICAgICAgICAg ICAgICA6ICJyIih0aGlzKSBcCkBAIC0yMTAsNyArMjEwLDcgQEAgc3RydWN0IEF0b21pYyB7CiAg ICAgICAgICAgICAgICAgOiAiciIodmFsdWUpLCAiciIodGhpcykgXAogICAgICAgICAgICAgICAg IDogIm1lbW9yeSIpOyBcCiAgICAgICAgIH0gZWxzZSB7IFwKLSAgICAgICAgICAgIGFzbSAoIFwK KyAgICAgICAgICAgIGFzbSB2b2xhdGlsZSAoIFwKICAgICAgICAgICAgICAgICAic3R4ciIgc3Vm Zml4ICIgJXcwLCAlIiBtb2RpZmllciAiMSwgWyUyXSIgXAogICAgICAgICAgICAgICAgIDogIj0m ciIocmVzdWx0KSBcCiAgICAgICAgICAgICAgICAgOiAiciIodmFsdWUpLCAiciIodGhpcykgXApk aWZmIC0tZ2l0IGEvU291cmNlL1dURi93dGYvUGxhdGZvcm0uaCBiL1NvdXJjZS9XVEYvd3RmL1Bs YXRmb3JtLmgKaW5kZXggZGYwZDdmZmYxNTgxODMyNjMyNjA1NDIzNjExMzhkODQ5NWYwMjAzOS4u MjZjZDQ5N2FhMDYxNGZlNWVmYTY0ZGY5MDNkYWQ5ZDM4OGFhZTQyMSAxMDA2NDQKLS0tIGEvU291 cmNlL1dURi93dGYvUGxhdGZvcm0uaAorKysgYi9Tb3VyY2UvV1RGL3d0Zi9QbGF0Zm9ybS5oCkBA IC03NTcsOCArNzU3LDcgQEAKICNkZWZpbmUgRU5BQkxFX0NPTkNVUlJFTlRfSlMgMQogI2VuZGlm CiAKLS8qIEZJWE1FOiBFbmFibGUgaXQgb24gTGludXggb25jZSBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTY5NTEwIGlzIGZpeGVkLiAqLwotI2lmIENQVShBUk02NCkg JiYgT1MoREFSV0lOKQorI2lmIENQVShBUk02NCkKICNkZWZpbmUgSEFWRV9MTF9TQyAxCiAjZW5k aWYgLy8gQ1BVKEFSTTY0KSAmJiBPUyhEQVJXSU4pCiAK