Bug 116933

Summary:

getData16SlowCase shows up on Mobile Gmail profile

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

Page Loading

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED WONTFIX

Severity:

Normal

CC:

abarth, barraclough, benjamin, darin, ggaren, koivisto, msaboff, psolanki

Priority:

Keywords:

BlinkMergeCandidate

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Work in progress	none
A/B test results of variants of the patch	none

Ryosuke Niwa

Reported 2013-05-29 01:07:15 PDT

Consider merging https://chromium.googlesource.com/chromium/blink/+/0d0e52b5e2d15baadb7703028e83e6736751830b When loading Mobile Gmail in content_shell, StringImpl::getData16SlowCase shows up as 0.35% of the total profile. We're hitting this code path because the HTML parser causes *all* character tokens to (temporarily) have 16 string buffers because ExternalCharacterTokenBuffer operates in terms of UChar*, which assumes a 16 bit string buffer. This CL changes CharacterTokenBuffer to operate in terms of offsets into Strings, which is agnostic as to the backing buffer for the String. As a result, we can often just use the HTMLTokenizer-generated String in the DOM directly. After this CL, getData16SlowCase doesn't show up on the profile at all.

Attachments
Work in progress (10.69 KB, patch) 2013-05-29 21:51 PDT, Ryosuke Niwa	no flags	Details Formatted Diff Diff
A/B test results of variants of the patch (39.22 KB, text/html) 2013-05-29 21:51 PDT, Ryosuke Niwa	no flags	Details
View All Add attachment proposed patch, testcase, etc.

Darin Adler

Comment 1 2013-05-29 12:27:00 PDT

Sounds like a really good optimization. We should get that one in.

Adam Barth

Comment 2 2013-05-29 18:38:02 PDT

It's less of a big deal for the main thread parser, but probably worth merging in any case.

Ryosuke Niwa

Comment 3 2013-05-29 21:51:19 PDT

Created attachment 203308 [details] Work in progress

Ryosuke Niwa

Comment 4 2013-05-29 21:51:58 PDT

Created attachment 203309 [details] A/B test results of variants of the patch

Ryosuke Niwa

Comment 5 2013-05-29 21:53:57 PDT

I've made many variants of this patch but all of them had a net negative impact on the performance at least on html-parser.html.

Adam Barth

Comment 6 2013-05-29 22:56:10 PDT

Did you test it with the threaded parser?

Ryosuke Niwa

Comment 7 2013-05-29 22:57:39 PDT

(In reply to comment #6) > Did you test it with the threaded parser? No. It's conceivable that this is a performance gain with the threaded parser.

Note You need to log in before you can comment on or make changes to this bug.