Bug 202676 - [JSC] Add fast path for String#localeCompare
Summary: [JSC] Add fast path for String#localeCompare
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-07 19:35 PDT by Yusuke Suzuki
Modified: 2019-10-29 14:43 PDT (History)
11 users (show)

See Also:


Attachments
Patch (3.84 KB, patch)
2019-10-29 01:18 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (4.88 KB, patch)
2019-10-29 01:42 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (4.87 KB, patch)
2019-10-29 01:43 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (6.41 KB, patch)
2019-10-29 13:24 PDT, Yusuke Suzuki
mark.lam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2019-10-07 19:35:11 PDT
...
Comment 1 Yusuke Suzuki 2019-10-29 01:12:51 PDT
I've tried small improvement and gets significantly better result: if string is all-ascii, use ucol_strcollUTF8.
Comment 2 Yusuke Suzuki 2019-10-29 01:18:33 PDT
Created attachment 382164 [details]
Patch
Comment 3 Yusuke Suzuki 2019-10-29 01:42:12 PDT
Created attachment 382165 [details]
Patch
Comment 4 Yusuke Suzuki 2019-10-29 01:43:24 PDT
Created attachment 382166 [details]
Patch
Comment 5 Yusuke Suzuki 2019-10-29 13:24:03 PDT
Created attachment 382215 [details]
Patch
Comment 6 Mark Lam 2019-10-29 13:36:21 PDT
Comment on attachment 382215 [details]
Patch

r=me.  Do we already have test cases for 8bit vs 8bit, 16 bit vs 16bit, 8bit vs 16bit, and 16bit vs 8bit localeCompare?  If not, can you add this test to make sure there's no bug introduced here?  Also an API test to verify that StringView::isAllASCII() is working would also be nice.  Thanks.
Comment 7 Yusuke Suzuki 2019-10-29 13:40:13 PDT
(In reply to Mark Lam from comment #6)
> Comment on attachment 382215 [details]
> Patch
> 
> r=me.  Do we already have test cases for 8bit vs 8bit, 16 bit vs 16bit, 8bit
> vs 16bit, and 16bit vs 8bit localeCompare?  If not, can you add this test to
> make sure there's no bug introduced here?  Also an API test to verify that
> StringView::isAllASCII() is working would also be nice.  Thanks.

Oops, I forgot uploading the test.
Comment 8 Radar WebKit Bug Importer 2019-10-29 14:14:53 PDT
<rdar://problem/56720843>
Comment 9 Yusuke Suzuki 2019-10-29 14:43:38 PDT
Committed r251736: <https://trac.webkit.org/changeset/251736>