Introduce v8-cache-strategies-for-cache-storage setting flags.

Introduce v8-cache-strategies-for-cache-storage setting flags.

This cl depends on https://codereview.chromium.org/1915833002/

- When the flag is "none", we don't store the V8 code cache in CacheStorage.
- When the flag is "normal", V8 code cache will be stored when the script is loaded twice in 72 hours. This behavior is same as V8 code cache in HTTPCache and implemented in V8ScriptRunner.
- When the flag is "aggressive", V8 code cache will be stored when the script is loaded for the first time.

We will monitor PageLoad.Clients.ServiceWorker.Timing2.NavigationToFirstContentfulPaint metrics (https://codereview.chromium.org/1871393002/) to decide which strategy is the best one. (If FirstMeaningfulPaint which is being developed by ksakamoto@ will be available, we will use it.)

BUG=581613
Committed: https://crrev.com/dbcbef409e1add7b7887d88b97ceadcf3d32fb20
Cr-Commit-Position: refs/heads/master@{#392629}

[horo, 2016-04-25 07:33:45]

Patchset #1 (id:1) has been deleted

[horo, 2016-04-25 09:22:37]

Description was changed from

==========
Introduce v8-cache-strategies-for-cache-storage setting flag and finch
variations.


BUG=581613
==========

to

==========
Introduce v8-cache-strategies-for-cache-storage setting flags and finch
variations.

This cl depends on https://codereview.chromium.org/1915833002/

BUG=581613
==========

[horo, 2016-04-25 09:22:53]

Description was changed from

==========
Introduce v8-cache-strategies-for-cache-storage setting flags and finch
variations.

This cl depends on https://codereview.chromium.org/1915833002/

BUG=581613
==========

to

==========
Introduce v8-cache-strategies-for-cache-storage setting flags and Finch
variations.

This cl depends on https://codereview.chromium.org/1915833002/

BUG=581613
==========

[horo, 2016-04-25 09:27:14]

Description was changed from

==========
Introduce v8-cache-strategies-for-cache-storage setting flags and Finch
variations.

This cl depends on https://codereview.chromium.org/1915833002/

BUG=581613
==========

to

==========
Introduce v8-cache-strategies-for-cache-storage setting flags and Finch
variations.

This cl depends on https://codereview.chromium.org/1915833002/

- When the flag is "none", we don't store the V8 code cache in CacheStorage.
- When the flag is "normal", V8 code cache will be stored when the script is
loaded twice in 72 hours. This behavior is same as V8 code cache in HTTPCache
and implemented in V8ScriptRunner.
- When the flag is "aggressive", V8 code cache will be stored when the script is
loaded for the first time.


BUG=581613
==========

[horo, 2016-04-25 09:29:06]

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

[horo, 2016-04-25 09:29:07]

falken@
Could you please review this?

[falken, 2016-04-26 15:29:52]

Sorry for the delay, this completely slipped my mind. The overall code approach
looks good, I'll look again more carefully tomorrow.

One question: I assume the Finch experiment is temporary, after which one of
"none" or "normal" or "aggressive" will be chosen. What metric will you look at
to decide which one is best? I looked over the design doc but didn't see it.

[horo, 2016-04-27 06:25:34]

On 2016/04/26 15:29:52, falken wrote:
> Sorry for the delay, this completely slipped my mind. The overall code
approach
> looks good, I'll look again more carefully tomorrow.
> 
> One question: I assume the Finch experiment is temporary, after which one of
> "none" or "normal" or "aggressive" will be chosen. What metric will you look
at
> to decide which one is best? I looked over the design doc but didn't see it.

We will monitor
PageLoad.Clients.ServiceWorker.Timing2.NavigationToFirstContentfulPaint metrics
(https://codereview.chromium.org/1871393002/) to decide which strategy is the
best one. (If FirstMeaningfulPaint which is being developed by ksakamoto@ will
be available, we will use it.)

[falken, 2016-04-27 08:07:38]

I think this lgtm. Thanks for adding the info about the metric in the doc. Maybe
add it to the CL description too?

https://codereview.chromium.org/1914963002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/ScriptLoader.cpp (right):

https://codereview.chromium.org/1914963002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/ScriptLoader.cpp:328: }
no braces to be consistent with rest of file

https://codereview.chromium.org/1914963002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/Settings.in (right):

https://codereview.chromium.org/1914963002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/Settings.in:311: # V8 code cache for
CacheStorage supports three type strategies(none, normal and aggressive).
nit: "three types of strategies", space before "(none)"

[horo, 2016-04-27 08:14:33]

Description was changed from

==========
Introduce v8-cache-strategies-for-cache-storage setting flags and Finch
variations.

This cl depends on https://codereview.chromium.org/1915833002/

- When the flag is "none", we don't store the V8 code cache in CacheStorage.
- When the flag is "normal", V8 code cache will be stored when the script is
loaded twice in 72 hours. This behavior is same as V8 code cache in HTTPCache
and implemented in V8ScriptRunner.
- When the flag is "aggressive", V8 code cache will be stored when the script is
loaded for the first time.


BUG=581613
==========

to

==========
Introduce v8-cache-strategies-for-cache-storage setting flags and Finch
variations.

This cl depends on https://codereview.chromium.org/1915833002/

- When the flag is "none", we don't store the V8 code cache in CacheStorage.
- When the flag is "normal", V8 code cache will be stored when the script is
loaded twice in 72 hours. This behavior is same as V8 code cache in HTTPCache
and implemented in V8ScriptRunner.
- When the flag is "aggressive", V8 code cache will be stored when the script is
loaded for the first time.

We will monitor
PageLoad.Clients.ServiceWorker.Timing2.NavigationToFirstContentfulPaint metrics
(https://codereview.chromium.org/1871393002/) to decide which strategy is the
best one. (If FirstMeaningfulPaint which is being developed by ksakamoto@ will
be available, we will use it.)


BUG=581613
==========

[horo, 2016-04-27 08:15:36]

Updated the CL description.
Thank you.

https://codereview.chromium.org/1914963002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/ScriptLoader.cpp (right):

https://codereview.chromium.org/1914963002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/ScriptLoader.cpp:328: }
On 2016/04/27 08:07:37, falken wrote:
> no braces to be consistent with rest of file

Done.

https://codereview.chromium.org/1914963002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/Settings.in (right):

https://codereview.chromium.org/1914963002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/Settings.in:311: # V8 code cache for
CacheStorage supports three type strategies(none, normal and aggressive).
On 2016/04/27 08:07:37, falken wrote:
> nit: "three types of strategies", space before "(none)"

Done.

[vogelheim, 2016-04-27 18:22:21]

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

[vogelheim, 2016-04-27 18:22:22]

drive-by comment:

Your implementation of v8-cache-strategies-for-cache-storage follows that of
v8-cache-options, which employs an awful lot of plumbing in order to obtain the
command-line and experiment flag in the browser process, and then pass them to
the renderer process, and then to V8.

But: Meanwhile the infrastructure for this has been much improved, and now the
feature flags and command-line flags are automatically available in the
renderer. So today, you could accomplish the same thing in a simpler fashion.
crrev.com/1864063002 is an example of how to pass flags from command-line &
features to V8. (It doesn't quite fit your use case, but I think it's pretty
close.)

I'm not sure if it's worth the effort to re-write this, though.. :-/

[vogelheim, 2016-04-27 18:22:39]

vogelheim@chromium.org changed reviewers:
- vogelheim@chromium.org

[horo, 2016-04-30 05:36:58]

On 2016/04/27 18:22:22, vogelheim wrote:
> drive-by comment:
> 
> Your implementation of v8-cache-strategies-for-cache-storage follows that of
> v8-cache-options, which employs an awful lot of plumbing in order to obtain
the
> command-line and experiment flag in the browser process, and then pass them to
> the renderer process, and then to V8.
> 
> But: Meanwhile the infrastructure for this has been much improved, and now the
> feature flags and command-line flags are automatically available in the
> renderer. So today, you could accomplish the same thing in a simpler fashion.
> crrev.com/1864063002 is an example of how to pass flags from command-line &
> features to V8. (It doesn't quite fit your use case, but I think it's pretty
> close.)
> 
> I'm not sure if it's worth the effort to re-write this, though.. :-/

vogelheim@
Thank you for the comment.
I uploaded Patch Set 3 which checks the flags in the renderer process.

[horo, 2016-05-09 04:10:02]

Patchset #4 (id:80001) has been deleted

[horo, 2016-05-09 04:13:47]

horo@chromium.org changed reviewers:
+ jochen@chromium.org

[horo, 2016-05-09 04:13:47]

jochen@

Could you please review content/* and third_party/WebKit/*?

[jochen (gone - plz use gerrit), 2016-05-09 15:26:28]

lgtm

[horo, 2016-05-09 22:25:33]

horo@chromium.org changed reviewers:
+ asvitkine@chromium.org

[horo, 2016-05-09 22:25:34]

asvitkine@
Could you please review testing/variations/* and
tools/metrics/histograms/histograms.xml?

[Alexei Svitkine (slow), 2016-05-10 15:24:44]

lgtm

please modify CL description to not use the term Finch since that's an internal
codename

[horo, 2016-05-10 15:27:52]

Description was changed from

==========
Introduce v8-cache-strategies-for-cache-storage setting flags and Finch
variations.

This cl depends on https://codereview.chromium.org/1915833002/

- When the flag is "none", we don't store the V8 code cache in CacheStorage.
- When the flag is "normal", V8 code cache will be stored when the script is
loaded twice in 72 hours. This behavior is same as V8 code cache in HTTPCache
and implemented in V8ScriptRunner.
- When the flag is "aggressive", V8 code cache will be stored when the script is
loaded for the first time.

We will monitor
PageLoad.Clients.ServiceWorker.Timing2.NavigationToFirstContentfulPaint metrics
(https://codereview.chromium.org/1871393002/) to decide which strategy is the
best one. (If FirstMeaningfulPaint which is being developed by ksakamoto@ will
be available, we will use it.)


BUG=581613
==========

to

==========
Introduce v8-cache-strategies-for-cache-storage setting flags.

This cl depends on https://codereview.chromium.org/1915833002/

- When the flag is "none", we don't store the V8 code cache in CacheStorage.
- When the flag is "normal", V8 code cache will be stored when the script is
loaded twice in 72 hours. This behavior is same as V8 code cache in HTTPCache
and implemented in V8ScriptRunner.
- When the flag is "aggressive", V8 code cache will be stored when the script is
loaded for the first time.

We will monitor
PageLoad.Clients.ServiceWorker.Timing2.NavigationToFirstContentfulPaint metrics
(https://codereview.chromium.org/1871393002/) to decide which strategy is the
best one. (If FirstMeaningfulPaint which is being developed by ksakamoto@ will
be available, we will use it.)


BUG=581613
==========

[horo, 2016-05-10 15:28:36]

Thank you.

> please modify CL description to not use the term Finch since that's an
internal
> codename

Done.

[horo, 2016-05-10 15:28:43]

The CQ bit was checked by horo@chromium.org

[horo, 2016-05-10 15:28:44]

The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org
Link to the patchset: https://codereview.chromium.org/1914963002/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2016-05-10 15:29:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1914963002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1914963002/100001

[commit-bot: I haz the power, 2016-05-10 17:03:03]

Description was changed from

==========
Introduce v8-cache-strategies-for-cache-storage setting flags.

This cl depends on https://codereview.chromium.org/1915833002/

- When the flag is "none", we don't store the V8 code cache in CacheStorage.
- When the flag is "normal", V8 code cache will be stored when the script is
loaded twice in 72 hours. This behavior is same as V8 code cache in HTTPCache
and implemented in V8ScriptRunner.
- When the flag is "aggressive", V8 code cache will be stored when the script is
loaded for the first time.

We will monitor
PageLoad.Clients.ServiceWorker.Timing2.NavigationToFirstContentfulPaint metrics
(https://codereview.chromium.org/1871393002/) to decide which strategy is the
best one. (If FirstMeaningfulPaint which is being developed by ksakamoto@ will
be available, we will use it.)


BUG=581613
==========

to

==========
Introduce v8-cache-strategies-for-cache-storage setting flags.

This cl depends on https://codereview.chromium.org/1915833002/

- When the flag is "none", we don't store the V8 code cache in CacheStorage.
- When the flag is "normal", V8 code cache will be stored when the script is
loaded twice in 72 hours. This behavior is same as V8 code cache in HTTPCache
and implemented in V8ScriptRunner.
- When the flag is "aggressive", V8 code cache will be stored when the script is
loaded for the first time.

We will monitor
PageLoad.Clients.ServiceWorker.Timing2.NavigationToFirstContentfulPaint metrics
(https://codereview.chromium.org/1871393002/) to decide which strategy is the
best one. (If FirstMeaningfulPaint which is being developed by ksakamoto@ will
be available, we will use it.)


BUG=581613
==========

[commit-bot: I haz the power, 2016-05-10 17:03:05]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-05-10 17:05:27]

Description was changed from

==========
Introduce v8-cache-strategies-for-cache-storage setting flags.

This cl depends on https://codereview.chromium.org/1915833002/

- When the flag is "none", we don't store the V8 code cache in CacheStorage.
- When the flag is "normal", V8 code cache will be stored when the script is
loaded twice in 72 hours. This behavior is same as V8 code cache in HTTPCache
and implemented in V8ScriptRunner.
- When the flag is "aggressive", V8 code cache will be stored when the script is
loaded for the first time.

We will monitor
PageLoad.Clients.ServiceWorker.Timing2.NavigationToFirstContentfulPaint metrics
(https://codereview.chromium.org/1871393002/) to decide which strategy is the
best one. (If FirstMeaningfulPaint which is being developed by ksakamoto@ will
be available, we will use it.)


BUG=581613
==========

to

==========
Introduce v8-cache-strategies-for-cache-storage setting flags.

This cl depends on https://codereview.chromium.org/1915833002/

- When the flag is "none", we don't store the V8 code cache in CacheStorage.
- When the flag is "normal", V8 code cache will be stored when the script is
loaded twice in 72 hours. This behavior is same as V8 code cache in HTTPCache
and implemented in V8ScriptRunner.
- When the flag is "aggressive", V8 code cache will be stored when the script is
loaded for the first time.

We will monitor
PageLoad.Clients.ServiceWorker.Timing2.NavigationToFirstContentfulPaint metrics
(https://codereview.chromium.org/1871393002/) to decide which strategy is the
best one. (If FirstMeaningfulPaint which is being developed by ksakamoto@ will
be available, we will use it.)

BUG=581613

Committed: https://crrev.com/dbcbef409e1add7b7887d88b97ceadcf3d32fb20
Cr-Commit-Position: refs/heads/master@{#392629}
==========

[commit-bot: I haz the power, 2016-05-10 17:05:29]

Patchset 4 (id:??) landed as
https://crrev.com/dbcbef409e1add7b7887d88b97ceadcf3d32fb20
Cr-Commit-Position: refs/heads/master@{#392629}