Not sure why you're assigning this one to me.

I didn't have any context for this bug, and figured it has been assigned to me by mistake.

What is the "the network service" ?

I can't explain why yiningc's assigned the bug to you, but the network service involves running network code out of process, as a mojo service.  An API that allows creating two NetworkContexts (The new top-level network object, used by consumers in the browser process) that share everything but a cache just seems much too ugly, and the dual cache decision was pretty hacky in the first place.

Not sure how that makes any difference to media though, I mean network stuff is always handled by a different process, it just used to be the browser. So what is this "dual cache decision", and who made it? (can we assign the bug to them?)

Extremely unlikely - it's been this way for quite a while.  Looks like it was done by hclam, back in 2009?  http://codereview.chromium.org/19747.  I suspect he was on the media team, but I'm not positive of that.  He was not on the network team.

Regardless, I'm not sure why we need to assign the bug at this point?

Yes he was long ago on media team; I'd support any experiment turning these off and watching impact on our metrics.

I agree with Dale.

+morlovich@: Do you know if there's anything special with net::MEDIA_CACHE? 

I did a code search, and net::MEDIA_CACHE cache type doesn't seem to do any optimization. The special handling code (added in https://codereview.chromium.org/19747/) seem to be gone.

We could start by removing net::MEDIA_CACHE cache type first (before running a 
field trial to combine media URLRequestContexts with main URLRequestContexts). 

I'm pretty net::MEDIA_CACHE is just for histograms, though defer to Maks on anything cache-related.

The big thing is the two separate directories, so video files don't end up kicking out everything else.

As for other effects:
The blockfile cache uses totally different eviction algorithm for the main cache than everything else, with media cache not included. Not sure if that's intentional.

Also looks like optimistic ops for simple are off for media cache which... is pretty much a bug. Woops.

(Those both check for net::DISK_CACHE rather than net::MEDIA_CACHE).

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3f0d18e0cfc5c485c743631c8d55737e559e7cd4

commit 3f0d18e0cfc5c485c743631c8d55737e559e7cd4
Author: Matt Menke <mmenke@chromium.org>
Date: Thu May 17 22:33:51 2018

Add an experimental "feature" to disable the media cache.

When enabled, a Profile's media URLRequestContext will just use the
same disk cache as the main one. Also affect App Media
URLRequestContexts.

Bug: 789657
Change-Id: I4150324ad84c1679495bd512dd0231edb30c733b
Reviewed-on: https://chromium-review.googlesource.com/1064858
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559712}
[modify] https://crrev.com/3f0d18e0cfc5c485c743631c8d55737e559e7cd4/chrome/browser/profiles/profile_browsertest.cc
[modify] https://crrev.com/3f0d18e0cfc5c485c743631c8d55737e559e7cd4/chrome/browser/profiles/profile_impl_io_data.cc
[modify] https://crrev.com/3f0d18e0cfc5c485c743631c8d55737e559e7cd4/chrome/common/chrome_features.cc
[modify] https://crrev.com/3f0d18e0cfc5c485c743631c8d55737e559e7cd4/chrome/common/chrome_features.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b0f4c7ce78b25c620cd3bd3253520cdabcb63ee9

commit b0f4c7ce78b25c620cd3bd3253520cdabcb63ee9
Author: Matt Menke <mmenke@chromium.org>
Date: Fri Jul 06 19:25:30 2018

Add a "feature" to scale the disk cache size.

This is for use with the same disk cache for media experiment.

Bug: 789657
Change-Id: I3374166de4811df85b6c38e8988fee42a609f96f
Reviewed-on: https://chromium-review.googlesource.com/1124764
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573040}
[modify] https://crrev.com/b0f4c7ce78b25c620cd3bd3253520cdabcb63ee9/net/disk_cache/cache_util.cc
[modify] https://crrev.com/b0f4c7ce78b25c620cd3bd3253520cdabcb63ee9/net/disk_cache/cache_util.h
[modify] https://crrev.com/b0f4c7ce78b25c620cd3bd3253520cdabcb63ee9/net/disk_cache/cache_util_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e226149af1b3f003b5d3f59e19ee17bef7419c42

commit e226149af1b3f003b5d3f59e19ee17bef7419c42
Author: Matt Menke <mmenke@chromium.org>
Date: Wed Jul 11 20:05:30 2018

Delete the media cache(s) when using a single combined HTTP cache.

This is so that I can try increasing the media cache size in experiments
without increasing Chrome's profile size (Except possibly for profiles
that have never been used to play video).

Bug: 789657
Change-Id: I45e8f8c6e346a1fa7d05b0bea3e8b001d7dc41e1
Reviewed-on: https://chromium-review.googlesource.com/1130162
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574296}
[modify] https://crrev.com/e226149af1b3f003b5d3f59e19ee17bef7419c42/chrome/browser/profiles/profile_browsertest.cc
[modify] https://crrev.com/e226149af1b3f003b5d3f59e19ee17bef7419c42/chrome/browser/profiles/profile_impl_io_data.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb81f73d09c9efd4ff7eb181ce3ffe105d1549a8

commit eb81f73d09c9efd4ff7eb181ce3ffe105d1549a8
Author: Mike West <mkwst@chromium.org>
Date: Thu Jul 12 09:35:30 2018

Revert "Delete the media cache(s) when using a single combined HTTP cache."

This reverts commit e226149af1b3f003b5d3f59e19ee17bef7419c42.

Reason for revert: 

