Precache tracking database

components/precache/core/precache_database.cc

Index: components/precache/core/precache_database.cc
diff --git a/components/precache/core/precache_database.cc b/components/precache/core/precache_database.cc
new file mode 100644
index 0000000000000000000000000000000000000000..5f1687d18a99c61d650c1ae9221abe1c26bf227e
--- /dev/null
+++ b/components/precache/core/precache_database.cc
@@ -0,0 +1,181 @@
+// Copyright 2013 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 "components/precache/core/precache_database.h"
+
+#include "base/metrics/histogram.h"
+#include "base/time/time.h"

[mmenke, 2013/10/30 18:29:08]

No need to include stuff here that's in the header.

[sclittle, 2013/10/30 20:29:48]

Removed it from the header.

+#include "components/precache/core/precache_statistics_table.h"
+#include "components/precache/core/precache_url_table.h"
+#include "sql/connection.h"
+#include "url/gurl.h"
+
+namespace {
+
+// The number of days old that an entry in the precache URL table can be before
+// it is considered "old" and is removed from the table.
+const int64 kPrecacheHistoryExpiryPeriodDays = 60;
+
+void RecordSingleDayPrecacheUMA(
+    const precache::PrecacheStatisticsTable::PrecacheStatistics& stats) {
+  UMA_HISTOGRAM_COUNTS("Precache.DailyPrecachedKB",
+                       stats.precached_bytes / 1024);
+  UMA_HISTOGRAM_COUNTS("Precache.DailyDownloadedKB",
+                       stats.downloaded_bytes / 1024);
+  UMA_HISTOGRAM_COUNTS("Precache.DailyDownloadedKB.Cellular",
+                       stats.downloaded_bytes_cellular / 1024);
+  UMA_HISTOGRAM_COUNTS("Precache.DailySavedKB", stats.saved_bytes / 1024);
+  UMA_HISTOGRAM_COUNTS("Precache.DailySavedKB.Cellular",
+                       stats.saved_bytes_cellular / 1024);
+
+  if (stats.saved_bytes + stats.downloaded_bytes > 0) {
+    UMA_HISTOGRAM_PERCENTAGE("Precache.DailySavingsPercentage",
+                             stats.saved_bytes * 100.0 /
+                                 (stats.saved_bytes + stats.downloaded_bytes));
+  }
+
+  if (stats.saved_bytes_cellular + stats.downloaded_bytes_cellular > 0) {
+    UMA_HISTOGRAM_PERCENTAGE(
+        "Precache.DailySavingsPercentage.Cellular",
+        stats.saved_bytes_cellular * 100.0 /
+            (stats.saved_bytes_cellular + stats.downloaded_bytes_cellular));
+  }
+}
+
+}  // namespace
+
+namespace precache {
+
+PrecacheDatabase::PrecacheDatabase()
+    : precache_url_table_(new PrecacheURLTable()),
+      precache_statistics_table_(new PrecacheStatisticsTable()) {
+  // A PrecacheDatabase can be constructed on any thread.
+  DetachFromThread();
+}
+
+PrecacheDatabase::~PrecacheDatabase() {
+  // Since the PrecacheDatabase is refcounted, it will only be deleted if there
+  // are no references remaining to it, meaning that it is not in use. Thus, it
+  // is safe to delete it, regardless of what thread we are on.
+  DetachFromThread();
+}
+
+void PrecacheDatabase::Init(scoped_ptr<sql::Connection> db) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(!db_);  // Init must only be called once.
+  DCHECK(db);    // |db| must not be NULL.
+
+  db_.reset(db.release());
+  if (!IsDatabaseAccessible()) {
+    // Don't initialize the URL table or statistics table if unable to access
+    // the database.
+    return;
+  }
+
+  precache_url_table_->Init(db_.get());
+  precache_statistics_table_->Init(db_.get());
+}
+
+void PrecacheDatabase::ReportAndDeleteOldStats(base::Time end_date) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(db_);
+
+  if (!IsDatabaseAccessible()) {
+    // Do nothing if unable to access the database.
+    return;
+  }
+
+  // Delete old precache history that has expired.
+  precache_url_table_->DeleteAllPrecachedBetween(
+      base::Time(),
+      end_date - base::TimeDelta::FromDays(kPrecacheHistoryExpiryPeriodDays));
+
+  PrecacheStatisticsTable::PrecacheStatisticsMap stats_map;
+  precache_statistics_table_->GetAllStatsBetween(base::Time(), end_date,
+                                                 &stats_map);
+
+  // Report UMA for every row of old statistics in the statistics table. There
+  // won't be any rows in the statistics table for days when nothing was fetched
+  // or precached.
+  for (PrecacheStatisticsTable::PrecacheStatisticsMap::const_iterator it =
+           stats_map.begin();
+       it != stats_map.end(); ++it) {
+    RecordSingleDayPrecacheUMA(it->second);
+  }

