144731 2015-05-07 01:43:19 -0700 Special-case Int32 values in JSON.stringify(). 2015-05-07 11:25:39 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED Performance P2 Normal --- 1 kling kling benjamin ggaren kling msaboff oldest_to_newest 1092545 0 kling 2015-05-07 01:43:19 -0700 We should have a fast-path in JSON.stringify() for Int32 values. 1092546 1 252580 kling 2015-05-07 01:46:10 -0700 Created attachment 252580 Patch Boom: json-stringify-tinderbox 60.176+-8.602 ^ 39.277+-1.632 ^ definitely 1.5321x faster 1092584 2 252580 msaboff 2015-05-07 07:50:44 -0700 Comment on attachment 252580 Patch r=me 1092596 3 252580 kling 2015-05-07 08:58:01 -0700 Comment on attachment 252580 Patch Clearing flags on attachment: 252580 Committed r183928: <http://trac.webkit.org/changeset/183928> 1092597 4 kling 2015-05-07 08:58:06 -0700 All reviewed patches have been landed. Closing bug. 1092605 5 252580 darin 2015-05-07 09:14:27 -0700 Comment on attachment 252580 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252580&action=review > Source/JavaScriptCore/runtime/JSONObject.cpp:403 > if (value.isNumber()) { > - double number = value.asNumber(); > - if (!std::isfinite(number)) > - builder.appendLiteral("null"); > - else > - builder.appendECMAScriptNumber(number); > + if (value.isInt32()) > + builder.appendNumber(value.asInt32()); > + else { > + double number = value.asNumber(); > + if (!std::isfinite(number)) > + builder.appendLiteral("null"); > + else > + builder.appendECMAScriptNumber(number); > + } > return StringifySucceeded; > } Would we get better code by writing these two separately? I ask because isNumber() is really just isInt32() || isDouble(). And if isDouble() is true then we can call asDouble(). Maybe the compiler will fold everything for us and remove all the dead code, but I think it would be nicer to just write it out like this: if (value.isInt32()) { builder.appendNumber(value.asInt32()); return StringifySucceeded; } if (value.isDouble()) { double number = value.asDouble(); if (!std::isfinite(number)) builder.appendLiteral("null"); else builder.appendECMAScriptNumber(number); return StringifySucceeded; } 1092655 6 ggaren 2015-05-07 11:25:15 -0700 c:\cygwin\home\buildbot\webkit\webkitbuild\release\include\private\javascriptcore\X86Assembler.h(1811): error C2039: 'twoByteOp64' : is not a member of 'JSC::X86Assembler::X86InstructionFormatter' [C:\cygwin\home\buildbot\WebKit\Source\JavaScriptCore\JavaScriptCore.vcxproj\LLInt\LLIntOffsetsExtractor\LLIntOffsetsExtractor.vcxproj] c:\cygwin\home\buildbot\webkit\webkitbuild\release\include\private\javascriptcore\X86Assembler.h(2332) : see declaration of 'JSC::X86Assembler::X86InstructionFormatter' 1092658 7 ggaren 2015-05-07 11:25:39 -0700 Oops! Wrong bug :(. 252580 2015-05-07 01:46:10 -0700 2015-05-07 08:58:01 -0700 Patch bug-144731.diff text/plain 1766 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDMyMzhkMGYuLjdmOGMxMjggMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRD b3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDE1LTA1LTA3ICBBbmRyZWFzIEtsaW5n ICA8YWtsaW5nQGFwcGxlLmNvbT4KKworICAgICAgICBTcGVjaWFsLWNhc2UgSW50MzIgdmFsdWVz IGluIEpTT04uc3RyaW5naWZ5KCkuCisgICAgICAgIDxodHRwczovL3dlYmtpdC5vcmcvYi8xNDQ3 MzE+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQWRk IGEgZmFzdCBwYXRoIGZvciBzZXJpYWxpemluZyBJbnQzMiB2YWx1ZXMgdG8gSlNPTi4gVGhpcyBp cyBmYXIgZmFzdGVyIHRoYW4gZHJhZ2dpbmcKKyAgICAgICAgc2ltcGxlIGludGVnZXJzIHRocm91 Z2ggdGhlIGZ1bGwtYmxvd24gZHRvYSgpIG1hY2hpbmVyeS4KKworICAgICAgICB+NTAlIHNwZWVk dXAgb24gS3Jha2VuL2pzb24tc3RyaW5naWZ5LXRpbmRlcmJveC4KKworICAgICAgICAqIHJ1bnRp bWUvSlNPTk9iamVjdC5jcHA6CisgICAgICAgIChKU0M6OlN0cmluZ2lmaWVyOjphcHBlbmRTdHJp bmdpZmllZFZhbHVlKToKKwogMjAxNS0wNS0wNiAgQW5kcmVhcyBLbGluZyAgPGFrbGluZ0BhcHBs ZS5jb20+CiAKICAgICAgICAgRG9uJ3QgYWxsb2NhdGUgYSBTdHJpbmdJbXBsIGZvciBldmVyeSBO dW1iZXIgSlNWYWx1ZSBpbiBKU09OLnN0cmluZ2lmeSgpLgpkaWZmIC0tZ2l0IGEvU291cmNlL0ph dmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNPTk9iamVjdC5jcHAgYi9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvcnVudGltZS9KU09OT2JqZWN0LmNwcAppbmRleCBhZmI2ZDE2Li41ODFiZDQ1IDEwMDY0NAot LS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU09OT2JqZWN0LmNwcAorKysgYi9T b3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU09OT2JqZWN0LmNwcApAQCAtMzkwLDExICsz OTAsMTUgQEAgU3RyaW5naWZpZXI6OlN0cmluZ2lmeVJlc3VsdCBTdHJpbmdpZmllcjo6YXBwZW5k U3RyaW5naWZpZWRWYWx1ZShTdHJpbmdCdWlsZGVyJgogICAgIH0KIAogICAgIGlmICh2YWx1ZS5p c051bWJlcigpKSB7Ci0gICAgICAgIGRvdWJsZSBudW1iZXIgPSB2YWx1ZS5hc051bWJlcigpOwot ICAgICAgICBpZiAoIXN0ZDo6aXNmaW5pdGUobnVtYmVyKSkKLSAgICAgICAgICAgIGJ1aWxkZXIu YXBwZW5kTGl0ZXJhbCgibnVsbCIpOwotICAgICAgICBlbHNlCi0gICAgICAgICAgICBidWlsZGVy LmFwcGVuZEVDTUFTY3JpcHROdW1iZXIobnVtYmVyKTsKKyAgICAgICAgaWYgKHZhbHVlLmlzSW50 MzIoKSkKKyAgICAgICAgICAgIGJ1aWxkZXIuYXBwZW5kTnVtYmVyKHZhbHVlLmFzSW50MzIoKSk7 CisgICAgICAgIGVsc2UgeworICAgICAgICAgICAgZG91YmxlIG51bWJlciA9IHZhbHVlLmFzTnVt YmVyKCk7CisgICAgICAgICAgICBpZiAoIXN0ZDo6aXNmaW5pdGUobnVtYmVyKSkKKyAgICAgICAg ICAgICAgICBidWlsZGVyLmFwcGVuZExpdGVyYWwoIm51bGwiKTsKKyAgICAgICAgICAgIGVsc2UK KyAgICAgICAgICAgICAgICBidWlsZGVyLmFwcGVuZEVDTUFTY3JpcHROdW1iZXIobnVtYmVyKTsK KyAgICAgICAgfQogICAgICAgICByZXR1cm4gU3RyaW5naWZ5U3VjY2VlZGVkOwogICAgIH0KIAo=