Bug 73584 – Changes proposed for 73457 slow down Kraken json-parse-financial

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

[Michael Saboff]

The changes proposed for bug 73457 slow down Kraken json-parse-financial by about 3%.

[Michael Saboff]

Created attachment 117475 [details]
Patch

[Darin Adler]

Comment on attachment 117475 [details]
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.

[Sam Weinig]

Comment on attachment 117475 [details]
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 &&  *

[Michael Saboff]

(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.

[Michael Saboff]

Committed r101747: <http://trac.webkit.org/changeset/101747>