73175 2011-11-27 17:53:22 -0800 DFG should not emit GetMethod node 2011-11-27 18:15:46 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 73178 1 fpizlo webkit-unassigned oldest_to_newest 509515 0 fpizlo 2011-11-27 17:53:22 -0800 The GetMethod node used to serve as an optimization for GetById, if the GetById was being syntactically used as the base of a method call. But since then, we have replaced all of the profitable cases of GetMethod with a pair of CheckStructure nodes, and a pair of WeakJSConstant nodes. Hence the GetMethod node is currently only being emitted when we know, based on profiling, that the GetMethod optimization had actually failed. We should simply emit a normal GetById node, if we know that it is unprofitable to emit a pair of CheckStructure/WeakJSConstant nodes. 509517 1 116688 fpizlo 2011-11-27 17:56:16 -0800 Created attachment 116688 the patch If anything, this is neutral. Though it does seem to have hints of a win on V8. Benchmark report for SunSpider, V8, and Kraken on nitroflex.local (MacBookPro8,2). VMs tested: "TipOfTree" at /Volumes/Data/pizlo/quinary/OpenSource/WebKitBuild/Release/jsc (r101201) "NoGetMethod" at /Volumes/Data/pizlo/OpenSource/WebKitBuild/Release/jsc (r101201) Collected 12 samples per benchmark/VM, with 4 VM invocations per benchmark. Emitted a call to gc() between sample measurements. Used 1 benchmark iteration per VM invocation for warm-up. Used the jsc-specific preciseTime() function to get microsecond-level timing. Reporting benchmark execution times with 95% confidence intervals in milliseconds. TipOfTree NoGetMethod SunSpider: 3d-cube 7.1764+-0.1742 6.8968+-0.1244 might be 1.0405x faster 3d-morph 7.9859+-0.3041 7.8839+-0.1212 might be 1.0129x faster 3d-raytrace 7.2032+-0.2497 ? 7.3030+-0.1322 ? might be 1.0138x slower access-binary-trees 1.5779+-0.0552 1.5116+-0.0364 might be 1.0439x faster access-fannkuch 6.3811+-0.1948 6.1966+-0.0416 might be 1.0298x faster access-nbody 3.5178+-0.0694 ? 3.5251+-0.0510 ? access-nsieve 2.5911+-0.0593 ? 2.5923+-0.0493 ? bitops-3bit-bits-in-byte 1.2408+-0.0188 ? 1.2525+-0.0257 ? bitops-bits-in-byte 2.4423+-0.0506 2.4300+-0.0808 bitops-bitwise-and 3.4546+-0.0736 3.3714+-0.0364 might be 1.0247x faster bitops-nsieve-bits 5.3373+-0.0818 ? 5.3556+-0.0673 ? controlflow-recursive 2.0290+-0.0279 ? 2.0734+-0.0300 ? might be 1.0219x slower crypto-aes 7.3307+-0.2431 7.2280+-0.1415 might be 1.0142x faster crypto-md5 2.4306+-0.0582 ? 2.4351+-0.0529 ? crypto-sha1 2.1583+-0.1186 ? 2.1933+-0.0843 ? might be 1.0162x slower date-format-tofte 10.3251+-0.2437 10.1724+-0.2506 might be 1.0150x faster date-format-xparb 9.5827+-0.1299 ? 10.2392+-0.5614 ? might be 1.0685x slower math-cordic 6.3008+-0.0530 ? 6.3821+-0.0956 ? might be 1.0129x slower math-partial-sums 7.6734+-0.1439 7.5819+-0.1422 might be 1.0121x faster math-spectral-norm 2.3584+-0.0326 ? 2.3878+-0.0569 ? might be 1.0125x slower regexp-dna 11.1242+-0.2322 11.1141+-0.2045 string-base64 3.8605+-0.0873 ? 3.8903+-0.1043 ? string-fasta 6.5896+-0.0792 ! 6.8609+-0.1590 ! definitely 1.0412x slower string-tagcloud 11.5641+-0.2413 11.4625+-0.3104 string-unpack-code 20.4668+-0.3587 20.4124+-0.3204 string-validate-input 5.5790+-0.1431 5.4070+-0.0983 might be 1.0318x faster <arithmetic> * 6.0878+-0.0354 6.0830+-0.0295 might be 1.0008x faster <geometric> 4.8526+-0.0254 4.8472+-0.0226 might be 1.0011x faster <harmonic> 3.8238+-0.0239 3.8198+-0.0231 might be 1.0011x faster TipOfTree NoGetMethod V8: crypto 71.4757+-0.5945 ? 71.6543+-0.8676 ? deltablue 152.1513+-1.2838 151.8126+-1.0527 earley-boyer 85.1455+-0.9026 ? 87.0858+-1.3455 ? might be 1.0228x slower raytrace 56.6661+-0.2108 56.6320+-0.4331 regexp 106.8908+-1.4783 ^ 104.3510+-0.3253 ^ definitely 1.0243x faster richards 119.1301+-0.3283 ^ 117.3293+-0.2406 ^ definitely 1.0153x faster splay 75.3844+-1.1653 73.4300+-0.9663 might be 1.0266x faster <arithmetic> 95.2634+-0.3838 94.6136+-0.3198 might be 1.0069x faster <geometric> * 90.6590+-0.3797 90.0993+-0.3332 might be 1.0062x faster <harmonic> 86.4365+-0.3894 85.9689+-0.3435 might be 1.0054x faster TipOfTree NoGetMethod Kraken: ai-astar 491.0792+-0.4440 ? 494.3727+-4.1607 ? audio-beat-detection 192.6858+-2.6961 192.5974+-1.3912 audio-dft 276.7559+-5.9948 270.8880+-3.6428 might be 1.0217x faster audio-fft 126.6115+-0.8848 125.7453+-0.7885 audio-oscillator 252.6736+-3.4506 251.3422+-3.0310 imaging-darkroom 305.5309+-4.4214 303.5602+-4.0916 imaging-desaturate 217.7886+-1.3524 217.6962+-1.2634 imaging-gaussian-blur 559.9125+-3.2074 559.3110+-3.3036 json-parse-financial 59.3956+-0.4921 59.1615+-0.4678 json-stringify-tinderbox 74.2847+-0.2681 ! 76.2764+-1.0000 ! definitely 1.0268x slower stanford-crypto-aes 99.3376+-1.1413 ? 99.9840+-1.1110 ? stanford-crypto-ccm 103.0129+-1.2054 ? 104.6516+-1.4472 ? might be 1.0159x slower stanford-crypto-pbkdf2 193.9439+-0.7385 ^ 190.5408+-1.1591 ^ definitely 1.0179x faster stanford-crypto-sha256-iterative 85.1716+-0.7458 84.8109+-0.3782 <arithmetic> * 217.0132+-0.4951 216.4956+-0.5738 might be 1.0024x faster <geometric> 173.9576+-0.3890 173.8070+-0.4777 might be 1.0009x faster <harmonic> 140.9356+-0.2903 ? 141.2115+-0.4391 ? might be 1.0020x slower TipOfTree NoGetMethod All benchmarks: <arithmetic> 82.1981+-0.1766 81.9445+-0.1872 might be 1.0031x faster <geometric> 21.7955+-0.0763 21.7565+-0.0628 might be 1.0018x faster <harmonic> 6.7338+-0.0413 6.7265+-0.0398 might be 1.0011x faster TipOfTree NoGetMethod Geomean of preferred means: <scaled-result> 49.2923+-0.1406 49.1388+-0.0899 might be 1.0031x faster 509519 2 116688 barraclough 2011-11-27 18:00:59 -0800 Comment on attachment 116688 the patch r+, keeping the GetMethod code for now seems okay, but we shouldn't leave redundant code fester for too long, so we should do something to track this – maybe open another bug indicating the rest of the GetMethod code should be removed at some point? 509522 3 fpizlo 2011-11-27 18:05:50 -0800 (In reply to comment #2) > (From update of attachment 116688 [details]) > r+, keeping the GetMethod code for now seems okay, but we shouldn't leave redundant code fester for too long, so we should do something to track this – maybe open another bug indicating the rest of the GetMethod code should be removed at some point? Agree! https://bugs.webkit.org/show_bug.cgi?id=73178 509526 4 fpizlo 2011-11-27 18:15:21 -0800 Landed in http://trac.webkit.org/changeset/101217 509527 5 116688 fpizlo 2011-11-27 18:15:46 -0800 Comment on attachment 116688 the patch Clearing flags. 116688 2011-11-27 17:56:16 -0800 2011-11-27 18:15:46 -0800 the patch nogetmethod_patch_1.diff text/plain 1923 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTAxMjE2KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBA CisyMDExLTExLTI3ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg REZHIHNob3VsZCBub3QgZW1pdCBHZXRNZXRob2Qgbm9kZQorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NzMxNzUKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKyAgICAgICAgCisgICAgICAgIFJlcGxhY2VzIGFsbCBpbnN0YW5jZXMg b2YgdGhlIEdldE1ldGhvZCBub2RlIHdpdGggR2V0QnlJZC4gVGhpcyBhcHBlYXJzIHRvCisgICAg ICAgIGJlIGEgc2xpZ2h0IHdpbiBvbiBWOC4gVGhpcyBwYXRjaCBsZWF2ZXMgR2V0TWV0aG9kIHN1 cHBvcnQgaW4gdGhlIGNvZGUtYmFzZSwKKyAgICAgICAgbWFraW5nIHRoaXMgZGVjaXNpb24gZWFz eSB0byByZXZlcnNlLCBmb3Igbm93LgorCisgICAgICAgICogZGZnL0RGR0J5dGVDb2RlUGFyc2Vy LmNwcDoKKyAgICAgICAgKEpTQzo6REZHOjpCeXRlQ29kZVBhcnNlcjo6cGFyc2VCbG9jayk6CisK IDIwMTEtMTEtMjYgIEhhamltZSBNb3JyaXRhICA8bW9ycml0YUBjaHJvbWl1bS5vcmc+CiAKICAg ICAgICAgTmVlZHMgV1RGX0lOTElORSBhbmQgSlNfSU5MSU5FCkluZGV4OiBTb3VyY2UvSmF2YVNj cmlwdENvcmUvZGZnL0RGR0J5dGVDb2RlUGFyc2VyLmNwcAo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvZGZnL0RGR0J5dGVDb2RlUGFyc2VyLmNwcAkocmV2aXNpb24gMTAxMjAx KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdCeXRlQ29kZVBhcnNlci5jcHAJKHdv cmtpbmcgY29weSkKQEAgLTE2NTMsMTAgKzE2NTMsOCBAQCBib29sIEJ5dGVDb2RlUGFyc2VyOjpw YXJzZUJsb2NrKHVuc2lnbmVkCiAgICAgICAgICAgICAgICAgICAgIGFkZFRvR3JhcGgoQ2hlY2tT dHJ1Y3R1cmUsIE9wSW5mbyhtX2dyYXBoLmFkZFN0cnVjdHVyZVNldChtZXRob2RDYWxsLmNhY2hl ZFByb3RvdHlwZVN0cnVjdHVyZS5nZXQoKSkpLCBjZWxsQ29uc3RhbnQobWV0aG9kQ2FsbC5jYWNo ZWRQcm90b3R5cGUuZ2V0KCkpKTsKICAgICAgICAgICAgICAgICAKICAgICAgICAgICAgICAgICBz ZXQoZ2V0SW5zdHJ1Y3Rpb25bMV0udS5vcGVyYW5kLCBjZWxsQ29uc3RhbnQobWV0aG9kQ2FsbC5j YWNoZWRGdW5jdGlvbi5nZXQoKSkpOwotICAgICAgICAgICAgfSBlbHNlIHsKLSAgICAgICAgICAg ICAgICBOb2RlSW5kZXggZ2V0TWV0aG9kID0gYWRkVG9HcmFwaChHZXRNZXRob2QsIE9wSW5mbyhp ZGVudGlmaWVyKSwgT3BJbmZvKHByZWRpY3Rpb24pLCBiYXNlKTsKLSAgICAgICAgICAgICAgICBz ZXQoZ2V0SW5zdHJ1Y3Rpb25bMV0udS5vcGVyYW5kLCBnZXRNZXRob2QpOwotICAgICAgICAgICAg fQorICAgICAgICAgICAgfSBlbHNlCisgICAgICAgICAgICAgICAgc2V0KGdldEluc3RydWN0aW9u WzFdLnUub3BlcmFuZCwgYWRkVG9HcmFwaChHZXRCeUlkLCBPcEluZm8oaWRlbnRpZmllciksIE9w SW5mbyhwcmVkaWN0aW9uKSwgYmFzZSkpOwogICAgICAgICAgICAgCiAgICAgICAgICAgICBtX2N1 cnJlbnRJbmRleCArPSBPUENPREVfTEVOR1RIKG9wX21ldGhvZF9jaGVjaykgKyBPUENPREVfTEVO R1RIKG9wX2dldF9ieV9pZCk7CiAgICAgICAgICAgICBjb250aW51ZTsK