Bug 81070 - Avoid StringImpl::getData16SlowCase() when sorting array
Summary: Avoid StringImpl::getData16SlowCase() when sorting array
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-13 19:19 PDT by Benjamin Poulain
Modified: 2012-03-14 22:11 PDT (History)
1 user (show)

See Also:


Attachments
Patch (4.59 KB, patch)
2012-03-13 19:27 PDT, Benjamin Poulain
ggaren: review+
ggaren: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Poulain 2012-03-13 19:19:30 PDT
When sorting a JSArray, one of the bottleneck is the conversion from StringImpl::getData16SlowCase()
Comment 1 Benjamin Poulain 2012-03-13 19:27:27 PDT
Created attachment 131773 [details]
Patch
Comment 2 Geoffrey Garen 2012-03-14 12:37:05 PDT
Comment on attachment 131773 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=131773&action=review

> Source/JavaScriptCore/wtf/text/StringImpl.h:782
> +    if (string1 && string2) {

It would be better to NULL check string1 and string2 at the head of the function, and return early if NULL, rather than NULL checking more than once in the body of the function, and indenting so much of the code. I think this would work:

if (!string1)
    return -1;
if (!string2)
    return string1->length();
....
Comment 3 Geoffrey Garen 2012-03-14 12:38:11 PDT
> if (!string2)
>     return string1->length();

Oops!

if (!string2)
    return string1->length() ? 1 : -1;
Comment 4 Benjamin Poulain 2012-03-14 12:44:24 PDT
> It would be better to NULL check string1 and string2 at the head of the function, and return early if NULL, rather than NULL checking more than once in the body of the function, and indenting so much of the code. I think this would work:

Good point! I'll update that

Thanks for the review.
Comment 5 Benjamin Poulain 2012-03-14 22:11:56 PDT
Committed r110822: <http://trac.webkit.org/changeset/110822>