Telemetry: Create new profile creator large_profile_creator.

tools/perf/profile_creators/fast_navigation_profile_extender.py

Index: tools/perf/profile_creators/fast_navigation_profile_extender.py
diff --git a/tools/perf/profile_creators/fast_navigation_profile_extender.py b/tools/perf/profile_creators/fast_navigation_profile_extender.py
index 397002856d105f0541ee75549e37ebc30e772aae..fd0df3e083b0ad13f72f714a285813142f8b74e8 100644
--- a/tools/perf/profile_creators/fast_navigation_profile_extender.py
+++ b/tools/perf/profile_creators/fast_navigation_profile_extender.py
@@ -1,6 +1,7 @@
 # Copyright 2015 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.
+import multiprocessing
 import time
 
 from telemetry.core import browser_finder
@@ -24,21 +25,18 @@ class FastNavigationProfileExtender(object):
   def __init__(self):
     super(FastNavigationProfileExtender, self).__init__()
 
+    # The path of the profile that the browser will use while its running.

[sullivan, 2015/02/18 21:18:05]

nit: it's

[erikchen, 2015/02/18 22:04:01]

Done.

+    # This member is initialized during SetUp().
+    self.profile_path = None
+
     # A reference to the browser that will be performing all of the tab
     # navigations.
+    # This member is initialized during SetUp().
     self._browser = None
 
-    # A static copy of the urls that this class is going to navigate to.
-    self._navigation_urls = None
-
     # The number of tabs to use.
-    self._NUM_TABS = 15
-
-    # The number of pages to load in parallel.
-    self._NUM_PARALLEL_PAGES = 15
-
-    assert self._NUM_PARALLEL_PAGES <= self._NUM_TABS, (' the batch size can\'t'
-        ' be larger than the number of available tabs')
+    # This member is initialized during SetUp().
+    self._NUM_TABS = None

[nednguyen, 2015/02/18 22:43:14]

Why not make this a parameter in the constructor?

[erikchen, 2015/02/18 23:32:30]

Done.

 
     # The amount of time to wait for a batch of pages to finish loading.
     self._BATCH_PAGE_LOAD_TIMEOUT_IN_SECONDS = 10
@@ -55,19 +53,71 @@ class FastNavigationProfileExtender(object):
       profile, and sufficient information to choose a specific browser binary.
     """
     try:
-      self._navigation_urls = self.GetUrlsToNavigate()
-      self._SetUpBrowser(finder_options)
+      self.SetUp(finder_options)
       self._PerformNavigations()
     finally:
-      self._TearDownBrowser()
+      self.TearDown()
+
+  def GetUrlsToNavigate(self, maximum_batch_size):
+    """Gets URLs for the browser to navigate to.
+
+    Intended for subclass override.
+
+    Args:
+      maximum_batch_size: A positive integer indicating the maximum allowed
+      length of the returned list.
+    Returns:
+      A list of urls to be navigated to.
+    """
+    raise NotImplementedError()
 
-  def GetUrlsToNavigate(self):
-    """Returns a list of urls to be navigated to.
+  def ShouldExit(self):
+    """Returns a boolean indicating whether profile extension is finished.
 
     Intended for subclass override.
     """
     raise NotImplementedError()
 
+  def NumTabs(self):
+    """Returns the number of tabs to simultaneously load pages in.
+
+    Can be overridden by subclasses.
+    """
+    return multiprocessing.cpu_count()

[nednguyen, 2015/02/18 22:43:14]

Why do you think hat number of tabs should equal number of cpus?

[erikchen, 2015/02/18 23:32:30]

I moved this logic into the specific subclasses, and added comments. In short: A
lot of these operations are CPU bound, so having 1 tab per logical core is close
to optimal.

+
+  def SetUp(self, finder_options):
+    """Finds the browser, starts the browser, and opens the requisite number of
+    tabs.
+
+    Can be overridden by subclasses. Subclasses must call the super class
+    implementation.
+    """
+    self.profile_path = finder_options.output_profile_path
+    self._NUM_TABS = self.NumTabs()
+    possible_browser = self._GetPossibleBrowser(finder_options)
+    self._browser = possible_browser.Create(finder_options)

[sullivan, 2015/02/18 21:18:05]

I think there are some browser types this won't work on: android devices, cros
devices, trybots, single tab browsers. Should assert that this is a browser the
code will work on here.

[erikchen, 2015/02/18 22:04:01]

I added asserts for "supports_tab_control" and os is mac, windows or linux. I
didn't see a reason to prevent this from running on try-bots.

+
+    while(len(self._browser.tabs) < self._NUM_TABS):
+      self._browser.tabs.New()
+
+  def TearDown(self):
+    """Teardown that is guaranteed to be executed before the instance is
+    destroyed.
+
+    Can be overridden by subclasses. Subclasses must call the super class
+    implementation.
+    """
+    if self._browser:
+      self._browser.Close()
+      self._browser = None
+
+  def FinishedNavigationOfBatch(self):

[nednguyen, 2015/02/18 22:43:14]

CleanUpAfterBatchNavigation

[erikchen, 2015/02/18 23:32:30]

Done.

+    """A hook for subclasses to perform cleanup after each batch of
+    navigations.
+
+    Can be overridden by subclasses.
+    """
+    pass
 
   def _GetPossibleBrowser(self, finder_options):
     """Return a possible_browser with the given options."""
@@ -162,40 +212,26 @@ class FastNavigationProfileExtender(object):
         # Ignore time outs and web page crashes.
         pass
 
-  def _SetUpBrowser(self, finder_options):
-    """Finds the browser, starts the browser, and opens the requisite number of
-    tabs."""
-    possible_browser = self._GetPossibleBrowser(finder_options)
-    self._browser = possible_browser.Create(finder_options)
-
-    for _ in range(self._NUM_TABS):
-      self._browser.tabs.New()
-
   def _PerformNavigations(self):
-    """Performs the navigations specified by |_navigation_urls| in large
-    batches."""
-    # The index of the first url that has not yet been navigated to.
-    navigation_url_index = 0
+    """Repeatedly fetches a batch of urls, and navigates to those urls. This
+    will run until an empty batch is returned, or ShouldExit() returns True.
+    """
     while True:
       # Generate the next batch of navigations.
+      urls = self.GetUrlsToNavigate(self._NUM_TABS)
+      if len(urls) == 0:
+        break
+
       batch = []
-      max_index = min(navigation_url_index + self._NUM_PARALLEL_PAGES,
-          len(self._navigation_urls))
-      for i in range(navigation_url_index, max_index):
-        url = self._navigation_urls[i]
-        tab = self._browser.tabs[i % self._NUM_TABS]
+      for i in range(len(urls)):
+        url = urls[i]
+        tab = self._browser.tabs[i]
         batch.append((tab, url))
-      navigation_url_index = max_index
 
       queued_tabs = self._BatchNavigateTabs(batch)
       self._WaitForQueuedTabsToLoad(queued_tabs)
 
-      if navigation_url_index == len(self._navigation_urls):
-        break
+      self.FinishedNavigationOfBatch()
 
-  def _TearDownBrowser(self):
-    """Teardown that is guaranteed to be executed before the instance is
-    destroyed."""
-    if self._browser:
-      self._browser.Close()
-      self._browser = None
+      if self.ShouldExit():

[nednguyen, 2015/02/18 22:43:14]

Just looking at this method name, it's unclear to me when it's called and what
is the state of the world when it's called.

maybe ShouldExitAfterBatchNavigation()?

[erikchen, 2015/02/18 23:32:30]

Done.

+        break