crosperf: Add option to not download debug symbols

This patch introduces a new option --download_debug to crosperf.
We will only download debug symbols when this option is set to True.
This will save space when perf test users doesn't need it.

TEST=passed unittests.
BUG=chromium:1010294

Change-Id: Ic80958377f1c25cb04e3b7b56b40fafcada6d2d8
Reviewed-on: https://chromium-review.googlesource.com/1865958
Tested-by: Zhizhou Yang <zhizhouy@google.com>
Commit-Ready: Zhizhou Yang <zhizhouy@google.com>
Legacy-Commit-Queue: Commit Bot <commit-bot@chromium.org>
Reviewed-by: Denis Nikitin <denik@chromium.org>

7 files changed, 25 insertions, 12 deletions

diff --git a/crosperf/crosperf_unittest.py b/crosperf/crosperf_unittest.py
index 4f40d139..faf1badf 100755
--- a/crosperf/crosperf_unittest.py
+++ b/crosperf/crosperf_unittest.py
@@ -69,7 +69,7 @@ class CrosperfTest(unittest.TestCase):
     settings = crosperf.ConvertOptionsToSettings(options)
     self.assertIsNotNone(settings)
     self.assertIsInstance(settings, settings_factory.GlobalSettings)
-    self.assertEqual(len(settings.fields), 37)
+    self.assertEqual(len(settings.fields), 38)
     self.assertTrue(settings.GetField('rerun'))
     argv = ['crosperf/crosperf.py', 'temp.exp']
     options, _ = parser.parse_known_args(argv)
diff --git a/crosperf/download_images.py b/crosperf/download_images.py
index e02c5817..f855cb60 100644
--- a/crosperf/download_images.py
+++ b/crosperf/download_images.py
@@ -293,7 +293,7 @@ class ImageDownloader(object):
     return debug_rel_path
 
   def Run(self, chromeos_root, xbuddy_label, autotest_path, debug_path,
-          perf_args):
+          download_debug):
     build_id = self.GetBuildID(chromeos_root, xbuddy_label)
     image_name = ('gs://chromeos-image-archive/%s/chromiumos_test_image.tar.xz'
                   % build_id)
@@ -317,7 +317,7 @@ class ImageDownloader(object):
     if autotest_path == '':
       autotest_path = self.DownloadAutotestFiles(chromeos_root, build_id)
 
-    if debug_path == '' and perf_args:
+    if debug_path == '' and download_debug:
       debug_path = self.DownloadDebugFile(chromeos_root, build_id)
 
     return image_path, autotest_path, debug_path
diff --git a/crosperf/download_images_unittest.py b/crosperf/download_images_unittest.py
index 68a84676..dd83c3fa 100755
--- a/crosperf/download_images_unittest.py
+++ b/crosperf/download_images_unittest.py
@@ -167,7 +167,7 @@ class ImageDownloaderTestcast(unittest.TestCase):
     test_empty_debug_path = ''
     test_autotest_path = '/tmp/autotest'
     test_debug_path = '/tmp/debug'
-    perf_args = '-a'
+    download_debug = True
 
     # Set values to test/check.
     self.called_download_image = False
@@ -224,7 +224,7 @@ class ImageDownloaderTestcast(unittest.TestCase):
     # Call Run.
     image_path, autotest_path, debug_path = downloader.Run(
         test_chroot, test_build_id, test_empty_autotest_path,
-        test_empty_debug_path, perf_args)
+        test_empty_debug_path, download_debug)
 
     # Make sure it called both _DownloadImage and _UncompressImage
     self.assertTrue(self.called_download_image)
@@ -244,7 +244,7 @@ class ImageDownloaderTestcast(unittest.TestCase):
 
     image_path, autotest_path, debug_path = downloader.Run(
         test_chroot, test_build_id, test_autotest_path, test_debug_path,
-        perf_args)
+        download_debug)
 
     # Verify that downloadAutotestFiles was not called
     self.assertFalse(self.called_download_autotest_files)
@@ -263,7 +263,7 @@ class ImageDownloaderTestcast(unittest.TestCase):
     # Call Run again.
     self.assertRaises(download_images.MissingImage, downloader.Run, test_chroot,
                       test_autotest_path, test_debug_path, test_build_id,
-                      perf_args)
+                      download_debug)
 
     # Verify that UncompressImage and downloadAutotestFiles were not called,
     # since _DownloadImage "failed"
