Bug 125619

Summary: Improve the find word boundary performance
Product: WebKit Reporter: KyungTae Kim <ktf.kim>
Component: Layout and RenderingAssignee: KyungTae Kim <ktf.kim>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, buildbot, commit-queue, darin, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 126830    
Attachments:
Description Flags
testcase
none
Patch
none
Patch
none
Patch none

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.