Bug 152203

Summary:

CachedScript could have a copy-free path for all-ASCII scripts.

Product:

WebKit

Reporter:

Andreas Kling <kling>

Component:

WebCore JavaScript

Assignee:

Andreas Kling <kling>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aestes, benjamin, cdumez, cmarcelo, commit-queue, japhet, keith_miller, kling, koivisto, mark.lam, msaboff, saam

Priority:

Keywords:

Performance

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch for EWS	none
Patch	none
Patch	koivisto: review+
Patch for landing	none

Description Andreas Kling 2015-12-11 16:52:27 PST

If a script resource consists of nothing but ASCII characters, we could have CachedScript::script() return a String that just wraps the internal SharedBuffer's contents instead of dragging the data through TextDecoder.

Comment 1 Andreas Kling 2015-12-12 13:58:02 PST

Created attachment 267242 [details]
Patch for EWS

Comment 2 Andreas Kling 2015-12-12 22:17:50 PST

Created attachment 267254 [details]
Patch

Comment 3 Andreas Kling 2015-12-13 01:26:51 PST

Created attachment 267256 [details]
Patch

Comment 4 Antti Koivisto 2015-12-13 01:45:01 PST

Comment on attachment 267256 [details]
Patch

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

Nice, r=me

> Source/WebCore/loader/cache/CachedScript.h:65
> +    enum ASCIIResourceOptimizationState { Unknown, Optimized, NotOptimized };
> +    ASCIIResourceOptimizationState m_ASCIIOptimizationState { Unknown };

Maybe this could be more explicit about what "Optimized" means? Something along the lines of DataAndDecodedStringHaveSameBytes.

Comment 5 Andreas Kling 2015-12-13 11:09:10 PST

Created attachment 267265 [details]
Patch for landing

Comment 6 WebKit Commit Bot 2015-12-13 12:03:49 PST

Comment on attachment 267265 [details]
Patch for landing

Clearing flags on attachment: 267265

Committed r194017: <http://trac.webkit.org/changeset/194017>

Comment 7 WebKit Commit Bot 2015-12-13 12:03:55 PST

All reviewed patches have been landed.  Closing bug.

Comment 8 Darin Adler 2015-12-13 13:31:17 PST

Comment on attachment 267256 [details]
Patch

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

> Source/WebCore/loader/cache/CachedScript.cpp:74
> +    return encoding == "UTF-8" || encoding == "ISO-8859-1" || encoding == "ASCII";

This seems to be way too conservative. Almost all encodings would answer yes for this. Pretty much everything except for UTF-16 and UTF-32, and I think TextEncoding can answer this question. It's strange to be asking this given just a string rather than TextEncoding.