predictors: Add prefetching hit/miss histograms.

chrome/browser/predictors/resource_prefetch_predictor.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/predictors/resource_prefetch_predictor.h"
 
 #include <map>
 #include <set>
 #include <utility>
 
@@ skipping 102 matching lines @@
     visit_count_ = url_row.visit_count();
   return true;
 }
 
 void GetUrlVisitCountTask::DoneRunOnMainThread() {
   callback_.Run(visit_count_, *summary_);
 }
 
 GetUrlVisitCountTask::~GetUrlVisitCountTask() {}
 
+void ReportPrefetchAccuracy(
+    const ResourcePrefetcher::PrefetcherStats& stats,
+    const std::vector<ResourcePrefetchPredictor::URLRequestSummary>&
+        summaries) {
+  if (stats.requests_stats.empty())
+    return;
+
+  std::set<GURL> urls;
+  for (const auto& summary : summaries)
+    urls.insert(summary.resource_url);
+
+  int cached_misses_count = 0;
+  int not_cached_misses_count = 0;
+  int cached_hits_count = 0;
+  int not_cached_hits_count = 0;
+  size_t misses_bytes = 0;
+  size_t hits_bytes = 0;
+
+  for (const auto& request_stats : stats.requests_stats) {
+    bool hit = urls.find(request_stats.resource_url) != urls.end();
+    bool cached = request_stats.was_cached;
+    size_t bytes = request_stats.total_received_bytes;
+
+    cached_hits_count += cached && hit;
+    cached_misses_count += cached && !hit;
+    not_cached_hits_count += !cached && hit;
+    not_cached_misses_count += !cached && !hit;
+    misses_bytes += !hit * bytes;
+    hits_bytes += hit * bytes;
+  }
+
+  UMA_HISTOGRAM_COUNTS_100(
+      internal::kResourcePrefetchPredictorPrefetchMissesCountCached,
+      cached_misses_count);
+  UMA_HISTOGRAM_COUNTS_100(
+      internal::kResourcePrefetchPredictorPrefetchMissesCountNotCached,
+      not_cached_misses_count);
+  UMA_HISTOGRAM_COUNTS_100(
+      internal::kResourcePrefetchPredictorPrefetchHitsCountCached,
+      cached_hits_count);
+  UMA_HISTOGRAM_COUNTS_100(
+      internal::kResourcePrefetchPredictorPrefetchHitsCountNotCached,
+      not_cached_hits_count);
+  UMA_HISTOGRAM_COUNTS_10000(
+      internal::kResourcePrefetchPredictorPrefetchHitsSize, hits_bytes / 1024);
+  UMA_HISTOGRAM_COUNTS_10000(
+      internal::kResourcePrefetchPredictorPrefetchMissesSize,
+      misses_bytes / 1024);
+}
+
 void ReportPredictionAccuracy(
     const std::vector<GURL>& predicted_urls,
     const ResourcePrefetchPredictor::PageRequestSummary& summary) {
   DCHECK(!predicted_urls.empty());
 
   if (predicted_urls.empty() || summary.subresource_requests.empty())
     return;
 
   std::set<GURL> predicted_urls_set(predicted_urls.begin(),
                                     predicted_urls.end());
@@ skipping 394 matching lines @@
   if (!prefetch_manager_.get())  // Not enabled.
     return;
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&ResourcePrefetcherManager::MaybeRemovePrefetch,
                  prefetch_manager_, url));
 }
 
 void ResourcePrefetchPredictor::OnPrefetchingFinished(
-    const GURL& main_frame_url) {
+    const GURL& main_frame_url,
+    const ResourcePrefetcher::PrefetcherStats& stats) {
   if (observer_)
-    observer_->OnPrefetchingFinished(main_frame_url);
+    observer_->OnPrefetchingFinished(main_frame_url, stats);

[alexilin, 2017/02/09 15:24:17]

You don't use stats inside an observer. Why?

[Benoit L, 2017/02/13 16:13:15]

Done.

+
+  prefetcher_stats_.insert({main_frame_url, stats});
 }
 
 bool ResourcePrefetchPredictor::IsUrlPrefetchable(const GURL& main_frame_url) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   if (initialization_state_ != INITIALIZED)
     return false;
 
   return GetPrefetchData(main_frame_url, nullptr);
 }
 
