Bug 125619 - Improve the find word boundary performance
Summary: Improve the find word boundary performance
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: KyungTae Kim
URL:
Keywords:
Depends on:
Blocks: 126830
  Show dependency treegraph
 
Reported: 2013-12-12 00:20 PST by KyungTae Kim
Modified: 2014-01-11 19:58 PST (History)
5 users (show)

See Also:


Attachments
testcase (110.11 KB, text/html)
2013-12-12 00:20 PST, KyungTae Kim
no flags Details
Patch (3.10 KB, patch)
2013-12-12 00:33 PST, KyungTae Kim
no flags Details | Formatted Diff | Diff
Patch (3.24 KB, patch)
2013-12-12 01:20 PST, KyungTae Kim
no flags Details | Formatted Diff | Diff
Patch (4.33 KB, patch)
2013-12-12 16:18 PST, KyungTae Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description KyungTae Kim 2013-12-12 00:20:17 PST
Created attachment 219051 [details]
testcase

Currently, the finding end word boundary has poor performance when the word is very wrong.

If you double click on the texts in the attached file, that takes several seconds to select the word.

Both startWordBoudary and endWordBoundary uses findWordBoundary that returns both start and end boundary, then use only start or end boundary.

By using the separate function for finding end word boundary, the word selection performance can be improved by up to 50%.
Comment 1 KyungTae Kim 2013-12-12 00:33:30 PST
Created attachment 219052 [details]
Patch
Comment 2 Build Bot 2013-12-12 01:03:32 PST
Comment on attachment 219052 [details]
Patch

Attachment 219052 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/48138217
Comment 3 KyungTae Kim 2013-12-12 01:20:25 PST
Created attachment 219057 [details]
Patch
Comment 4 Sam Weinig 2013-12-12 10:41:25 PST
Comment on attachment 219057 [details]
Patch

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

> Source/WebCore/editing/VisibleUnits.cpp:653
> +#if PLATFORM(MAC)

Please don't put platform specific changes in this platform independent file.
Comment 5 Darin Adler 2013-12-12 10:48:15 PST
It’s OK to do this by adding the new function. We should add a Mac implementation too, even if there is no performance benefit on Mac.
Comment 6 KyungTae Kim 2013-12-12 16:18:44 PST
Created attachment 219133 [details]
Patch
Comment 7 Darin Adler 2013-12-12 17:32:31 PST
Comment on attachment 219133 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        No tests because no operation changes.

Normally we are able to build performance tests that show benefits in cases like this.

> Source/WebCore/platform/text/mac/TextBoundaries.mm:52
> +void findEndWordBoundary(const UChar* chars, int len, int position, int* end)
> +{
> +    NSString* string = [[NSString alloc] initWithCharactersNoCopy:const_cast<unichar*>(chars)
> +        length:len freeWhenDone:NO];
> +    NSAttributedString* attr = [[NSAttributedString alloc] initWithString:string];
> +    NSRange range = [attr doubleClickAtIndex:(position >= len) ? len - 1 : position];
> +    [attr release];
> +    [string release];
> +    *end = range.location + range.length;
> +}

Looks fine. Or we could just have this call findWordBoundary.
Comment 8 WebKit Commit Bot 2013-12-12 20:56:38 PST
Comment on attachment 219133 [details]
Patch

Clearing flags on attachment: 219133

Committed r160526: <http://trac.webkit.org/changeset/160526>
Comment 9 WebKit Commit Bot 2013-12-12 20:56:41 PST
All reviewed patches have been landed.  Closing bug.