73584 2011-12-01 13:11:43 -0800 Changes proposed for 73457 slow down Kraken json-parse-financial 2011-12-05 15:20:13 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 73236 1 msaboff msaboff oldest_to_newest 513030 0 msaboff 2011-12-01 13:11:43 -0800 The changes proposed for bug 73457 slow down Kraken json-parse-financial by about 3%. 513045 1 117475 msaboff 2011-12-01 13:25:27 -0800 Created attachment 117475 Patch 513169 2 117475 darin 2011-12-01 15:07:26 -0800 Comment on attachment 117475 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117475&action=review Why 4 bytes and not 8 on 64-bit architectures? Can this share code with StringHash.h instead of adding more copies of code that does the same thing? > Source/JavaScriptCore/wtf/text/StringImpl.cpp:714 > +#if !(CPU(ARM) || CPU(SH4) || CPU(MIPS) || CPU(SPARC)) StringHash.h uses a different list for this. 513413 3 117475 sam 2011-12-01 19:44:16 -0800 Comment on attachment 117475 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117475&action=review Please take Darin's comment into account. > Source/JavaScriptCore/wtf/text/StringImpl.cpp:754 > + if (length & 1 && *reinterpret_cast<const UChar*>(aCharacters) != *reinterpret_cast<const UChar*>(bCharacters)) Extra whitespace between && * 513506 4 msaboff 2011-12-01 22:35:08 -0800 (In reply to comment #2) > (From update of attachment 117475 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=117475&action=review > > Why 4 bytes and not 8 on 64-bit architectures? Can this share code with StringHash.h instead of adding more copies of code that does the same thing? > > > Source/JavaScriptCore/wtf/text/StringImpl.cpp:714 > > +#if !(CPU(ARM) || CPU(SH4) || CPU(MIPS) || CPU(SPARC)) > > StringHash.h uses a different list for this. Spent more time on this and added 64 bit comparisons for x86-64. This change sped up SunSpider string-unpack-code by ~3%. Made an initial stab at merging the StringHash code with this code, but it proved to be more work than expected. Putting the new equal() implementations in StringImpl.h and calling them from StringHash.h didn't work. The compiler tried to convert the first argument LChar* to a String which ended up with a recursive call. Created https://bugs.webkit.org/show_bug.cgi?id=73622 "Eliminate Duplicate word at a time equal code in StringImpl.cpp and StringHash.h" to address this duplicity. 513535 5 msaboff 2011-12-01 23:06:56 -0800 Committed r101747: <http://trac.webkit.org/changeset/101747> 117475 2011-12-01 13:25:27 -0800 2011-12-01 19:44:16 -0800 Patch 73584.patch text/plain 5093 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTAxNjk5KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBA CisyMDExLTEyLTAxICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIENoYW5nZXMgcHJvcG9zZWQgZm9yIDczNDU3IHNsb3cgZG93biBLcmFrZW4ganNvbi1wYXJz ZS1maW5hbmNpYWwKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTczNTg0CisKKyAgICAgICAgUmVzdHJ1Y3R1cmVkIFN0cmluZ0ltcGw6OmVxdWFsIHRvIHRh a2UgYWR2YW50YWdlIG9mIDQgYnl0ZXMKKyAgICAgICAgYXQgYSB0aW1lIHdoZW4gcG9zc2libGUu CisKKyAgICAgICAgVGhpcyBpcyB3b3J0aCB+MyUgb24gS3Jha2VuIGpzb24tcGFyc2UtZmluYW5j aWFsLiAgSXQgYWxzbyAKKyAgICAgICAgYmVuZWZpdHMgdmFyaW91cyBvdGhlciBiZW5jaG1hcmtz LgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogd3Rm L3RleHQvU3RyaW5nSW1wbC5jcHA6CisgICAgICAgIChXVEY6OmVxdWFsKToKKwogMjAxMS0xMi0w MSAgRmlsaXAgUGl6bG8gIDxmcGl6bG9AYXBwbGUuY29tPgogCiAgICAgICAgIEFSTXY3IG9ubHkg YWxsb3dzIGZvciBvbmUtc2hvdCBwYXRjaGluZyBvZiBjb21wYWN0IG9mZnNldHMsIHdoaWxlIHRo ZQpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL3d0Zi90ZXh0L1N0cmluZ0ltcGwuY3BwCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvdGV4dC9TdHJpbmdJbXBsLmNw cAkocmV2aXNpb24gMTAxNDM5KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL3d0Zi90ZXh0L1N0 cmluZ0ltcGwuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC03MTEsMTQgKzcxMSw2MSBAQCBmbG9hdCBT dHJpbmdJbXBsOjp0b0Zsb2F0KGJvb2wqIG9rLCBib29sCiAgICAgcmV0dXJuIGNoYXJhY3RlcnNU b0Zsb2F0KGNoYXJhY3RlcnMxNigpLCBtX2xlbmd0aCwgb2ssIGRpZFJlYWROdW1iZXIpOwogfQog Ci1zdGF0aWMgYm9vbCBlcXVhbChjb25zdCBVQ2hhciogYSwgY29uc3QgTENoYXIqIGIsIGludCBs ZW5ndGgpCisjaWYgIShDUFUoQVJNKSB8fCBDUFUoU0g0KSB8fCBDUFUoTUlQUykgfHwgQ1BVKFNQ QVJDKSkKKy8vIERvIGNvbXBhcmlzb25zIDQtYnl0ZXMtYXQtYS10aW1lIG9uIGFyY2hpdGVjdHVy ZXMgd2hlcmUgaXQncyBzYWZlLgorCitBTFdBWVNfSU5MSU5FIHN0YXRpYyBib29sIGVxdWFsKGNv bnN0IExDaGFyKiBhLCBjb25zdCBMQ2hhciogYiwgdW5zaWduZWQgbGVuZ3RoKQogewotICAgIEFT U0VSVChsZW5ndGggPj0gMCk7Ci0gICAgd2hpbGUgKGxlbmd0aC0tKSB7Ci0gICAgICAgIExDaGFy IGJjID0gKmIrKzsKLSAgICAgICAgaWYgKCphKysgIT0gYmMpCisgICAgY29uc3QgdWludDMyX3Qq IGFDaGFyYWN0ZXJzID0gcmVpbnRlcnByZXRfY2FzdDxjb25zdCB1aW50MzJfdCo+KGEpOworICAg IGNvbnN0IHVpbnQzMl90KiBiQ2hhcmFjdGVycyA9IHJlaW50ZXJwcmV0X2Nhc3Q8Y29uc3QgdWlu dDMyX3QqPihiKTsKKworICAgIHVuc2lnbmVkIHdvcmRMZW5ndGggPSBsZW5ndGggPj4gMjsKKyAg ICBmb3IgKHVuc2lnbmVkIGkgPSAwOyBpICE9IHdvcmRMZW5ndGg7ICsraSkgeworICAgICAgICBp ZiAoKmFDaGFyYWN0ZXJzKysgIT0gKmJDaGFyYWN0ZXJzKyspCisgICAgICAgICAgICByZXR1cm4g ZmFsc2U7CisgICAgfQorCisgICAgbGVuZ3RoICY9IDM7CisKKyAgICBpZiAobGVuZ3RoKSB7Cisg ICAgICAgIGNvbnN0IExDaGFyKiBhUmVtYWluZGVyID0gcmVpbnRlcnByZXRfY2FzdDxjb25zdCBM Q2hhcio+KGFDaGFyYWN0ZXJzKTsKKyAgICAgICAgY29uc3QgTENoYXIqIGJSZW1haW5kZXIgPSBy ZWludGVycHJldF9jYXN0PGNvbnN0IExDaGFyKj4oYkNoYXJhY3RlcnMpOworCisgICAgICAgIGZv ciAodW5zaWduZWQgaSA9IDA7IGkgPCAgbGVuZ3RoOyArK2kpIHsKKyAgICAgICAgICAgIGlmIChh UmVtYWluZGVyW2ldICE9IGJSZW1haW5kZXJbaV0pCisgICAgICAgICAgICAgICAgcmV0dXJuIGZh bHNlOworICAgICAgICB9CisgICAgfQorCisgICAgcmV0dXJuIHRydWU7Cit9CisKK0FMV0FZU19J TkxJTkUgc3RhdGljIGJvb2wgZXF1YWwoY29uc3QgVUNoYXIqIGEsIGNvbnN0IFVDaGFyKiBiLCB1 bnNpZ25lZCBsZW5ndGgpCit7CisgICAgY29uc3QgdWludDMyX3QqIGFDaGFyYWN0ZXJzID0gcmVp bnRlcnByZXRfY2FzdDxjb25zdCB1aW50MzJfdCo+KGEpOworICAgIGNvbnN0IHVpbnQzMl90KiBi Q2hhcmFjdGVycyA9IHJlaW50ZXJwcmV0X2Nhc3Q8Y29uc3QgdWludDMyX3QqPihiKTsKKworICAg IHVuc2lnbmVkIHdvcmRMZW5ndGggPSBsZW5ndGggPj4gMTsKKyAgICBmb3IgKHVuc2lnbmVkIGkg PSAwOyBpICE9IHdvcmRMZW5ndGg7ICsraSkgeworICAgICAgICBpZiAoKmFDaGFyYWN0ZXJzKysg IT0gKmJDaGFyYWN0ZXJzKyspCiAgICAgICAgICAgICByZXR1cm4gZmFsc2U7CiAgICAgfQorCisg ICAgaWYgKGxlbmd0aCAmIDEgJiYgICpyZWludGVycHJldF9jYXN0PGNvbnN0IFVDaGFyKj4oYUNo YXJhY3RlcnMpICE9ICpyZWludGVycHJldF9jYXN0PGNvbnN0IFVDaGFyKj4oYkNoYXJhY3RlcnMp KQorICAgICAgICByZXR1cm4gZmFsc2U7CisKKyAgICByZXR1cm4gdHJ1ZTsKK30KKyNlbmRpZgor Cit0ZW1wbGF0ZSA8dHlwZW5hbWUgQ2hhclR5cGVMLCB0eXBlbmFtZSBDaGFyVHlwZVI+CitBTFdB WVNfSU5MSU5FIHN0YXRpYyBib29sIGVxdWFsKGNvbnN0IENoYXJUeXBlTCogYSwgY29uc3QgQ2hh clR5cGVSKiBiLCB1bnNpZ25lZCBsZW5ndGgpCit7CisgICAgZm9yICh1bnNpZ25lZCBpID0gMDsg aSAhPSBsZW5ndGg7ICsraSkgeworICAgICAgICBpZiAoYVtpXSAhPSBiW2ldKQorICAgICAgICAg ICAgcmV0dXJuIGZhbHNlOworICAgIH0KKwogICAgIHJldHVybiB0cnVlOwogfQogCkBAIC0xMzUw LDE3ICsxMzk3LDYgQEAgYm9vbCBlcXVhbChjb25zdCBTdHJpbmdJbXBsKiBhLCBjb25zdCBTdAog ICAgIHJldHVybiBTdHJpbmdIYXNoOjplcXVhbChhLCBiKTsKIH0KIAotdGVtcGxhdGUgPHR5cGVu YW1lIENoYXJUeXBlTCwgdHlwZW5hbWUgQ2hhclR5cGVSPgotQUxXQVlTX0lOTElORSBzdGF0aWMg Ym9vbCBlcXVhbChjb25zdCBDaGFyVHlwZUwqIGEsIGNvbnN0IENoYXJUeXBlUiogYiwgdW5zaWdu ZWQgbGVuZ3RoKQotewotICAgIGZvciAodW5zaWduZWQgaSA9IDA7IGkgIT0gbGVuZ3RoOyArK2kp IHsKLSAgICAgICAgaWYgKGFbaV0gIT0gYltpXSkKLSAgICAgICAgICAgIHJldHVybiBmYWxzZTsK LSAgICB9Ci0gICAgCi0gICAgcmV0dXJuIHRydWU7Ci19Ci0KIGJvb2wgZXF1YWwoY29uc3QgU3Ry aW5nSW1wbCogYSwgY29uc3QgTENoYXIqIGIsIHVuc2lnbmVkIGxlbmd0aCkKIHsKICAgICBpZiAo IWEpCkBAIC0xNDIwLDM5ICsxNDU2LDkgQEAgYm9vbCBlcXVhbChjb25zdCBTdHJpbmdJbXBsKiBh LCBjb25zdCBVQwogCiAgICAgaWYgKGEtPmxlbmd0aCgpICE9IGxlbmd0aCkKICAgICAgICAgcmV0 dXJuIGZhbHNlOwotICAgIC8vIEZJWE1FOiBwZXJoYXBzIHdlIHNob3VsZCBoYXZlIGEgbW9yZSBh YnN0cmFjdCBtYWNybyB0aGF0IGluZGljYXRlcyB3aGVuCi0gICAgLy8gZ29pbmcgNCBieXRlcyBh dCBhIHRpbWUgaXMgdW5zYWZlCi0jaWYgQ1BVKEFSTSkgfHwgQ1BVKFNINCkgfHwgQ1BVKE1JUFMp IHx8IENQVShTUEFSQykKLSAgICBjb25zdCBVQ2hhciogYXMgPSBhLT5jaGFyYWN0ZXJzKCk7Ci0g ICAgZm9yICh1bnNpZ25lZCBpID0gMDsgaSAhPSBsZW5ndGg7ICsraSkKLSAgICAgICAgaWYgKGFz W2ldICE9IGJbaV0pCi0gICAgICAgICAgICByZXR1cm4gZmFsc2U7Ci0gICAgcmV0dXJuIHRydWU7 Ci0jZWxzZQotICAgIGlmIChhLT5pczhCaXQoKSkgewotICAgICAgICBjb25zdCBMQ2hhciogYXMg PSBhLT5jaGFyYWN0ZXJzOCgpOwotICAgICAgICBmb3IgKHVuc2lnbmVkIGkgPSAwOyBpICE9IGxl bmd0aDsgKytpKQotICAgICAgICAgICAgaWYgKGFzW2ldICE9IGJbaV0pCi0gICAgICAgICAgICAg ICAgcmV0dXJuIGZhbHNlOwotICAgICAgICByZXR1cm4gdHJ1ZTsKLSAgICB9Ci0KLSAgICAvLyBE byBjb21wYXJpc29uIDQtYnl0ZXMtYXQtYS10aW1lIG9uIGFyY2hpdGVjdHVyZXMgd2hlcmUgaXQn cyBzYWZlLgotCi0gICAgY29uc3QgdWludDMyX3QqIGFDaGFyYWN0ZXJzID0gcmVpbnRlcnByZXRf Y2FzdDxjb25zdCB1aW50MzJfdCo+KGEtPmNoYXJhY3RlcnMxNigpKTsKLSAgICBjb25zdCB1aW50 MzJfdCogYkNoYXJhY3RlcnMgPSByZWludGVycHJldF9jYXN0PGNvbnN0IHVpbnQzMl90Kj4oYik7 Ci0KLSAgICB1bnNpZ25lZCBoYWxmTGVuZ3RoID0gbGVuZ3RoID4+IDE7Ci0gICAgZm9yICh1bnNp Z25lZCBpID0gMDsgaSAhPSBoYWxmTGVuZ3RoOyArK2kpIHsKLSAgICAgICAgaWYgKCphQ2hhcmFj dGVycysrICE9ICpiQ2hhcmFjdGVycysrKQotICAgICAgICAgICAgcmV0dXJuIGZhbHNlOwotICAg IH0KLQotICAgIGlmIChsZW5ndGggJiAxICYmICAqcmVpbnRlcnByZXRfY2FzdDxjb25zdCB1aW50 MTZfdCo+KGFDaGFyYWN0ZXJzKSAhPSAqcmVpbnRlcnByZXRfY2FzdDxjb25zdCB1aW50MTZfdCo+ KGJDaGFyYWN0ZXJzKSkKLSAgICAgICAgcmV0dXJuIGZhbHNlOwotCi0gICAgcmV0dXJuIHRydWU7 Ci0jZW5kaWYKKyAgICBpZiAoYS0+aXM4Qml0KCkpCisgICAgICAgIHJldHVybiBlcXVhbChhLT5j aGFyYWN0ZXJzOCgpLCBiLCBsZW5ndGgpOworICAgIHJldHVybiBlcXVhbChhLT5jaGFyYWN0ZXJz MTYoKSwgYiwgbGVuZ3RoKTsKIH0KIAogYm9vbCBlcXVhbElnbm9yaW5nQ2FzZShTdHJpbmdJbXBs KiBhLCBTdHJpbmdJbXBsKiBiKQo=