Do not cache resource metadata if the source code size is small.

Do not cache resource metadata if the source code size is small.

For small source code, parsing/compiling may be faster than
deserializing.

R=jochen@chromium.org, vogelheim@chromium.org
BUG=chromium:399580
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=191554

[vogelheim, 2015-03-09 14:30:45]

lgtm

[jochen (gone - plz use gerrit), 2015-03-09 14:31:26]

lgtm

[Yang, 2015-03-09 14:37:45]

The CQ bit was checked by yangguo@chromium.org

[commit-bot: I haz the power, 2015-03-09 14:38:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/988303003/1

[commit-bot: I haz the power, 2015-03-09 16:03:52]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=191554

[jbroman, 2015-03-09 19:59:55]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/992003003/ by jbroman@chromium.org.

The reason for reverting is: Causes
content_browsertests:ServiceWorkerVersionBrowserV8CacheTest.Restart to time out.

This is blocking the Blink roll: https://codereview.chromium.org/986373002/.

[Yang, 2015-03-09 21:22:09]

On 2015/03/09 19:59:55, jbroman wrote:
> A revert of this CL (patchset #1 id:1) has been created in
> https://codereview.chromium.org/992003003/ by mailto:jbroman@chromium.org.
> 
> The reason for reverting is: Causes
> content_browsertests:ServiceWorkerVersionBrowserV8CacheTest.Restart to time
out.
> 
> This is blocking the Blink roll: https://codereview.chromium.org/986373002/.

The reason is that the failing test runs a small ServiceWorker script and waits
for it to be cached. With this change, it never happens.

horo@, falken@, do you have a suggestion how to fix this? We could make the
script size larger by inserting comments, but that does not scale if we decide
to increase the size limit. Alternatively, I could change this CL to ignore the
limit for the "code" option, but honor it for other options. WDYT?

[Yang, 2015-03-09 21:22:48]

The reason is that the failing test runs a small ServiceWorker script and waits
for it to be cached. With this change, it never happens.

horo@, falken@, do you have a suggestion how to fix this? We could make the
script size larger by inserting comments, but that does not scale if we decide
to increase the size limit. Alternatively, I could change this CL to ignore the
limit for the "code" option, but honor it for other options. WDYT?

[falken, 2015-03-10 00:54:04]

falken@chromium.org changed reviewers:
+ falken@chromium.org

[falken, 2015-03-10 00:54:05]

You and horo@ know more than me about this.. but I feel it's better to make a
sw.js padded with comments instead of changing the CL behavior to pass this
test. If the script size increases the test will fail and we can re-update... if
it ever increases to a huge amount (1MB?) we might look at other options like
dynamically generating the JS.

https://codereview.chromium.org/988303003/diff/1/Source/bindings/core/v8/V8Sc...
File Source/bindings/core/v8/V8ScriptRunner.cpp (right):

https://codereview.chromium.org/988303003/diff/1/Source/bindings/core/v8/V8Sc...
Source/bindings/core/v8/V8ScriptRunner.cpp:283: // Do not cache for small
scripts, even if caching is available.
This comment kind of just repeats the code. It'd be nicer to mention the reason
as in the patch description: "For small source code, parsing/compiling may be
faster than deserializing."

[horo, 2015-03-10 01:12:40]

On 2015/03/09 21:22:48, Yang wrote:
> The reason is that the failing test runs a small ServiceWorker script and
waits
> for it to be cached. With this change, it never happens.
> 
> horo@, falken@, do you have a suggestion how to fix this? We could make the
> script size larger by inserting comments, but that does not scale if we decide
> to increase the size limit. Alternatively, I could change this CL to ignore
the
> limit for the "code" option, but honor it for other options. WDYT?

How about adding "force-cache" mode?
Isn't it acceptable?