[mmenke, 2013/10/30 18:29:08]

suggestion:  Just report stats for the previous day, if we have them.

Downside:  We lose stats.  There will be a bias against light users in our
numbers.  Upside:  We don't combine numbers from different days, or from
different algorithms as we try new things.  Otherwise, whenever you play with
things, you'll continue to get old stats for days.

[sclittle, 2013/10/30 20:29:48]

You have a good point, but we'd continue to get old stats for days even if we do
only report stats for the previous day.

This is because a resource could be precached days before it eventually gets
used, so if we play with different algorithms or other things on the server
side, we would have to wait for the clients' caches to catch up anyway.

Since we'll be getting data related to old precaches regardless, we're probably
better off reporting all old stats so that we don't lose any stats.

[mmenke, 2013/10/31 16:44:59]

I'm not so sure, though you are certainly correct that my proposal isn't a
perfect fix.

+
+  precache_statistics_table_->DeleteAllStatsBetween(base::Time(), end_date);
+}
+
+void PrecacheDatabase::RecordURLFetched(GURL url, base::Time fetch_time,
+                                        int64 size, bool was_cached,
+                                        bool is_precaching, bool is_cellular) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(db_);
+
+  if (!IsDatabaseAccessible()) {
+    // Don't track anything if unable to access the database.
+    return;
+  }
+
+  if (is_precaching) {
+    if (was_cached && !precache_url_table_->HasURL(url)) {
+      // Since the fetch came from the cache, and there's no entry in the
+      // URL table for the URL, this means that the resource was in the cache
+      // because of user browsing. Thus, precaching this resource had no effect,
+      // so ignore it.
+      return;
+    }
+
+    if (!was_cached) {
+      // The precache only counts as overhead if it was downloaded over the
+      // network.
+      PrecacheStatisticsTable::PrecacheStatistics stats;
+      stats.precached_bytes = size;

[mmenke, 2013/10/30 18:29:08]

I'm still not following....These bytes were not in fact precached, they were
downloaded, possibly over a cellular network, possibly not.

[sclittle, 2013/10/30 20:29:48]

Renamed "is_precaching" to "is_precache" to hopefully clarify this.

+      precache_statistics_table_->IncreaseDailyStats(fetch_time, stats);
+    }
+
+    precache_url_table_->AddURL(url, fetch_time);
+    return;
+  }
+
+  if (!was_cached) {
+    // The fetch was served over the network, so count it as downloaded bytes.
+    PrecacheStatisticsTable::PrecacheStatistics stats;
+    stats.downloaded_bytes = size;
+    if (is_cellular) {
+      stats.downloaded_bytes_cellular = size;
+    }
+    precache_statistics_table_->IncreaseDailyStats(fetch_time, stats);
+
+    // Since the fetch was over the network, delete any record of it having been
+    // precached from the URL table. If it had been precached, the resource must
+    // have expired.
+    precache_url_table_->DeleteURL(url);
+    return;
+  }
+
+  if (was_cached && precache_url_table_->HasURL(url)) {
+    // The fetch was served from the cache, and since there's an entry for this
+    // URL in the URL table, this means that the resource was served from the
+    // cache only because precaching put it there. Thus, precaching was helpful,
+    // so count the fetch as saved bytes.
+    PrecacheStatisticsTable::PrecacheStatistics stats;
+    stats.saved_bytes = size;
+    if (is_cellular) {
+      stats.saved_bytes_cellular = size;
+    }
+    precache_statistics_table_->IncreaseDailyStats(fetch_time, stats);
+
+    precache_url_table_->DeleteURL(url);
+  }
+}
+
+bool PrecacheDatabase::IsDatabaseAccessible() const {
+  DCHECK(CalledOnValidThread());
+  DCHECK(db_);
+
+  return db_->is_open();
+}
+
+}  // namespace precache