Bug 73584 - Changes proposed for 73457 slow down Kraken json-parse-financial
: Changes proposed for 73457 slow down Kraken json-parse-financial
Status: RESOLVED FIXED
: WebKit
JavaScriptCore
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
: 73236
  Show dependency treegraph
 
Reported: 2011-12-01 13:11 PST by
Modified: 2011-12-05 15:20 PST (History)


Attachments
Patch (4.97 KB, patch)
2011-12-01 13:25 PST, Michael Saboff
sam: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-12-01 13:11:43 PST
The changes proposed for bug 73457 slow down Kraken json-parse-financial by about 3%.
------- Comment #1 From 2011-12-01 13:25:27 PST -------
Created an attachment (id=117475) [details]
Patch
------- Comment #2 From 2011-12-01 15:07:26 PST -------
(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.
------- Comment #3 From 2011-12-01 19:44:16 PST -------
(From update of attachment 117475 [details])
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 &&  *
------- Comment #4 From 2011-12-01 22:35:08 PST -------
(In reply to comment #2)
> (From update of attachment 117475 [details] [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.
------- Comment #5 From 2011-12-01 23:06:56 PST -------
Committed r101747: <http://trac.webkit.org/changeset/101747>