Performance tests for bookmark sync.

chrome/test/live_sync/performance_live_bookmarks_sync_test.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 "base/time.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sync/profile_sync_service_harness.h"
+#include "chrome/test/live_sync/live_bookmarks_sync_test.h"
+*/

[Raghu Simha, 2011/06/16 22:42:55]

Remove this block if it's unnecessary.

[braffert, 2011/06/17 18:19:46]

Done.

+
+#include "chrome/browser/sync/profile_sync_service_harness.h"
+#include "chrome/test/live_sync/live_bookmarks_sync_test.h"
+#include "chrome/test/live_sync/live_sync_timing_helper.h"
+
+static const int kNumBookmarks = 150;
+
+// TODO(braffert): Consider the range / resolution of these test points.
+static const int kNumBenchmarkPoints = 18;
+static const int kBenchmarkPoints[] = {1, 10, 20, 30, 40, 50, 75, 100, 125,
+                                       150, 175, 200, 225, 250, 300, 350, 400,
+                                       500};
+
+// TODO(braffert): Move this class into its own .h/.cc files.  What should the
+// class files be named as opposed to the file containing the tests themselves?

[Raghu Simha, 2011/06/16 22:42:55]

How about naming the class LiveBookmarksSyncPerformanceTest and moving it to
live_bookmarks_sync_performance_test.{h/cc}? (I realise all the class names in
this directory are wordy, but consider it an artifact of chrome history :) )

+class PerformanceLiveBookmarksSyncTest
+    : public TwoClientLiveBookmarksSyncTest {
+ public:
+  PerformanceLiveBookmarksSyncTest() : urlNumber(0), urlTitleNumber(0) {}
+
+  // Pairs the test class with the timer helper for measuring sync times.
+  void SetUp() OVERRIDE;

[Raghu Simha, 2011/06/16 22:42:55]

With the change I've suggested to LiveSyncTimingHelper::TimeUntilQuiescence, we
won't need this method.

[braffert, 2011/06/17 18:19:46]

Done.

+
+  // Adds |nURLs| new unique bookmarks to the bookmark bar for |profile|.
+  void AddURLs(int profile, int nURLs);

[Raghu Simha, 2011/06/16 22:42:55]

Style guide recommends num_urls over nURLs.

[braffert, 2011/06/17 18:19:46]

Done.

+
+  // Updates the URL for all bookmarks in the bookmark bar for |profile|.
+  void UpdateURLs(int profile);
+
+  // Removes all bookmarks in the bookmark bar for |profile|.
+  void RemoveURLs(int profile);
+
+  // Remvoes all bookmarks in the bookmark bars for all profiles.  Called
+  // between benchmark iterations.
+  void Cleanup();
+
+  LiveSyncTimingHelper timing_helper;

[Raghu Simha, 2011/06/16 22:42:55]

With the change I've suggested to LiveSyncTimingHelper::TimeUntilQuiescence, we
won't need this member.

[braffert, 2011/06/17 18:19:46]

Done.

+
+ private:
+  // Returns a new unique bookmark URL.
+  GURL NextIndexedURL();

[Raghu Simha, 2011/06/16 22:42:55]

IndexedURL returns a wstring while NextIndexedURL returns a GURL. Better to be
consistent and make them return the same type.

Easy fix: Make NextIndexedURL return a wstring and do a find-replace in the
performance tests.
Maybe painful fix: Make IndexedURL return a GURL and do a find-replace in all
the bookmarks tests.

Your choice :)

[braffert, 2011/06/17 18:19:46]

Taking the easy option here - just to keep this CL more focused.

+
+  // Returns a new unique bookmark title.
+  std::wstring NextIndexedURLTitle();
+
+  int urlNumber;

[Raghu Simha, 2011/06/16 22:42:55]

Style guide recommends that variable names are lower_case_with_underscore and
not mixedCase. (Here and everywhere else in this file)

[braffert, 2011/06/17 18:19:46]

Done.

