Record the time to update top sites through db thread to understand if this is low and how slow it …

components/history/core/browser/top_sites_backend.cc

 // Copyright (c) 2011 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/history/core/browser/top_sites_backend.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/files/file_path.h"
 #include "base/location.h"
 #include "base/memory/ref_counted.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/single_thread_task_runner.h"
 #include "base/task/cancelable_task_tracker.h"
+#include "base/trace_event/trace_event.h"
 #include "components/history/core/browser/top_sites_database.h"
 #include "sql/connection.h"
 
 namespace history {
 
 TopSitesBackend::TopSitesBackend(
     const scoped_refptr<base::SingleThreadTaskRunner>& db_task_runner)
     : db_(new TopSitesDatabase()), db_task_runner_(db_task_runner) {
   DCHECK(db_task_runner_);
 }
@@ skipping 71 matching lines @@
   if (db_) {
     db_->GetPageThumbnails(&(thumbnails->most_visited),
                            &(thumbnails->url_to_images_map));
   }
 }
 
 void TopSitesBackend::UpdateTopSitesOnDBThread(const TopSitesDelta& delta) {
   if (!db_)
     return;
 
+  TRACE_EVENT0("startup", "history::TopSitesBackend::UpdateTopSitesOnDBThread");

[rkaplow, 2015/03/17 22:41:12]

trace event should even be before the !db check.

[yao, 2015/03/18 19:38:05]

I was trying to differenciate the cases where io does happen and io doesn't. 
If io doesn't happen, then this function won't cost a lot at all. 

Or is it that tracing have to be put at the start of a function? 

[rkaplow, 2015/03/19 20:30:03]

I think TRACE is supposed to be always at the start. The idea is to show
whenever the method is called, and indeed in this case the method is called. If
you do this you're hiding when this is happening which is misleading.

One option is to have a method which is everything after the return, like an
ExecuteUpdate() and you can add your metrics to that.

+  // TODO(yiyaoliu): Remove the histogram and related code when crbug/223430 is
+  // fixed.
+  base::Time begin_time = base::Time::Now();
+
   for (size_t i = 0; i < delta.deleted.size(); ++i)
     db_->RemoveURL(delta.deleted[i]);
 
   for (size_t i = 0; i < delta.added.size(); ++i)
     db_->SetPageThumbnail(delta.added[i].url, delta.added[i].rank, Images());
 
   for (size_t i = 0; i < delta.moved.size(); ++i)
     db_->UpdatePageRank(delta.moved[i].url, delta.moved[i].rank);
+
+  UMA_HISTOGRAM_TIMES("History.UpdateTopSitesOnDBThreadTimeConsumption",

[rkaplow, 2015/03/17 22:41:12]

I would get rid of 'Consumption' 
FunctionTime is fine.

[yao, 2015/03/18 19:38:06]

Done.

+                      base::Time::Now() - begin_time);
 }
 
 void TopSitesBackend::SetPageThumbnailOnDBThread(const MostVisitedURL& url,
                                                  int url_rank,
                                                  const Images& thumbnail) {
   if (!db_)
     return;
 
   db_->SetPageThumbnail(url, url_rank, thumbnail);
 }
 
 void TopSitesBackend::ResetDatabaseOnDBThread(const base::FilePath& file_path) {
   DCHECK(db_task_runner_->BelongsToCurrentThread());
   db_.reset(NULL);
   sql::Connection::Delete(db_path_);
   db_.reset(new TopSitesDatabase());
   InitDBOnDBThread(db_path_);
 }
 
 }  // namespace history