Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2508933002:
tools: Local tests for the speculative prefetch predictor. (Closed)
Descriptiontools: Local tests for the speculative prefetch predictor.
Adds a tool to reset Chrome's state, push a predictor database to a
device, and load a page after a prefetch, collecting loading metrics.
This is a follow-up to crrev.com/2496563002.
Also adds a way to only enable external prefetch requests in the predictor.
BUG=655980
Committed: https://crrev.com/b65cb1a116e559085ec372ea2ea4891bd00b1181
Cr-Commit-Position: refs/heads/master@{#435270}
Patch Set 1 #Patch Set 2 : . #
Total comments: 26
Patch Set 3 : Address comments. #Patch Set 4 : Rebase/ #
Total comments: 6
Patch Set 5 : Rebase on https://codereview.chromium.org/2522873002/ #Patch Set 6 : . #
Total comments: 4
Patch Set 7 : Rebase. #
Total comments: 13
Patch Set 8 : Comments. #Patch Set 9 : . #
Messages
Total messages: 34 (21 generated)
lizeb@chromium.org changed reviewers: + pasko@chromium.org
The CQ bit was checked by lizeb@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
I could not follow what an 'external prefetch' is and from where it is plumbed into the resource prefetch predictor. I see 'external only' being set from flags and disables all prefetches. That's not very informative. https://codereview.chromium.org/2508933002/diff/20001/chrome/browser/predicto... File chrome/browser/predictors/resource_prefetch_common.cc (right): https://codereview.chromium.org/2508933002/diff/20001/chrome/browser/predicto... chrome/browser/predictors/resource_prefetch_common.cc:29: const int kUrlHostLearningMode = ResourcePrefetchPredictorConfig::URL_LEARNING | constexpr has less risks creating entries in .data section, please use it. https://codereview.chromium.org/2508933002/diff/20001/chrome/browser/predicto... File chrome/browser/predictors/resource_prefetch_common.h (right): https://codereview.chromium.org/2508933002/diff/20001/chrome/browser/predicto... chrome/browser/predictors/resource_prefetch_common.h:76: EXTERNAL_ONLY = 1 << 4 we need to explain somewhere what an 'external' request is. This place is probably the best for it. https://codereview.chromium.org/2508933002/diff/20001/chrome/browser/predicto... File chrome/browser/predictors/resource_prefetch_predictor.cc (right): https://codereview.chromium.org/2508933002/diff/20001/chrome/browser/predicto... chrome/browser/predictors/resource_prefetch_predictor.cc:525: if (!config_.ExternalPrefetchingOnly()) so 'external prefetch only' disables all prefetching or am I missing something? https://codereview.chromium.org/2508933002/diff/20001/tools/android/customtab... File tools/android/customtabs_benchmark/scripts/customtabs_benchmark.py (right): https://codereview.chromium.org/2508933002/diff/20001/tools/android/customtab... tools/android/customtabs_benchmark/scripts/customtabs_benchmark.py:71: delay_to_launch_url, cold, chrome_args, reset_chrome_state=False): s/=False// please avoid default arguments, they can explode later https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... File tools/resource_prefetch_predictor/common.py (right): https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/common.py:23: def CreateBaseArgumentParser(description): inlining this function on every use will increase readability, won't add many lines of code, please do https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/common.py:29: def ParseAndReturnArgs(parser): as the above, this utility is quite small and does not provide much value, adds another hop to see what it means when reading the code. if it were part of tools/android/loading, that would have been probably a better incapsulation (i.e. avoided directly exposing OPTIONS outside tools/android/loading) https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... File tools/resource_prefetch_predictor/generate_database.py (right): https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/generate_database.py:16: import common the name can clash, prefetch_predictor_common feels safer https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... File tools/resource_prefetch_predictor/prefetch_benchmark.py (right): https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/prefetch_benchmark.py:1: #!/usr/bin/python every time I get a review in tools/resource_prefetch_predictor I get reminded that a better place for this is in chrome/browser/predictors/tools, but afraid to cause too many conflicts for you. Can you do it at a point that is comfortable for you? Thanks :) https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/prefetch_benchmark.py:37: 'Loads a URL with prefetch and prints loading metrics.') s/prefetch/resource_prefetch_predictor/ would be better understood by other teams https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/prefetch_benchmark.py:54: # Launch Chrome a first time to recreate the local state. nit: s/a/for the/ https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/prefetch_benchmark.py:63: # the profile directory is owned by root. What ensures that Chrome creates the database? I am worried that Chrome may start, but the db file is not created .. the file will be later owned by root and Chrome would not be able to update it. https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/prefetch_benchmark.py:78: chrome_args, False) this function has quite a few parameters, maintaining the right order is error-prone, better pass with parameter names. https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/prefetch_benchmark.py:84: logging.basicConfig(level=logging.INFO) setting up logging before initialization of an external component would be more debuggable, so unless it spits too much currently please do it
https://codereview.chromium.org/2508933002/diff/20001/chrome/browser/predicto... File chrome/browser/predictors/resource_prefetch_common.cc (right): https://codereview.chromium.org/2508933002/diff/20001/chrome/browser/predicto... chrome/browser/predictors/resource_prefetch_common.cc:29: const int kUrlHostLearningMode = ResourcePrefetchPredictorConfig::URL_LEARNING | On 2016/11/17 15:59:04, pasko wrote: > constexpr has less risks creating entries in .data section, please use it. Done. https://codereview.chromium.org/2508933002/diff/20001/chrome/browser/predicto... File chrome/browser/predictors/resource_prefetch_common.h (right): https://codereview.chromium.org/2508933002/diff/20001/chrome/browser/predicto... chrome/browser/predictors/resource_prefetch_common.h:76: EXTERNAL_ONLY = 1 << 4 On 2016/11/17 15:59:04, pasko wrote: > we need to explain somewhere what an 'external' request is. This place is > probably the best for it. Done. https://codereview.chromium.org/2508933002/diff/20001/chrome/browser/predicto... File chrome/browser/predictors/resource_prefetch_predictor.cc (right): https://codereview.chromium.org/2508933002/diff/20001/chrome/browser/predicto... chrome/browser/predictors/resource_prefetch_predictor.cc:525: if (!config_.ExternalPrefetchingOnly()) On 2016/11/17 15:59:04, pasko wrote: > so 'external prefetch only' disables all prefetching or am I missing something? It disables prefetches triggered by observing a navigation, but not ones from directly calling ResourcePrefetchPredictor::StartPrefetching. Added a comment in the place where this flag is defined. https://codereview.chromium.org/2508933002/diff/20001/tools/android/customtab... File tools/android/customtabs_benchmark/scripts/customtabs_benchmark.py (right): https://codereview.chromium.org/2508933002/diff/20001/tools/android/customtab... tools/android/customtabs_benchmark/scripts/customtabs_benchmark.py:71: delay_to_launch_url, cold, chrome_args, reset_chrome_state=False): On 2016/11/17 15:59:04, pasko wrote: > s/=False// > > please avoid default arguments, they can explode later Done. https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... File tools/resource_prefetch_predictor/common.py (right): https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/common.py:23: def CreateBaseArgumentParser(description): On 2016/11/17 15:59:04, pasko wrote: > inlining this function on every use will increase readability, won't add many > lines of code, please do Done. https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/common.py:29: def ParseAndReturnArgs(parser): On 2016/11/17 15:59:04, pasko wrote: > as the above, this utility is quite small and does not provide much value, adds > another hop to see what it means when reading the code. > > if it were part of tools/android/loading, that would have been probably a better > incapsulation (i.e. avoided directly exposing OPTIONS outside > tools/android/loading) Done. https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... File tools/resource_prefetch_predictor/generate_database.py (right): https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/generate_database.py:16: import common On 2016/11/17 15:59:04, pasko wrote: > the name can clash, prefetch_predictor_common feels safer Done. https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... File tools/resource_prefetch_predictor/prefetch_benchmark.py (right): https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/prefetch_benchmark.py:1: #!/usr/bin/python On 2016/11/17 15:59:04, pasko wrote: > every time I get a review in tools/resource_prefetch_predictor I get reminded > that a better place for this is in chrome/browser/predictors/tools, but afraid > to cause too many conflicts for you. Can you do it at a point that is > comfortable for you? Thanks :) Acknowledged. https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/prefetch_benchmark.py:37: 'Loads a URL with prefetch and prints loading metrics.') On 2016/11/17 15:59:04, pasko wrote: > s/prefetch/resource_prefetch_predictor/ would be better understood by other > teams Done. https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/prefetch_benchmark.py:54: # Launch Chrome a first time to recreate the local state. On 2016/11/17 15:59:04, pasko wrote: > nit: s/a/for the/ Done. https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/prefetch_benchmark.py:63: # the profile directory is owned by root. On 2016/11/17 15:59:04, pasko wrote: > What ensures that Chrome creates the database? I am worried that Chrome may > start, but the db file is not created .. the file will be later owned by root > and Chrome would not be able to update it. Good catch, thanks! Done. https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/prefetch_benchmark.py:78: chrome_args, False) On 2016/11/17 15:59:04, pasko wrote: > this function has quite a few parameters, maintaining the right order is > error-prone, better pass with parameter names. Done. https://codereview.chromium.org/2508933002/diff/20001/tools/resource_prefetch... tools/resource_prefetch_predictor/prefetch_benchmark.py:84: logging.basicConfig(level=logging.INFO) On 2016/11/17 15:59:04, pasko wrote: > setting up logging before initialization of an external component would be more > debuggable, so unless it spits too much currently please do it Done.
The CQ bit was checked by lizeb@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
The CQ bit was checked by lizeb@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
ohh, it did not have these trace events in the first patch, and now introduced without any reviewers asking for it. => makes for longer review, since need to remind about mentioning it in the issue description and brings onboard more comments. It's more efficient to add new functionality in separate changes. Thanks. https://codereview.chromium.org/2508933002/diff/60001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java (right): https://codereview.chromium.org/2508933002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java:249: public void run() { this easier to read at least to me: try { TraceEvent.begin("CustomTabsConnection.warmupInternal()"); runInternal(); } finally { TraceEvent.end("CustomTabsConnection.warmupInternal()"); } but up to you
On 2016/11/21 17:33:03, pasko wrote: > ohh, it did not have these trace events in the first patch, and now introduced > without any reviewers asking for it. > > => makes for longer review, since need to remind about mentioning it in the > issue description and brings onboard more comments. It's more efficient to add > new functionality in separate changes. Thanks. > > https://codereview.chromium.org/2508933002/diff/60001/chrome/android/java/src... > File > chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java > (right): > > https://codereview.chromium.org/2508933002/diff/60001/chrome/android/java/src... > chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java:249: > public void run() { > this easier to read at least to me: > > try { > TraceEvent.begin("CustomTabsConnection.warmupInternal()"); > runInternal(); > } finally { > TraceEvent.end("CustomTabsConnection.warmupInternal()"); > } > > but up to you Sorry about that. I've put it into another CL. PTAL.
https://codereview.chromium.org/2508933002/diff/60001/chrome/browser/predicto... File chrome/browser/predictors/resource_prefetch_common.cc (right): https://codereview.chromium.org/2508933002/diff/60001/chrome/browser/predicto... chrome/browser/predictors/resource_prefetch_common.cc:84: config->mode |= kUrlHostLearningMode | kUrlHostPrefetchingMode; why setting kUrlHostPrefetchingMode bit here? I thought it would be ignored via EXTERNAL_ONLY. Is it going to be ignored only sometimes? https://codereview.chromium.org/2508933002/diff/60001/chrome/browser/predicto... chrome/browser/predictors/resource_prefetch_common.cc:88: config->mode |= kUrlHostLearningMode; why only learning is enabled here? https://codereview.chromium.org/2508933002/diff/60001/chrome/browser/predicto... chrome/browser/predictors/resource_prefetch_common.cc:229: bool ResourcePrefetchPredictorConfig::ExternalPrefetchingOnly() const { I am confused by this name "ExternalPrefetchingOnly" suggests to me that only external prefetching is enabled by this config. At the same time the code asserts that learning is allowed as well. https://codereview.chromium.org/2508933002/diff/60001/chrome/browser/predicto... File chrome/browser/predictors/resource_prefetch_common.h (right): https://codereview.chromium.org/2508933002/diff/60001/chrome/browser/predicto... chrome/browser/predictors/resource_prefetch_common.h:79: EXTERNAL_ONLY = 1 << 4 Non-orthogonal bits are confusing, especially when inferred from combinations of flags that are non-orthogonal themselves. It is unnecessarily hard to guess what you wanted. It seems it has some bugs, but perhaps some of those are intentional behaviors, but hard to tell which ones are shich. Orthogonal naming I am thinking about: URL_LEARNING, HOST_LEARNING, URL_PREFETCHING_FOR_MAIN_FRAMES, HOST_PREFETCHING_FOR_MAIN_FRAMES, URL_PREFETCHING_FOR_EXTERNAL_REQUESTS, HOST_PREFETCHING_FOR_EXTERNAL_REQUESTS Disabling prefetch for external requests while keeping prefetch for main frame requests may also be useful, supporting it would arrive essentially for free. https://codereview.chromium.org/2508933002/diff/100001/chrome/common/chrome_s... File chrome/common/chrome_switches.cc (right): https://codereview.chromium.org/2508933002/diff/100001/chrome/common/chrome_s... chrome/common/chrome_switches.cc:902: const char kSpeculativeResourceExternalPrefetchingEnabled[] = "external"; following the pattern of the above two, it should be named more like: kSpeculativeResourcePrefetchingExternal or: kSpeculativeResourcePrefetchingExternalOnly https://codereview.chromium.org/2508933002/diff/100001/chrome/common/chrome_s... chrome/common/chrome_switches.cc:902: const char kSpeculativeResourceExternalPrefetchingEnabled[] = "external"; nit: From naming it is not obvious whether "external" enables a subset of functionality provided by "enabled"? We could name it so it would be more obvious, like: "enabled" "enabled-external-only" or: "enabled-all" "enabled-external"
Split the CL and rebased. Thanks for the comments. PTAL. https://codereview.chromium.org/2508933002/diff/60001/chrome/browser/predicto... File chrome/browser/predictors/resource_prefetch_common.cc (right): https://codereview.chromium.org/2508933002/diff/60001/chrome/browser/predicto... chrome/browser/predictors/resource_prefetch_common.cc:88: config->mode |= kUrlHostLearningMode; On 2016/11/24 19:31:16, pasko wrote: > why only learning is enabled here? Done. https://codereview.chromium.org/2508933002/diff/100001/chrome/common/chrome_s... File chrome/common/chrome_switches.cc (right): https://codereview.chromium.org/2508933002/diff/100001/chrome/common/chrome_s... chrome/common/chrome_switches.cc:902: const char kSpeculativeResourceExternalPrefetchingEnabled[] = "external"; On 2016/11/24 19:31:16, pasko wrote: > nit: From naming it is not obvious whether "external" enables a subset of > functionality provided by "enabled"? We could name it so it would be more > obvious, like: > > "enabled" > "enabled-external-only" > > or: > > "enabled-all" > "enabled-external" Done. https://codereview.chromium.org/2508933002/diff/100001/chrome/common/chrome_s... chrome/common/chrome_switches.cc:902: const char kSpeculativeResourceExternalPrefetchingEnabled[] = "external"; On 2016/11/24 19:31:16, pasko wrote: > following the pattern of the above two, it should be named more like: > > kSpeculativeResourcePrefetchingExternal > > or: > > kSpeculativeResourcePrefetchingExternalOnly Done.
The CQ bit was checked by lizeb@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
overall looks good, a few small suggestions and questions https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... File tools/resource_prefetch_predictor/generate_database.py (right): https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... tools/resource_prefetch_predictor/generate_database.py:32: import prefetch_predictor_common why importing twice? https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... File tools/resource_prefetch_predictor/prefetch_benchmark.py (right): https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... tools/resource_prefetch_predictor/prefetch_benchmark.py:92: + ['--trace-startup', '--trace-startup-duration=20']) what do we use from --trace-startup? https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... File tools/resource_prefetch_predictor/prefetch_predictor_common.py (right): https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... tools/resource_prefetch_predictor/prefetch_predictor_common.py:7: import argparse not necessary? https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... tools/resource_prefetch_predictor/prefetch_predictor_common.py:23: def FindDevice(device_id): can we use device_setup.GetDeviceFromSerial()? https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... tools/resource_prefetch_predictor/prefetch_predictor_common.py:28: matching_devices = [d for d in devices if str(d) == device_id] nit: constructing the list to only get the first element? also 4 lines but more readable for my imperative brain: for d in devices: if str(d) == device_id: return d return None # or throw, or just use device_setup.GetDeviceFromSerial just a nit, not necessary to do change anything https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... tools/resource_prefetch_predictor/prefetch_predictor_common.py:35: """Sets up a device and returns an instance of RemoteChromeController.""" should document |additional_flags| and the class for |device|
https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... File tools/resource_prefetch_predictor/generate_database.py (right): https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... tools/resource_prefetch_predictor/generate_database.py:32: import prefetch_predictor_common On 2016/11/30 12:53:46, pasko wrote: > why importing twice? ... Sorry about that. Done. https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... File tools/resource_prefetch_predictor/prefetch_benchmark.py (right): https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... tools/resource_prefetch_predictor/prefetch_benchmark.py:92: + ['--trace-startup', '--trace-startup-duration=20']) On 2016/11/30 12:53:46, pasko wrote: > what do we use from --trace-startup? Debugging. The trace is there, easy to look at it. Do you want this removed? https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... File tools/resource_prefetch_predictor/prefetch_predictor_common.py (right): https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... tools/resource_prefetch_predictor/prefetch_predictor_common.py:7: import argparse On 2016/11/30 12:53:46, pasko wrote: > not necessary? Done. https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... tools/resource_prefetch_predictor/prefetch_predictor_common.py:23: def FindDevice(device_id): On 2016/11/30 12:53:46, pasko wrote: > can we use device_setup.GetDeviceFromSerial()? Thanks! Done. https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... tools/resource_prefetch_predictor/prefetch_predictor_common.py:28: matching_devices = [d for d in devices if str(d) == device_id] On 2016/11/30 12:53:46, pasko wrote: > nit: constructing the list to only get the first element? > > also 4 lines but more readable for my imperative brain: > > for d in devices: > if str(d) == device_id: > return d > return None # or throw, or just use device_setup.GetDeviceFromSerial > > just a nit, not necessary to do change anything Acknowledged. https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... tools/resource_prefetch_predictor/prefetch_predictor_common.py:35: """Sets up a device and returns an instance of RemoteChromeController.""" On 2016/11/30 12:53:46, pasko wrote: > should document |additional_flags| and the class for |device| Done.
lgtm, thank you https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... File tools/resource_prefetch_predictor/prefetch_benchmark.py (right): https://codereview.chromium.org/2508933002/diff/120001/tools/resource_prefetc... tools/resource_prefetch_predictor/prefetch_benchmark.py:92: + ['--trace-startup', '--trace-startup-duration=20']) On 2016/11/30 15:25:50, Benoit L wrote: > On 2016/11/30 12:53:46, pasko wrote: > > what do we use from --trace-startup? > > Debugging. The trace is there, easy to look at it. > Do you want this removed? no, not at all. I was wondering whether something relys on it, in which case I wanted to know what, coz that would have been an interesting wtf. Can you just drop a comment saying that this is not essential for operation of the benchmark, just extra events for debugging?
The CQ bit was checked by lizeb@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from pasko@chromium.org Link to the patchset: https://codereview.chromium.org/2508933002/#ps160001 (title: ".")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 160001, "attempt_start_ts": 1480520829037600,
"parent_rev": "fa8dc66fc643c757076297403cfa0afdae097b70", "commit_rev":
"f038744772a8c13bf4cd1f64b1d580f576099317"}
Message was sent while issue was closed.
Committed patchset #9 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== tools: Local tests for the speculative prefetch predictor. Adds a tool to reset Chrome's state, push a predictor database to a device, and load a page after a prefetch, collecting loading metrics. This is a follow-up to crrev.com/2496563002. Also adds a way to only enable external prefetch requests in the predictor. BUG=655980 ========== to ========== tools: Local tests for the speculative prefetch predictor. Adds a tool to reset Chrome's state, push a predictor database to a device, and load a page after a prefetch, collecting loading metrics. This is a follow-up to crrev.com/2496563002. Also adds a way to only enable external prefetch requests in the predictor. BUG=655980 Committed: https://crrev.com/b65cb1a116e559085ec372ea2ea4891bd00b1181 Cr-Commit-Position: refs/heads/master@{#435270} ==========
Message was sent while issue was closed.
Patchset 9 (id:??) landed as https://crrev.com/b65cb1a116e559085ec372ea2ea4891bd00b1181 Cr-Commit-Position: refs/heads/master@{#435270} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
