Bug 144731 – Special-case Int32 values in JSON.stringify().

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Andreas Kling]

We should have a fast-path in JSON.stringify() for Int32 values.

[Andreas Kling]

Created attachment 252580 [details]
Patch

Boom:

json-stringify-tinderbox        60.176+-8.602      ^      39.277+-1.632         ^ definitely 1.5321x faster

[Michael Saboff]

Comment on attachment 252580 [details]
Patch

r=me

[Andreas Kling]

Comment on attachment 252580 [details]
Patch

Clearing flags on attachment: 252580

Committed r183928: <http://trac.webkit.org/changeset/183928>

[Andreas Kling]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 252580 [details]
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;
    }

[Geoffrey Garen]

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'

[Geoffrey Garen]

Oops! Wrong bug :(.