New issue
Advanced search Search tips

Issue 631978 link

Starred by 2 users
Status: Fixed
Owner:
jkarlin@chromium.org
Closed: Aug 2016
Cc:
cmumford@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Optimize QueryCache algorithm in cases where only one lookup is necessary

Project Member Reported by jkarlin@chromium.org, Jul 27 2016 
https://codereview.chromium.org/2137833002/ will introduce a QueryCache algorithm.

In some cases only one lookup is necessary (e.g., if a request is provided and ignoreSearch is false) instead of enumerating all of the cache entries. Optimize for those cases.

Then we can erase the Match() implementation and have it point to MatchAll() and return the first entry.

Comment 1 by jkarlin@chromium.org, Jul 27 2016

Cc: cmumford@chromium.org

Comment 2 by jkarlin@chromium.org, Aug 12 2016

Owner: jkarlin@chromium.org
Status: Started (was: Available)
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 22 2016 

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

commit 4694c8366a320f4fa85f4781f8e2fad565d62ef4
Author: jkarlin <jkarlin@chromium.org>
Date: Mon Aug 22 18:48:56 2016

[CacheStorage] Use QueryCache everywhere

This CL consolidates many code paths around the new QueryCache method. This CL:

1) Optimizes QueryCache in the case that only one lookup needs to be performed
2) Adds vary checking (and ignore_vary support) to QueryCache (previously was only in match)
3) Changes Cache::Delete to use QueryCache
4) Changes Cache::Put to doom rather than call Delete before inserting
5) Changes Cache::Match to use QueryCache (via MatchAll)
6) Adds unittests for all query params
7) Adds layout tests for all query params
8) Removes the RuntimeEnabledFeature for the ignoreQuery option

As a consequence of using QueryCache everywhere, options are fully supported and tested.

For reviewers: the good stuff is in cache_storage_cache.cc, most of the rest is testing.

BUG= 631978 , 426309 , 499216 , 482256 , 520784 

Review-Url: https://codereview.chromium.org/2242883002
Cr-Commit-Position: refs/heads/master@{#413487}

[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/content/browser/cache_storage/cache_storage.cc
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/content/browser/cache_storage/cache_storage.h
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/content/browser/cache_storage/cache_storage_cache.cc
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/content/browser/cache_storage/cache_storage_cache.h
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/content/browser/cache_storage/cache_storage_cache_unittest.cc
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/content/browser/cache_storage/cache_storage_dispatcher_host.cc
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/content/browser/cache_storage/cache_storage_manager.cc
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/content/browser/cache_storage/cache_storage_manager.h
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/content/browser/cache_storage/cache_storage_manager_unittest.cc
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/content/browser/service_worker/service_worker_browsertest.cc
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/content/common/cache_storage/cache_storage_types.h
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-delete.js
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-keys.js
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-match.js
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-matchAll.js
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/third_party/WebKit/LayoutTests/http/tests/cachestorage/script-tests/cache-storage-match.js
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/cache-matchAll-expected.txt
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/third_party/WebKit/LayoutTests/http/tests/cachestorage/window/cache-matchAll-expected.txt
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/third_party/WebKit/LayoutTests/http/tests/cachestorage/worker/cache-matchAll-expected.txt
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/third_party/WebKit/Source/modules/cachestorage/Cache.cpp
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/third_party/WebKit/Source/modules/cachestorage/CacheStorage.cpp
[modify] https://crrev.com/4694c8366a320f4fa85f4781f8e2fad565d62ef4/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in

Comment 4 by jkarlin@chromium.org, Aug 23 2016

Status: Fixed (was: Started)
►

Sign in to add a comment