@@ skipping 85 matching lines @@
 
   NavigationMap::iterator nav_it =
       inflight_navigations_.find(nav_id_without_timing_info);
   if (nav_it == inflight_navigations_.end())
     return;
 
   // Remove the navigation from the inflight navigations.
   std::unique_ptr<PageRequestSummary> summary = std::move(nav_it->second);
   inflight_navigations_.erase(nav_it);
 
+  const GURL& main_frame_url = nav_id_without_timing_info.main_frame_url;
   std::vector<GURL> predicted_urls;
-  bool has_data = GetPrefetchData(nav_id_without_timing_info.main_frame_url,
-                                  &predicted_urls);
+  bool has_data = GetPrefetchData(main_frame_url, &predicted_urls);
   if (has_data)
     ReportPredictionAccuracy(predicted_urls, *summary);
 
+  auto it = prefetcher_stats_.find(main_frame_url);

[alexilin, 2017/02/09 15:24:17]

I suspect that prefetching can end after navigation complete in rare cases. Do
you think it is ok?

[Benoit L, 2017/02/13 16:13:15]

Yes, that's possible, but I don't think it's worth the complexity, as this would
happen for the case where a request which is a miss hangs, while the page loads.
wdyt?

[alexilin, 2017/02/13 17:07:05]

Yes, I also think that this case isn't worth the additional complexity. But it
reminded me that we should be aware of size of subresources that we prefetch. 
I think we need to abort too big resources while prefetching. Resource size
written to the database could be wrong (and we don't even store it yet).

+  if (it != prefetcher_stats_.end()) {
+    const std::vector<URLRequestSummary>& summaries =
+        summary->subresource_requests;
+    ReportPrefetchAccuracy(it->second, summaries);
+    prefetcher_stats_.erase(it);
+  }
+
+  // Remove old prefetches that haven't been claimed.

[alexilin, 2017/02/09 15:24:17]

Could you move this into CleanupAbandonedNavigations()? I also added cleanup
(timeout-based) code to this function in my CL.

[Benoit L, 2017/02/13 16:13:15]

Done.

+  base::TimeTicks now = base::TimeTicks::Now();
+  it = prefetcher_stats_.begin();
+  while (it != prefetcher_stats_.end()) {
+    if (now - it->second.start_time > base::TimeDelta::FromMinutes(5)) {
+      // No requests -> everything is a miss.
+      ReportPrefetchAccuracy(it->second, std::vector<URLRequestSummary>());
+      it = prefetcher_stats_.erase(it);
+    } else {
+      ++it;
+    }
+  }
+
   // Kick off history lookup to determine if we should record the URL.
   history::HistoryService* history_service =
       HistoryServiceFactory::GetForProfile(profile_,
                                            ServiceAccessType::EXPLICIT_ACCESS);
   DCHECK(history_service);
   history_service->ScheduleDBTask(
       std::unique_ptr<history::HistoryDBTask>(new GetUrlVisitCountTask(
           std::move(summary),
           base::Bind(&ResourcePrefetchPredictor::OnVisitCountLookup,
                      AsWeakPtr()))),
@@ skipping 580 matching lines @@
 TestObserver::~TestObserver() {
   predictor_->SetObserverForTesting(nullptr);
 }
 
 TestObserver::TestObserver(ResourcePrefetchPredictor* predictor)
     : predictor_(predictor) {
   predictor_->SetObserverForTesting(this);
 }
 
 }  // namespace predictors