diff --git a/crosperf/experiment_factory.py b/crosperf/experiment_factory.py
index 5b4d4b0d..1758f0c7 100644
--- a/crosperf/experiment_factory.py
+++ b/crosperf/experiment_factory.py
@@ -137,6 +137,10 @@ class ExperimentFactory(object):
     remote = new_remote
     rm_chroot_tmp = global_settings.GetField('rm_chroot_tmp')
     perf_args = global_settings.GetField('perf_args')
+    download_debug = global_settings.GetField('download_debug')
+    # Do not download debug symbols when perf_args is not specified.
+    if not perf_args and download_debug:
+      download_debug = False
     acquire_timeout = global_settings.GetField('acquire_timeout')
     cache_dir = global_settings.GetField('cache_dir')
     cache_only = global_settings.GetField('cache_only')
@@ -433,7 +437,7 @@ class ExperimentFactory(object):
           raise RuntimeError("Can not have empty 'build' field!")
         image, autotest_path, debug_path = label_settings.GetXbuddyPath(
             build, autotest_path, debug_path, board, chromeos_root, log_level,
-            perf_args)
+            download_debug)
 
       cache_dir = label_settings.GetField('cache_dir')
       chrome_src = label_settings.GetField('chrome_src')
diff --git a/crosperf/settings.py b/crosperf/settings.py
index 290abfc2..708697b1 100644
--- a/crosperf/settings.py
+++ b/crosperf/settings.py
@@ -71,7 +71,7 @@ class Settings(object):
         raise SyntaxError('Field %s is invalid.' % name)
 
   def GetXbuddyPath(self, path_str, autotest_path, debug_path, board,
-                    chromeos_root, log_level, perf_args):
+                    chromeos_root, log_level, download_debug):
     prefix = 'remote'
     l = logger.GetLogger()
     if (path_str.find('trybot') < 0 and path_str.find('toolchain') < 0 and
@@ -83,4 +83,4 @@ class Settings(object):
     # Returns three variables: image, autotest_path, debug_path
     return image_downloader.Run(
         misc.CanonicalizePath(chromeos_root), xbuddy_path, autotest_path,
-        debug_path, perf_args)
+        debug_path, download_debug)
diff --git a/crosperf/settings_factory.py b/crosperf/settings_factory.py
index 9057703f..1993a6c3 100644
--- a/crosperf/settings_factory.py
+++ b/crosperf/settings_factory.py
@@ -241,6 +241,14 @@ class GlobalSettings(Settings):
             'related counters. It must start with perf '
             'command record or stat followed by arguments.'))
     self.AddField(
+        BooleanField(
+            'download_debug',
+            default=True,
+            description='Download compressed debug symbols alongwith '
+            'image. This can provide more info matching symbols for'
+            'profiles, but takes larger space. By default, download'
+            'it only when perf_args is specified.'))
+    self.AddField(
         TextField(
             'cache_dir',
             default='',
diff --git a/crosperf/settings_factory_unittest.py b/crosperf/settings_factory_unittest.py
index d80dbb12..a303a065 100755
--- a/crosperf/settings_factory_unittest.py
+++ b/crosperf/settings_factory_unittest.py
@@ -50,7 +50,7 @@ class GlobalSettingsTest(unittest.TestCase):
   def test_init(self):
     res = settings_factory.GlobalSettings('g_settings')
     self.assertIsNotNone(res)
-    self.assertEqual(len(res.fields), 37)
+    self.assertEqual(len(res.fields), 38)
     self.assertEqual(res.GetField('name'), '')
     self.assertEqual(res.GetField('board'), '')
     self.assertEqual(res.GetField('skylab'), False)
@@ -66,6 +66,7 @@ class GlobalSettingsTest(unittest.TestCase):
     self.assertEqual(res.GetField('logging_level'), 'average')
     self.assertEqual(res.GetField('acquire_timeout'), 0)
     self.assertEqual(res.GetField('perf_args'), '')
+    self.assertEqual(res.GetField('download_debug'), True)
     self.assertEqual(res.GetField('cache_dir'), '')
     self.assertEqual(res.GetField('cache_only'), False)
     self.assertEqual(res.GetField('no_email'), False)
@@ -106,7 +107,7 @@ class SettingsFactoryTest(unittest.TestCase):
     g_settings = settings_factory.SettingsFactory().GetSettings(
         'global', 'global')
     self.assertIsInstance(g_settings, settings_factory.GlobalSettings)
-    self.assertEqual(len(g_settings.fields), 37)
+    self.assertEqual(len(g_settings.fields), 38)
 
 
 if __name__ == '__main__':