Bug 73584

Summary: Changes proposed for 73457 slow down Kraken json-parse-financial
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 73236    
Attachments:
Description Flags
Patch sam: review+

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