150570 2015-10-26 12:31:40 -0700 Add logging to warn about under-estimated FTL inline cache sizes. 2015-10-26 14:47:16 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 mark.lam mark.lam oldest_to_newest 1136595 0 mark.lam 2015-10-26 12:31:40 -0700 This makes it a little more convenient to check if something has changed and if we need to re-calibrate. 1136597 1 264062 mark.lam 2015-10-26 12:35:11 -0700 Created attachment 264062 the patch. 1136603 2 264062 ggaren 2015-10-26 12:55:50 -0700 Comment on attachment 264062 the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=264062&action=review > Source/JavaScriptCore/ftl/FTLCompile.cpp:177 > + if (verboseCompilationEnabled()) { > + static size_t maxSize = 0; > + if (maxSize < actualCodeSize) > + maxSize = actualCodeSize; > + dataLogF("ALERT: Under-estimated FTL Inline Cache Size for %s: estimated %zu, actual %zu, max %zu\n", codeDescription, sizeOfInlineCode, actualCodeSize, maxSize); > + } I'd like to be able to use this tool to figure out if we have undersized ICs by running benchmarks. But verbose compilation and dumping to stdout would fall over for benchmarks. Can we log and then crash, with a special option dedicated to this? 1136606 3 264062 mark.lam 2015-10-26 13:03:42 -0700 Comment on attachment 264062 the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=264062&action=review >> Source/JavaScriptCore/ftl/FTLCompile.cpp:177 >> + } > > I'd like to be able to use this tool to figure out if we have undersized ICs by running benchmarks. But verbose compilation and dumping to stdout would fall over for benchmarks. > > Can we log and then crash, with a special option dedicated to this? I'll add 2 options: 1. assertICSizes - will crash if IC size is inadequate. Useful for the automated regression test case (e.g. when running benchmarks). 2. dumpFailedICSizing - will dump to stdout and continue. Useful for finding the max size of new ICs being added. 1136636 4 264077 mark.lam 2015-10-26 14:34:59 -0700 Created attachment 264077 Patch for landing. Also added comments in the ChangeLog to describe how to use these options. 1136639 5 264077 mark.lam 2015-10-26 14:45:52 -0700 Comment on attachment 264077 Patch for landing. View in context: https://bugs.webkit.org/attachment.cgi?id=264077&action=review > Source/JavaScriptCore/ChangeLog:10 > + 1. JSC_dumpFailedICSizing - dumps an error message if the FTL encounters an > + IC size estimates that are less than the actual needed code size. "encounters an IC size estimates" ==> "encounters IC size estimates". Will fix before landing. > Source/JavaScriptCore/ChangeLog:13 > + This option is useful for when we add a new IC and want to compute estimate > + sizes for the IC. To do this: "compute estimate sizes for" ==> "compute an estimated size for". Will fix before landing. > Source/JavaScriptCore/ChangeLog:17 > + 2. Implement a test suite with scenarios that exercises all the code paths in "exercises" ==> "exercise". 1136641 6 mark.lam 2015-10-26 14:47:16 -0700 Landed in r191602: <http://trac.webkit.org/r191602>. 264062 2015-10-26 12:35:11 -0700 2015-10-26 14:34:59 -0700 the patch. bug-150570.patch text/plain 1624 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTkxNTkzKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBA CisyMDE1LTEwLTI2ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBB ZGQgbG9nZ2luZyB0byB3YXJuIGFib3V0IHVuZGVyLWVzdGltYXRlZCBGVEwgaW5saW5lIGNhY2hl IHNpemVzLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTUwNTcwCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg KiBmdGwvRlRMQ29tcGlsZS5jcHA6CisgICAgICAgIChKU0M6OkZUTDo6Z2VuZXJhdGVJbmxpbmVJ ZlBvc3NpYmxlT3V0T2ZMaW5lSWZOb3QpOgorCiAyMDE1LTEwLTI0ICBCcmlhbiBCdXJnICA8YmJ1 cmdAYXBwbGUuY29tPgogCiAgICAgICAgIFRlYWNoIGNyZWF0ZV9oYXNoX3RhYmxlIHRvIG9taXQg YnVpbHRpbnMgbWFjcm9zIHdoZW4gZ2VuZXJhdGluZyB0YWJsZXMgZm9yIG5hdGl2ZS1vbmx5IG9i amVjdHMKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9mdGwvRlRMQ29tcGlsZS5jcHAKPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2Z0bC9GVExDb21waWxlLmNwcAkocmV2 aXNpb24gMTkxNTkyKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2Z0bC9GVExDb21waWxlLmNw cAkod29ya2luZyBjb3B5KQpAQCAtMTY5LDYgKzE2OSwxMyBAQCBzdGF0aWMgdm9pZCBnZW5lcmF0 ZUlubGluZUlmUG9zc2libGVPdXRPCiAgICAgICAgIHJldHVybjsKICAgICB9CiAKKyAgICBpZiAo dmVyYm9zZUNvbXBpbGF0aW9uRW5hYmxlZCgpKSB7CisgICAgICAgIHN0YXRpYyBzaXplX3QgbWF4 U2l6ZSA9IDA7CisgICAgICAgIGlmIChtYXhTaXplIDwgYWN0dWFsQ29kZVNpemUpCisgICAgICAg ICAgICBtYXhTaXplID0gYWN0dWFsQ29kZVNpemU7CisgICAgICAgIGRhdGFMb2dGKCJBTEVSVDog VW5kZXItZXN0aW1hdGVkIEZUTCBJbmxpbmUgQ2FjaGUgU2l6ZSBmb3IgJXM6IGVzdGltYXRlZCAl enUsIGFjdHVhbCAlenUsIG1heCAlenVcbiIsIGNvZGVEZXNjcmlwdGlvbiwgc2l6ZU9mSW5saW5l Q29kZSwgYWN0dWFsQ29kZVNpemUsIG1heFNpemUpOworICAgIH0KKwogICAgIC8vIElmIHRoZXJl IGlzbid0IGVub3VnaCBzcGFjZSBpbiB0aGUgcHJvdmlkZWQgaW5saW5lIGNvZGUgYXJlYSwgYWxs b2NhdGUgb3V0IG9mIGxpbmUKICAgICAvLyBleGVjdXRhYmxlIG1lbW9yeSB0byBsaW5rIHRoZSBw cm92aWRlZCBjb2RlLiBQbGFjZSBhIGp1bXAgYXQgdGhlIGJlZ2lubmluZyBvZiB0aGUKICAgICAv LyBpbmxpbmUgYXJlYSBhbmQganVtcCB0byB0aGUgb3V0IG9mIGxpbmUgY29kZS4gU2ltaWxhcmx5 IHJldHVybiBieSBhcHBlbmRpbmcgYSBqdW1wCg== 264077 2015-10-26 14:34:59 -0700 2015-10-26 14:34:59 -0700 Patch for landing. bug-150570.patch text/plain 3878 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTkxNTk5KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDM4IEBA CisyMDE1LTEwLTI2ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBB ZGQgbG9nZ2luZyB0byB3YXJuIGFib3V0IHVuZGVyLWVzdGltYXRlZCBGVEwgaW5saW5lIGNhY2hl IHNpemVzLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTUwNTcwCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgR2VvZmZyZXkgR2FyZW4uCisKKyAgICAgICAg QWRkZWQgMiBvcHRpb25zOgorICAgICAgICAxLiBKU0NfZHVtcEZhaWxlZElDU2l6aW5nIC0gZHVt cHMgYW4gZXJyb3IgbWVzc2FnZSBpZiB0aGUgRlRMIGVuY291bnRlcnMgYW4KKyAgICAgICAgICAg SUMgc2l6ZSBlc3RpbWF0ZXMgdGhhdCBhcmUgbGVzcyB0aGFuIHRoZSBhY3R1YWwgbmVlZGVkIGNv ZGUgc2l6ZS4KKworICAgICAgICAgICBUaGlzIG9wdGlvbiBpcyB1c2VmdWwgZm9yIHdoZW4gd2Ug YWRkIGEgbmV3IElDIGFuZCB3YW50IHRvIGNvbXB1dGUgZXN0aW1hdGUKKyAgICAgICAgICAgc2l6 ZXMgZm9yIHRoZSBJQy4gIFRvIGRvIHRoaXM6CisgICAgICAgICAgIDEuIEJ1aWxkIGpzYyBmb3Ig dGhlIHRhcmdldCBwb3J0IHdpdGggYSB2ZXJ5IHNtYWxsIElDIHNpemUgKGVub3VnaCB0bworICAg ICAgICAgICAgICBzdG9yZSB0aGUganVtcCBpbnN0cnVjdGlvbiBuZWVkZWQgZm9yIHRoZSBvdXQg b2YgbGluZSBmYWxsYmFjaworICAgICAgICAgICAgICBpbXBsZW1lbnRhdGlvbikuCisgICAgICAg ICAgIDIuIEltcGxlbWVudCBhIHRlc3Qgc3VpdGUgd2l0aCBzY2VuYXJpb3MgdGhhdCBleGVyY2lz ZXMgYWxsIHRoZSBjb2RlIHBhdGhzIGluCisgICAgICAgICAgICAgIHRoZSBJQyBnZW5lcmF0b3Iu CisgICAgICAgICAgIDMuIFJ1biBqc2Mgd2l0aCBKU0NfZHVtcEZhaWxlZElDU2l6aW5nPXRydWUg b24gdGhlIHRlc3Qgc3VpdGUuCisgICAgICAgICAgIDQuIFRoZSBtYXggdmFsdWUgcmVwb3J0ZWQg YnkgdGhlIGR1bXBzIHdpbGwgYmUgdGhlIHdvcnN0IGNhc2Ugc2l6ZSBuZWVkZWQgdG8KKyAgICAg ICAgICAgICAgc3RvcmUgdGhlIElDLiAgV2Ugc2hvdWxkIHVzZSB0aGlzIHZhbHVlIGZvciBvdXIg ZXN0aW1hdGUuCisgICAgICAgICAgIDUuIFVwZGF0ZSB0aGUgSUMncyBlc3RpbWF0ZWQgc2l6ZSBh bmQgcmVidWlsZCBqc2MuCisgICAgICAgICAgIDYuIFJlLXJ1biAoMykgYW5kIGNvbmZpcm0gdGhh dCB0aGVyZSBhcmUgbm8gbW9yZSBlcnJvciBtZXNzYWdlcyBhYm91dCB0aGUKKyAgICAgICAgICAg ICAgSUMgc2l6aW5nLgorCisgICAgICAgIDIuIEpTQ19hc3NlcnRJQ1NpemluZyAtIHNhbWUgYXMg SlNDX2R1bXBGYWlsZWRJQ1NpemluZyBleGNlcHQgdGhhdCBpdCBhbHNvCisgICAgICAgICAgIGNy YXNoZXMgdGhlIFZNIGVhY2ggdGltZSBpdCBlbmNvdW50ZXJzIGFuIGluYWRlcXVhdGUgSUMgc2l6 ZSBlc3RpbWF0ZS4KKworICAgICAgICAgICBUaGlzIG9wdGlvbiBpcyB1c2VmdWwgZm9yIHJlZ3Jl c3Npb24gdGVzdGluZyB0byBlbnN1cmUgdGhhdCBvdXIgZXN0aW1hdGVzCisgICAgICAgICAgIGRv IG5vdCByZWdyZXNzLgorCisgICAgICAgICogZnRsL0ZUTENvbXBpbGUuY3BwOgorICAgICAgICAo SlNDOjpGVEw6OmdlbmVyYXRlSW5saW5lSWZQb3NzaWJsZU91dE9mTGluZUlmTm90KToKKyAgICAg ICAgKiBydW50aW1lL09wdGlvbnMuaDoKKwogMjAxNS0xMC0yNiAgU2FhbSBiYXJhdGkgIDxzYmFy YXRpQGFwcGxlLmNvbT4KIAogICAgICAgICByMTkwNzM1IENhdXNlZCB1cyB0byBtYXliZSB0cmFt cGxlIHRoZSBiYXNlJ3MgdGFnLUdQUiBvbiAzMi1iaXQgaW5saW5lIGNhY2hlIHdoZW4gdGhlIGNh Y2hlIGFsbG9jYXRlcyBhIHNjcmF0Y2ggcmVnaXN0ZXIgYW5kIHRoZW4ganVtcHMgdG8gdGhlIHNs b3cgcGF0aApJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2Z0bC9GVExDb21waWxlLmNwcAo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZnRsL0ZUTENvbXBpbGUuY3BwCShy ZXZpc2lvbiAxOTE1OTcpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZnRsL0ZUTENvbXBpbGUu Y3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xNjksNiArMTY5LDE1IEBAIHN0YXRpYyB2b2lkIGdlbmVy YXRlSW5saW5lSWZQb3NzaWJsZU91dE8KICAgICAgICAgcmV0dXJuOwogICAgIH0KIAorICAgIGlm IChPcHRpb25zOjphc3NlcnRJQ1NpemluZygpIHx8IE9wdGlvbnM6OmR1bXBGYWlsZWRJQ1Npemlu ZygpKSB7CisgICAgICAgIHN0YXRpYyBzaXplX3QgbWF4U2l6ZSA9IDA7CisgICAgICAgIGlmICht YXhTaXplIDwgYWN0dWFsQ29kZVNpemUpCisgICAgICAgICAgICBtYXhTaXplID0gYWN0dWFsQ29k ZVNpemU7CisgICAgICAgIGRhdGFMb2dGKCJBTEVSVDogVW5kZXItZXN0aW1hdGVkIEZUTCBJbmxp bmUgQ2FjaGUgU2l6ZSBmb3IgJXM6IGVzdGltYXRlZCAlenUsIGFjdHVhbCAlenUsIG1heCAlenVc biIsIGNvZGVEZXNjcmlwdGlvbiwgc2l6ZU9mSW5saW5lQ29kZSwgYWN0dWFsQ29kZVNpemUsIG1h eFNpemUpOworICAgICAgICBpZiAoT3B0aW9uczo6YXNzZXJ0SUNTaXppbmcoKSkKKyAgICAgICAg ICAgIENSQVNIKCk7CisgICAgfQorCiAgICAgLy8gSWYgdGhlcmUgaXNuJ3QgZW5vdWdoIHNwYWNl IGluIHRoZSBwcm92aWRlZCBpbmxpbmUgY29kZSBhcmVhLCBhbGxvY2F0ZSBvdXQgb2YgbGluZQog ICAgIC8vIGV4ZWN1dGFibGUgbWVtb3J5IHRvIGxpbmsgdGhlIHByb3ZpZGVkIGNvZGUuIFBsYWNl IGEganVtcCBhdCB0aGUgYmVnaW5uaW5nIG9mIHRoZQogICAgIC8vIGlubGluZSBhcmVhIGFuZCBq dW1wIHRvIHRoZSBvdXQgb2YgbGluZSBjb2RlLiBTaW1pbGFybHkgcmV0dXJuIGJ5IGFwcGVuZGlu ZyBhIGp1bXAKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL09wdGlvbnMuaAo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9PcHRpb25zLmgJKHJl dmlzaW9uIDE5MTU5NykKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL09wdGlvbnMu aAkod29ya2luZyBjb3B5KQpAQCAtMjgwLDYgKzI4MCw5IEBAIHR5cGVkZWYgY29uc3QgY2hhciog b3B0aW9uU3RyaW5nOwogICAgIFwKICAgICB2KHVuc2lnbmVkLCByZW9wdGltaXphdGlvblJldHJ5 Q291bnRlck1heCwgMCwgbnVsbHB0cikgIFwKICAgICBcCisgICAgdihib29sLCBhc3NlcnRJQ1Np emluZywgZmFsc2UsICJjcmFzaCBpZiBlc3RpbWF0ZWQgSUMgc2l6ZXMgYXJlIGluYWRlcXVhdGUi KSAgXAorICAgIHYoYm9vbCwgZHVtcEZhaWxlZElDU2l6aW5nLCBmYWxzZSwgImR1bXBzIGEgbG9n IGVudHJ5IGlmIGVzdGltYXRlZCBJQyBzaXplcyBhcmUgaW5hZGVxdWF0ZSIpICBcCisgICAgXAog ICAgIHYodW5zaWduZWQsIG1pbmltdW1PcHRpbWl6YXRpb25EZWxheSwgMSwgbnVsbHB0cikgXAog ICAgIHYodW5zaWduZWQsIG1heGltdW1PcHRpbWl6YXRpb25EZWxheSwgNSwgbnVsbHB0cikgXAog ICAgIHYoZG91YmxlLCBkZXNpcmVkUHJvZmlsZUxpdmVuZXNzUmF0ZSwgMC43NSwgbnVsbHB0cikg XAo=