`ProfileWithoutMediaCacheBrowserTest.DeleteMediaCache` and `ProfileWithoutMediaCacheBrowserTest.DeleteIsolatedAppMediaCache` have been failing, apparently since they were added by this CL.

They were added here: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.11%20Tests/28016 (which failed to actually run `browser_test`) and have been failing consistently since https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.11%20Tests/28018.


Original change's description:
> Delete the media cache(s) when using a single combined HTTP cache.
> 
> This is so that I can try increasing the media cache size in experiments
> without increasing Chrome's profile size (Except possibly for profiles
> that have never been used to play video).
> 
> Bug: 789657
> Change-Id: I45e8f8c6e346a1fa7d05b0bea3e8b001d7dc41e1
> Reviewed-on: https://chromium-review.googlesource.com/1130162
> Reviewed-by: Maks Orlovich <morlovich@chromium.org>
> Commit-Queue: Matt Menke <mmenke@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#574296}

TBR=mmenke@chromium.org,morlovich@chromium.org

Change-Id: Ibc519750f00ad7e698d279af6baffe77b32d30be
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 789657,  862945 
Reviewed-on: https://chromium-review.googlesource.com/1134886
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574513}
[modify] https://crrev.com/eb81f73d09c9efd4ff7eb181ce3ffe105d1549a8/chrome/browser/profiles/profile_browsertest.cc
[modify] https://crrev.com/eb81f73d09c9efd4ff7eb181ce3ffe105d1549a8/chrome/browser/profiles/profile_impl_io_data.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/685baf97d7a8e6c43e2f96f43f8bf5b7f78cb586

commit 685baf97d7a8e6c43e2f96f43f8bf5b7f78cb586
Author: Matt Menke <mmenke@chromium.org>
Date: Tue Jul 24 17:27:58 2018

Delete the media cache(s) when using a single combined HTTP cache.

This is so that I can try increasing the media cache size in experiments
without increasing Chrome's profile size (Except possibly for profiles
that have never been used to play video).

This is a relant of
https://chromium-review.googlesource.com/c/chromium/src/+/1130162, with
re-worked tests.

Bug: 789657
Change-Id: I057e2e657572e21732610aefc468550e58fabf77
Reviewed-on: https://chromium-review.googlesource.com/1145137
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577595}
[modify] https://crrev.com/685baf97d7a8e6c43e2f96f43f8bf5b7f78cb586/chrome/browser/profiles/profile_browsertest.cc
[modify] https://crrev.com/685baf97d7a8e6c43e2f96f43f8bf5b7f78cb586/chrome/browser/profiles/profile_impl_io_data.cc

matt, has this been resolved? let me know what's remaining, thanks!

No, it has not - need to launch a new field trial, and see how much of a difference a bigger cache with a unified cache makes with perf.  Was going to do it this week, but everyone's out, and am sufficiently paranoid about dealing with multiple features in a single field trial that I decided not to launch it while Maks is out.

I think we can probably go to Canary without this, though, so removing the Canary label, though it will make perf comparisons less meaningful.

Hey Matt, can you provide an update on this? Has nothing happened in 2 months or is there stuff that's not captured here? Thanks

I ran the trial for a month and a half or so with legacy behavior/merged cache/merged cache of 1.5x size/merged cache of 2x size.

Results were *extremely* noisy, to the extent that I don't have a whole lot of faith in even a months worth of aggregated data.  The network TimeToFirstByte and TotalTime (for request) histograms pretty clearly regressed a bit for all 3 experimental groups.  The cache hit rates seem to either barely improve or be about the same.  NavigationToFirstContentFul paint may have just slightly improved in some cases, worse in others...Honestly, given inconsistencies between groups (There's a bias of whose in a network service enabled vs no network service group vs network service enabled, which really isn't compatible with looking at a month's worth of data from this experiment), there's not a whole lot more I can say.

Also, as I said, the data is really, really noisy.  Data from one week to the next makes it difficult to figure out if there's a trend, other than network times definitely regression slightly, in aggregate.

FYI you can exclude users in network service trial in finch, e.g. https://uma.googleplex.com/p/chrome/variations?sid=79a8f62ae910ab261175561d917fa725

However it's still noisy. Generally perf numbers are only trusted in stable, did you consider running this experiment on a subset of stable users?

The requisite code isn't on stable, and don't stable trials require a lot of extra overhead (launch bug, design doc, etc?)

Oh, and as I said - there are significant differences between "Control" and "Not in trial" numbers for the NetworkService trial, which may be noise...or may be population biases.

It seems that only the last check-in (deleting the media cache) isn't in 69. If I'm reading Quota.AvailableDiskSpace correctly, the median Windows user has > 167GB free so it seems fine that we would use more profile space for users in this experiment.

Yes, stable experiments have a bunch of requirements per https://docs.google.com/document/d/1hJ1U8-7DNa7lGfTJWRgSgqQyNnOFO4Ks5Czr1-3--8I/edit#heading=h.ja0j8vjwrlfs. At the same time, afaik this is the only trusted channel for performance comparisons because the others users' are considered too biased.

Given that we don't have conclusive data so far for the media cache experiments, and we will start m70 stable trials next week for network-service at low percentages, the no-separate-media-cache can be considered as one of the (many) internal behavioral changes of network service. This does mean we don't have insight into what portion of performance difference, if any, are related to the combining of caches vs other changes.

Unassigning myself from network service issues.

Hi, I'm doing some triage of bugs under Internals>Media. Is it OK to have a P1 bug without an owner? 

In case not, I pinged morlovich@ who graciously accepted the honor. Thanks!