58994 2011-04-20 08:38:05 -0700 JSString::resolveRope inefficient for common 2 fiber case 2011-04-25 21:36:19 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 1 msaboff msaboff oldest_to_newest 389244 0 msaboff 2011-04-20 08:38:05 -0700 JSString::resolveRope is a generic routine to convert ropes into simple strings. It doesn't have an optimization for the common 2 fiber case. A restructuring of common and slow cases is in order. This would improve performance. This was found while tuning for the dromaeo DOM benchmark. 389267 1 90346 msaboff 2011-04-20 09:07:53 -0700 Created attachment 90346 Patch that splits resolveRope into common and slow cases Improves performance on dromaeo DOM-query benchmark. Suspect this will generally improve performance. Considered inlining the fast case, but felt the method was a little long for that. Although there are only two call sites for resolveRope, both are in JSString.h in common methods that will be inlined themselves. This could cause code bloat. 389471 2 90346 ggaren 2011-04-20 13:24:01 -0700 Comment on attachment 90346 Patch that splits resolveRope into common and slow cases What does SunSpider say about this change? 389856 3 msaboff 2011-04-20 22:49:34 -0700 SunSpider likes the change ;-) It is ~.7% improvement overall Noticeable improvements marked with <=== Test Without patch With patch Total 201.4ms +/- 0.1% 199.9ms +/- 0.1% 3d 28.2ms +/- 0.6% 28.1ms +/- 0.5% cube 10.0ms +/- 0.0% 10.0ms +/- 0.0% morph 8.1ms +/- 1.5% 8.1ms +/- 1.7% raytrace 10.1ms +/- 0.7% 10.0ms +/- 0.0% access 26.6ms +/- 0.5% 26.6ms +/- 0.5% binary-trees 2.5ms +/- 5.6% 2.6ms +/- 5.6% fannkuch 13.0ms +/- 0.0% 13.0ms +/- 0.3% nbody 7.0ms +/- 0.0% 7.0ms +/- 0.0% nsieve 4.0ms +/- 1.0% 4.0ms +/- 0.0% bitops 15.0ms +/- 0.0% 15.0ms +/- 0.0% 3bit-bits-in-byte 2.0ms +/- 0.0% 2.0ms +/- 0.0% bits-in-byte 5.0ms +/- 0.0% 5.0ms +/- 0.0% bitwise-and 3.0ms +/- 0.0% 3.0ms +/- 0.0% nsieve-bits 5.0ms +/- 0.0% 5.0ms +/- 0.0% controlflow 2.0ms +/- 0.0% 2.0ms +/- 0.0% recursive 2.0ms +/- 0.0% 2.0ms +/- 0.0% crypto 12.5ms +/- 1.1% 12.1ms +/- 0.9% aes 7.5ms +/- 1.9% 7.1ms +/- 1.5% <=== md5 3.0ms +/- 0.0% 3.0ms +/- 0.0% sha1 2.0ms +/- 0.0% 2.0ms +/- 0.0% date 23.3ms +/- 0.6% 23.0ms +/- 0.0% format-tofte 13.3ms +/- 1.0% 13.0ms +/- 0.0% <=== format-xparb 18.2ms +/- 0.6% 18.1ms +/- 0.4% cordic 5.0ms +/- 0.8% 5.0ms +/- 0.8% partial-sums 9.2ms +/- 1.2% 9.0ms +/- 0.6% <=== spectral-norm 4.0ms +/- 0.0% 4.0ms +/- 0.0% regexp 12.0ms +/- 0.5% 11.8ms +/- 1.0% dna 12.0ms +/- 0.5% 11.8ms +/- 1.0% <=== string 63.6ms +/- 0.3% 63.3ms +/- 0.2% base64 6.3ms +/- 2.1% 6.1ms +/- 1.3% <=== fasta 8.0ms +/- 0.0% 8.0ms +/- 0.0% tagcloud 16.2ms +/- 0.8% 16.2ms +/- 0.7% unpack-code 25.1ms +/- 0.3% 25.0ms +/- 0.2% validate-input 8.0ms +/- 0.0% 8.0ms +/- 0.0% 390122 4 msaboff 2011-04-21 10:23:20 -0700 Committed r84515: <http://trac.webkit.org/changeset/84515> 392238 5 90346 eric 2011-04-25 21:36:19 -0700 Comment on attachment 90346 Patch that splits resolveRope into common and slow cases Cleared review? from attachment 90346 so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again). 90346 2011-04-20 09:07:53 -0700 2011-04-25 21:36:19 -0700 Patch that splits resolveRope into common and slow cases 58994.patch text/plain 5667 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gODQzNzYpCisrKyBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjIgQEAK KzIwMTEtMDQtMjAgIE1pY2hhZWwgU2Fib2ZmICA8bXNhYm9mZkBhcHBsZS5jb20+CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSlNTdHJpbmc6OnJlc29s dmVSb3BlIGluZWZmaWNpZW50IGZvciBjb21tb24gMiBmaWJlciBjYXNlCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD01ODk5NAorCisgICAgICAgIFNwbGl0 IEpTU3RyaW5nOjpyZXNvbHZlUm9wZSBpbnRvIHRocmVlIHJvdXRpbmVzLgorICAgICAgICByZXNv bHZlUm9wZSBhbGxvY2F0ZXMgdGhlIG5ldyBidWZmZXIgYW5kIGhhbmRsZXMgdGhlIDEgb3IgMgor ICAgICAgICBmaWJlciBjYXNlIHdpdGggc2luZ2xlIGxldmVsIGZpYmVycy4KKyAgICAgICAgcmVz b2x2ZVJvcGVTbG93Q2FzZSBoYW5kbGVzIHRoZSBnZW5lcmFsIGNhc2UuCisgICAgICAgIG91dE9m TWVtb3J5IGhhbmRsZXMgdGhlIHJhcmUgb3V0IG9mIG1lbW9yeSBleGNlcHRpb24gY2FzZS4KKwor ICAgICAgICAqIHJ1bnRpbWUvSlNTdHJpbmcuY3BwOgorICAgICAgICAoSlNDOjpKU1N0cmluZzo6 cmVzb2x2ZVJvcGUpOgorICAgICAgICAoSlNDOjpKU1N0cmluZzo6cmVzb2x2ZVJvcGVTbG93Q2Fz ZSk6CisgICAgICAgIChKU0M6OkpTU3RyaW5nOjpvdXRPZk1lbW9yeSk6CisgICAgICAgICogcnVu dGltZS9KU1N0cmluZy5oOgorCiAyMDExLTA0LTIwICBCYWxhenMgS2VsZW1lbiAgPGtiYWxhenNA d2Via2l0Lm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBDc2FiYSBPc3p0cm9nb27DoWMuCklu ZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU1N0cmluZy5jcHAKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNTdHJpbmcuY3BwCShyZXZpc2lv biA4NDM2NykKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTU3RyaW5nLmNwcAko d29ya2luZyBjb3B5KQpAQCAtMzQsNiArMzQsNTAgQEAgbmFtZXNwYWNlIEpTQyB7CiAgICAgCiBz dGF0aWMgY29uc3QgdW5zaWduZWQgc3Vic3RyaW5nRnJvbVJvcGVDdXRvZmYgPSA0OwogCit2b2lk IEpTU3RyaW5nOjpyZXNvbHZlUm9wZShFeGVjU3RhdGUqIGV4ZWMpIGNvbnN0Cit7CisgICAgQVNT RVJUKGlzUm9wZSgpKTsKKworICAgIFVDaGFyKiBidWZmZXI7CisgICAgaWYgKFBhc3NSZWZQdHI8 U3RyaW5nSW1wbD4gbmV3SW1wbCA9IFN0cmluZ0ltcGw6OnRyeUNyZWF0ZVVuaW5pdGlhbGl6ZWQo bV9sZW5ndGgsIGJ1ZmZlcikpCisgICAgICAgIG1fdmFsdWUgPSBuZXdJbXBsOworICAgIGVsc2Ug eworICAgICAgICBvdXRPZk1lbW9yeShleGVjKTsKKyAgICAgICAgcmV0dXJuOworICAgIH0KKwor ICAgIFJvcGVJbXBsOjpGaWJlciBjdXJyZW50RmliZXIgPSBtX290aGVyLm1fZmliZXJzWzBdOwor CisgICAgaWYgKChtX2ZpYmVyQ291bnQgPiAyKSB8fCAoUm9wZUltcGw6OmlzUm9wZShjdXJyZW50 RmliZXIpKSAKKyAgICAgICAgfHwgKChtX2ZpYmVyQ291bnQgPT0gMikgJiYgKFJvcGVJbXBsOjpp c1JvcGUobV9vdGhlci5tX2ZpYmVyc1sxXSkpKSkgeworICAgICAgICByZXNvbHZlUm9wZVNsb3dD YXNlKGV4ZWMsIGJ1ZmZlcik7CisgICAgICAgIHJldHVybjsKKyAgICB9CisKKyAgICBVQ2hhciog cG9zaXRpb24gPSBidWZmZXI7CisgICAgU3RyaW5nSW1wbCogc3RyaW5nID0gc3RhdGljX2Nhc3Q8 U3RyaW5nSW1wbCo+KGN1cnJlbnRGaWJlcik7CisgICAgdW5zaWduZWQgbGVuZ3RoID0gc3RyaW5n LT5sZW5ndGgoKTsKKyAgICBTdHJpbmdJbXBsOjpjb3B5Q2hhcnMocG9zaXRpb24sIHN0cmluZy0+ Y2hhcmFjdGVycygpLCBsZW5ndGgpOworCisgICAgaWYgKG1fZmliZXJDb3VudCA+IDEpIHsKKyAg ICAgICAgcG9zaXRpb24gKz0gbGVuZ3RoOworICAgICAgICBjdXJyZW50RmliZXIgPSBtX290aGVy Lm1fZmliZXJzWzFdOworICAgICAgICBzdHJpbmcgPSBzdGF0aWNfY2FzdDxTdHJpbmdJbXBsKj4o Y3VycmVudEZpYmVyKTsKKyAgICAgICAgbGVuZ3RoID0gc3RyaW5nLT5sZW5ndGgoKTsKKyAgICAg ICAgU3RyaW5nSW1wbDo6Y29weUNoYXJzKHBvc2l0aW9uLCBzdHJpbmctPmNoYXJhY3RlcnMoKSwg bGVuZ3RoKTsKKyAgICAgICAgcG9zaXRpb24gKz0gbGVuZ3RoOworICAgIH0KKworICAgIEFTU0VS VCgoYnVmZmVyICsgbV9sZW5ndGgpID09IHBvc2l0aW9uKTsKKyAgICBmb3IgKHVuc2lnbmVkIGkg PSAwOyBpIDwgbV9maWJlckNvdW50OyArK2kpIHsKKyAgICAgICAgUm9wZUltcGw6OmRlcmVmKG1f b3RoZXIubV9maWJlcnNbaV0pOworICAgICAgICBtX290aGVyLm1fZmliZXJzW2ldID0gMDsKKyAg ICB9CisgICAgbV9maWJlckNvdW50ID0gMDsKKworICAgIEFTU0VSVCghaXNSb3BlKCkpOworfQor CiAvLyBPdmVydmlldzogdGhpcyBtZXRob2RzIGNvbnZlcnRzIGEgSlNTdHJpbmcgZnJvbSBob2xk aW5nIGEgc3RyaW5nIGluIHJvcGUgZm9ybQogLy8gZG93biB0byBhIHNpbXBsZSBVU3RyaW5nIHJl cHJlc2VudGF0aW9uLiAgSXQgZG9lcyBzbyBieSBidWlsZGluZyB1cCB0aGUgc3RyaW5nCiAvLyBi YWNrd2FyZHMsIHNpbmNlIHdlIHdhbnQgdG8gYXZvaWQgcmVjdXJzaW9uLCB3ZSBleHBlY3QgdGhh dCB0aGUgdHJlZSBzdHJ1Y3R1cmUKQEAgLTQzLDI3ICs4NywxMSBAQCBzdGF0aWMgY29uc3QgdW5z aWduZWQgc3Vic3RyaW5nRnJvbVJvcGVDCiAvLyB3ZSB3b3VsZCBsaWtlbHkgaGF2ZSB0byBwbGFj ZSBhbGwgb2YgdGhlIGNvbnN0aXR1ZW50IFN0cmluZ0ltcGxzIGludG8gdGhlCiAvLyBWZWN0b3Ig YmVmb3JlIHBlcmZvcm1pbmcgYW55IGNvbmNhdGVuYXRpb24sIGJ1dCBieSB3b3JraW5nIGJhY2t3 YXJkcyB3ZSBsaWtlbHkKIC8vIG9ubHkgZmlsbCB0aGUgcXVldWUgd2l0aCB0aGUgbnVtYmVyIG9m IHN1YnN0cmluZ3MgYXQgYW55IGdpdmVuIGxldmVsIGluIGEKLS8vIHJvcGUtb2Ytcm9wZXMuKQot dm9pZCBKU1N0cmluZzo6cmVzb2x2ZVJvcGUoRXhlY1N0YXRlKiBleGVjKSBjb25zdAorLy8gcm9w ZS1vZi1yb3Blcy4pICAgIAordm9pZCBKU1N0cmluZzo6cmVzb2x2ZVJvcGVTbG93Q2FzZShFeGVj U3RhdGUqIGV4ZWMsIFVDaGFyKiBidWZmZXIpIGNvbnN0CiB7Ci0gICAgQVNTRVJUKGlzUm9wZSgp KTsKKyAgICBVTlVTRURfUEFSQU0oZXhlYyk7CiAKLSAgICAvLyBBbGxvY2F0ZSB0aGUgYnVmZmVy IHRvIGhvbGQgdGhlIGZpbmFsIHN0cmluZywgcG9zaXRpb24gaW5pdGlhbGx5IHBvaW50cyB0byB0 aGUgZW5kLgotICAgIFVDaGFyKiBidWZmZXI7Ci0gICAgaWYgKFBhc3NSZWZQdHI8U3RyaW5nSW1w bD4gbmV3SW1wbCA9IFN0cmluZ0ltcGw6OnRyeUNyZWF0ZVVuaW5pdGlhbGl6ZWQobV9sZW5ndGgs IGJ1ZmZlcikpCi0gICAgICAgIG1fdmFsdWUgPSBuZXdJbXBsOwotICAgIGVsc2UgewotICAgICAg ICBmb3IgKHVuc2lnbmVkIGkgPSAwOyBpIDwgbV9maWJlckNvdW50OyArK2kpIHsKLSAgICAgICAg ICAgIFJvcGVJbXBsOjpkZXJlZihtX290aGVyLm1fZmliZXJzW2ldKTsKLSAgICAgICAgICAgIG1f b3RoZXIubV9maWJlcnNbaV0gPSAwOwotICAgICAgICB9Ci0gICAgICAgIG1fZmliZXJDb3VudCA9 IDA7Ci0gICAgICAgIEFTU0VSVCghaXNSb3BlKCkpOwotICAgICAgICBBU1NFUlQobV92YWx1ZSA9 PSBVU3RyaW5nKCkpOwotICAgICAgICBpZiAoZXhlYykKLSAgICAgICAgICAgIHRocm93T3V0T2ZN ZW1vcnlFcnJvcihleGVjKTsKLSAgICAgICAgcmV0dXJuOwotICAgIH0KICAgICBVQ2hhciogcG9z aXRpb24gPSBidWZmZXIgKyBtX2xlbmd0aDsKIAogICAgIC8vIFN0YXJ0IHdpdGggdGhlIGN1cnJl bnQgUm9wZUltcGwuCkBAIC05Niw3ICsxMjQsNyBAQCB2b2lkIEpTU3RyaW5nOjpyZXNvbHZlUm9w ZShFeGVjU3RhdGUqIGV4CiAgICAgICAgICAgICAgICAgICAgIG1fb3RoZXIubV9maWJlcnNbaV0g PSAwOwogICAgICAgICAgICAgICAgIH0KICAgICAgICAgICAgICAgICBtX2ZpYmVyQ291bnQgPSAw OwotCisgICAgICAgICAgICAgICAgCiAgICAgICAgICAgICAgICAgQVNTRVJUKCFpc1JvcGUoKSk7 CiAgICAgICAgICAgICAgICAgcmV0dXJuOwogICAgICAgICAgICAgfQpAQCAtMTA3LDcgKzEzNSwy MCBAQCB2b2lkIEpTU3RyaW5nOjpyZXNvbHZlUm9wZShFeGVjU3RhdGUqIGV4CiAgICAgICAgIH0K ICAgICB9CiB9Ci0gICAgCisKK3ZvaWQgSlNTdHJpbmc6Om91dE9mTWVtb3J5KEV4ZWNTdGF0ZSog ZXhlYykgY29uc3QKK3sKKyAgICBmb3IgKHVuc2lnbmVkIGkgPSAwOyBpIDwgbV9maWJlckNvdW50 OyArK2kpIHsKKyAgICAgICAgUm9wZUltcGw6OmRlcmVmKG1fb3RoZXIubV9maWJlcnNbaV0pOwor ICAgICAgICBtX290aGVyLm1fZmliZXJzW2ldID0gMDsKKyAgICB9CisgICAgbV9maWJlckNvdW50 ID0gMDsKKyAgICBBU1NFUlQoIWlzUm9wZSgpKTsKKyAgICBBU1NFUlQobV92YWx1ZSA9PSBVU3Ry aW5nKCkpOworICAgIGlmIChleGVjKQorICAgICAgICB0aHJvd091dE9mTWVtb3J5RXJyb3IoZXhl Yyk7Cit9CisKIC8vIFRoaXMgZnVuY3Rpb24gY29uc3RydXN0cyBhIHN1YnN0cmluZyBvdXQgb2Yg YSByb3BlIHdpdGhvdXQgZmxhdHRlbmluZyBieSByZXVzaW5nIHRoZSBleGlzdGluZyBmaWJlcnMu CiAvLyBUaGlzIGNhbiByZWR1Y2UgbWVtb3J5IHVzYWdlIHN1YnN0YW50aWFsbHkuIFNpbmNlIHRy YXZlcnNpbmcgcm9wZXMgaXMgc2xvdyB0aGUgZnVuY3Rpb24gd2lsbCByZXZlcnQgCiAvLyBiYWNr IHRvIGZsYXR0ZW5pbmcgaWYgdGhlIHJvcGUgdHVybnMgb3V0IHRvIGJlIGxvbmcuCkluZGV4OiBT b3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU1N0cmluZy5oCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTU3RyaW5nLmgJKHJldmlzaW9uIDg0MzY3KQor KysgU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNTdHJpbmcuaAkod29ya2luZyBjb3B5 KQpAQCAtMzU5LDYgKzM1OSw4IEBAIG5hbWVzcGFjZSBKU0MgewogICAgICAgICB9CiAKICAgICAg ICAgdm9pZCByZXNvbHZlUm9wZShFeGVjU3RhdGUqKSBjb25zdDsKKyAgICAgICAgdm9pZCByZXNv bHZlUm9wZVNsb3dDYXNlKEV4ZWNTdGF0ZSosIFVDaGFyKikgY29uc3Q7CisgICAgICAgIHZvaWQg b3V0T2ZNZW1vcnkoRXhlY1N0YXRlKikgY29uc3Q7CiAgICAgICAgIEpTU3RyaW5nKiBzdWJzdHJp bmdGcm9tUm9wZShFeGVjU3RhdGUqLCB1bnNpZ25lZCBvZmZzZXQsIHVuc2lnbmVkIGxlbmd0aCk7 CiAKICAgICAgICAgdm9pZCBhcHBlbmRTdHJpbmdJbkNvbnN0cnVjdCh1bnNpZ25lZCYgaW5kZXgs IGNvbnN0IFVTdHJpbmcmIHN0cmluZykK