+  int urlTitleNumber;
+  DISALLOW_COPY_AND_ASSIGN(PerformanceLiveBookmarksSyncTest);
+};
+
+void PerformanceLiveBookmarksSyncTest::SetUp() {
+  timing_helper.Setup(this);
+  LiveSyncTest::SetUp();
+}
+
+void PerformanceLiveBookmarksSyncTest::AddURLs(int profile, int nURLs) {
+  for (int i = 0; i < nURLs; ++i) {
+    ASSERT_TRUE(AddURL(
+        profile, 0, NextIndexedURLTitle(), NextIndexedURL()) != NULL);
+  }
+}
+
+void PerformanceLiveBookmarksSyncTest::UpdateURLs(int profile) {
+  for (int i = 0; i < GetBookmarkBarNode(profile)->child_count(); ++i) {
+    ASSERT_TRUE(SetURL(profile, GetBookmarkBarNode(profile)->GetChild(i),
+                       NextIndexedURL()));
+  }
+}
+
+void PerformanceLiveBookmarksSyncTest::RemoveURLs(int profile) {
+  while (GetBookmarkBarNode(profile)->child_count()) {
+    Remove(profile, GetBookmarkBarNode(profile), 0);
+  }
+}
+
+void PerformanceLiveBookmarksSyncTest::Cleanup() {
+  for (int i = 0; i < num_clients(); ++i) {
+    RemoveURLs(i);
+  }
+  ASSERT_TRUE(AwaitQuiescence());
+  ASSERT_EQ(0, GetBookmarkBarNode(0)->child_count());
+  ASSERT_TRUE(AllModelsMatch());
+}
+
+GURL PerformanceLiveBookmarksSyncTest::NextIndexedURL() {
+  return GURL(IndexedURL(urlNumber++));
+}
+
+std::wstring PerformanceLiveBookmarksSyncTest::NextIndexedURLTitle() {
+  return IndexedURLTitle(urlTitleNumber++);
+}
+
+// TODO(braffert): Possibly split each of these into separate up / down test
+// cases?
+IN_PROC_BROWSER_TEST_F(PerformanceLiveBookmarksSyncTest, Add) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  DisableVerifier();
+
+  DisableNetwork(GetProfile(1));
+  AddURLs(0, kNumBookmarks);
+  base::TimeDelta dt_up = timing_helper.TimeSyncCycle(GetClient(0));
+  ASSERT_EQ(kNumBookmarks, GetBookmarkBarNode(0)->child_count());
+  ASSERT_EQ(0, GetBookmarkBarNode(1)->child_count());
+
+  EnableNetwork(GetProfile(1));
+  base::TimeDelta dt_down = timing_helper.TimeMutualSyncCycle(GetClient(0),
+                                                              GetClient(1));
+  ASSERT_EQ(kNumBookmarks, GetBookmarkBarNode(0)->child_count());
+  ASSERT_TRUE(AllModelsMatch());
+
+  // TODO(braffert): Compare timings against some target value.
+}
+
+IN_PROC_BROWSER_TEST_F(PerformanceLiveBookmarksSyncTest, Update) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  DisableVerifier();
+
+  AddURLs(0, kNumBookmarks);
+  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
+  ASSERT_TRUE(AllModelsMatch());
+
+  DisableNetwork(GetProfile(1));
+  UpdateURLs(0);
+  base::TimeDelta dt_up = timing_helper.TimeSyncCycle(GetClient(0));
+  ASSERT_EQ(kNumBookmarks, GetBookmarkBarNode(0)->child_count());
+  ASSERT_EQ(kNumBookmarks, GetBookmarkBarNode(1)->child_count());
+  ASSERT_FALSE(AllModelsMatch());
+
+  EnableNetwork(GetProfile(1));
+  base::TimeDelta dt_down = timing_helper.TimeMutualSyncCycle(GetClient(0),
+                                                              GetClient(1));
+  ASSERT_EQ(kNumBookmarks, GetBookmarkBarNode(0)->child_count());
+  ASSERT_EQ(kNumBookmarks, GetBookmarkBarNode(1)->child_count());
+
+  // TODO(braffert): Compare timings against some target value.
+}
+
+IN_PROC_BROWSER_TEST_F(PerformanceLiveBookmarksSyncTest, Delete) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  DisableVerifier();
+
+  AddURLs(0, kNumBookmarks);
+  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
+  ASSERT_TRUE(AllModelsMatch());
+
+  DisableNetwork(GetProfile(1));
+  RemoveURLs(0);
+  base::TimeDelta dt_up = timing_helper.TimeSyncCycle(GetClient(0));
+  ASSERT_EQ(0, GetBookmarkBarNode(0)->child_count());
+  ASSERT_EQ(kNumBookmarks, GetBookmarkBarNode(1)->child_count());
+
+  EnableNetwork(GetProfile(1));
+  base::TimeDelta dt_down = timing_helper.TimeMutualSyncCycle(GetClient(0),
+                                                              GetClient(1));
+  ASSERT_EQ(0, GetBookmarkBarNode(0)->child_count());
+  ASSERT_EQ(0, GetBookmarkBarNode(1)->child_count());
+
+  // TODO(braffert): Compare timings against some target value.
+}
+
+IN_PROC_BROWSER_TEST_F(PerformanceLiveBookmarksSyncTest, Benchmark) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  DisableVerifier();
+
+  for (int i = 0; i < kNumBenchmarkPoints; ++i) {
+    int numBookmarks = kBenchmarkPoints[i];

[Raghu Simha, 2011/06/16 22:42:55]

numBookmarks -> num_bookmarks.

[braffert, 2011/06/17 18:19:46]

Done.  Curious - why do constants use a different naming convention?  Just so
that they are identifiable?

+
+    // Disable client 1.  Add bookmarks and time commit by client 0.
+    DisableNetwork(GetProfile(1));
+    AddURLs(0, numBookmarks);
+    base::TimeDelta dt_up = timing_helper.TimeSyncCycle(GetClient(0));
+    ASSERT_EQ(numBookmarks, GetBookmarkBarNode(0)->child_count());
+    ASSERT_EQ(0, GetBookmarkBarNode(1)->child_count());
+
+    // Enable client 1 and time update (new bookmarks).
+    EnableNetwork(GetProfile(1));
+    base::TimeDelta dt_down = timing_helper.TimeMutualSyncCycle(GetClient(0),
+                                                                GetClient(1));
+    ASSERT_EQ(numBookmarks, GetBookmarkBarNode(0)->child_count());
+    ASSERT_TRUE(AllModelsMatch());
+
+    VLOG(0) << std::endl << "Add: " << numBookmarks << " " <<
+            static_cast<double>(numBookmarks) / dt_up.InSecondsF() << " " <<
+            static_cast<double>(numBookmarks) / dt_down.InSecondsF();
+
+    // Disable client 1.  Modify bookmarks and time commit by client 0.
+    DisableNetwork(GetProfile(1));
+    UpdateURLs(0);
+    dt_up = timing_helper.TimeSyncCycle(GetClient(0));
+    ASSERT_EQ(numBookmarks, GetBookmarkBarNode(0)->child_count());
+    ASSERT_EQ(numBookmarks, GetBookmarkBarNode(1)->child_count());
+    ASSERT_FALSE(AllModelsMatch());
+
+    // Enable client 1 and time update (changed bookmarks).
+    EnableNetwork(GetProfile(1));
+    dt_down = timing_helper.TimeMutualSyncCycle(GetClient(0), GetClient(1));
+    ASSERT_EQ(numBookmarks, GetBookmarkBarNode(0)->child_count());
+    ASSERT_TRUE(AllModelsMatch());
+
+    VLOG(0) << std::endl << "Update: " << numBookmarks << " " <<
+            static_cast<double>(numBookmarks) / dt_up.InSecondsF() << " " <<
+            static_cast<double>(numBookmarks) / dt_down.InSecondsF();
+
+    // Disable client 1.  Delete bookmarks and time commit by client 0.
+    DisableNetwork(GetProfile(1));
+    RemoveURLs(0);
+    dt_up = timing_helper.TimeSyncCycle(GetClient(0));
+    ASSERT_EQ(0, GetBookmarkBarNode(0)->child_count());
+    ASSERT_EQ(numBookmarks, GetBookmarkBarNode(1)->child_count());
+
+    // Enable client 1 and time update (deleted bookmarks).
+    EnableNetwork(GetProfile(1));
+    dt_down = timing_helper.TimeMutualSyncCycle(GetClient(0), GetClient(1));
+    ASSERT_EQ(0, GetBookmarkBarNode(0)->child_count());
+    ASSERT_EQ(0, GetBookmarkBarNode(1)->child_count());
+
+    VLOG(0) << std::endl << "Delete: " << numBookmarks << " " <<
+            static_cast<double>(numBookmarks) / dt_up.InSecondsF() << " " <<
+            static_cast<double>(numBookmarks) / dt_down.InSecondsF();
+
+    Cleanup();
+  }
+}