Bug 1362148 - Extract package metrics collection to own function; r?ted draft
authorGregory Szorc <gps@mozilla.com>
Fri, 12 May 2017 13:18:51 -0700
changeset 589310 80cf1917a0b910e0867c7707091a5c67c42e519c
parent 589309 c3c28426468474a7aa51a10787d01ebbba10dd82
child 589311 b4719543ffdeba0a612f5067719140e05fb44899
push id62318
push userbmo:gps@mozilla.com
push dateTue, 06 Jun 2017 01:11:27 +0000
reviewersted
bugs1362148
milestone55.0a1
Bug 1362148 - Extract package metrics collection to own function; r?ted A subsequent commit will make all this code conditional. Rather than indent the world, it is easier to conditionally call a function. A benefit of the new code is that we skip some code for debug builds, which is one less thing that can break. MozReview-Commit-ID: fiUNBbikmy
testing/mozharness/mozharness/mozilla/building/buildbase.py
--- a/testing/mozharness/mozharness/mozilla/building/buildbase.py
+++ b/testing/mozharness/mozharness/mozilla/building/buildbase.py
@@ -1913,38 +1913,19 @@ or run without that action (ie: --no-{ac
         yield {
             'name': 'sccache requests_not_cacheable',
             'value': stats['stats']['requests_not_cacheable'],
             'extraOptions': self.perfherder_resource_options(),
             'alertThreshold': 50.0,
             'subtests': [],
         }
 
-    def generate_build_stats(self):
-        """grab build stats following a compile.
-
-        This action handles all statistics from a build: 'count_ctors'
-        and then posts to graph server the results.
-        We only post to graph server for non nightly build
-        """
-        if self.config.get('forced_artifact_build'):
-            self.info('Skipping due to forced artifact build.')
-            return
-
+    def _get_package_metrics(self):
         import tarfile
         import zipfile
-        c = self.config
-
-        if c.get('enable_count_ctors'):
-            self.info("counting ctors...")
-            self._count_ctors()
-        else:
-            self.info("ctors counts are disabled for this build.")
-
-        # Report some important file sizes for display in treeherder
 
         dirs = self.query_abs_dirs()
         packageName = self.query_buildbot_property('packageFilename')
 
         # if packageName is not set because we are not running in Buildbot,
         # then assume we are using MOZ_SIMPLE_PACKAGE_NAME, which means the
         # package is named one of target.{tar.bz2,zip,dmg}.
         if not packageName:
@@ -1989,45 +1970,73 @@ or run without that action (ie: --no-{ac
                                 if name in subtests:
                                     # File seen twice in same archive;
                                     # ignore to avoid confusion.
                                     subtests[name] = None
                                 else:
                                     subtests[name] = size
                 for name in subtests:
                     if subtests[name] is not None:
-                        self.info('TinderboxPrint: Size of %s<br/>%s bytes\n' % (
-                            name, subtests[name]))
-                        size_measurements.append({'name': name, 'value': subtests[name]})
+                        self.info('TinderboxPrint: Size of %s<br/>%s bytes\n' %
+                                  (name, subtests[name]))
+                        size_measurements.append(
+                            {'name': name, 'value': subtests[name]})
             except:
                 self.info('Unable to search %s for component sizes.' % installer)
                 size_measurements = []
 
+        if not installer_size and not size_measurements:
+            return
+
+        if installer.endswith('.apk'): # Android
+            yield {
+                "name": "installer size",
+                "value": installer_size,
+                "alertChangeType": "absolute",
+                "alertThreshold": (200 * 1024),
+                "subtests": size_measurements
+            }
+        else:
+            yield {
+                "name": "installer size",
+                "value": installer_size,
+                "alertThreshold": 1.0,
+                "subtests": size_measurements
+            }
+
+    def generate_build_stats(self):
+        """grab build stats following a compile.
+
+        This action handles all statistics from a build: 'count_ctors'
+        and then posts to graph server the results.
+        We only post to graph server for non nightly build
+        """
+        if self.config.get('forced_artifact_build'):
+            self.info('Skipping due to forced artifact build.')
+            return
+
+        c = self.config
+
+        if c.get('enable_count_ctors'):
+            self.info("counting ctors...")
+            self._count_ctors()
+        else:
+            self.info("ctors counts are disabled for this build.")
+
+        # Report some important file sizes for display in treeherder
+
         perfherder_data = {
             "framework": {
                 "name": "build_metrics"
             },
             "suites": [],
         }
-        if (installer_size or size_measurements) and not c.get('debug_build'):
-            if installer.endswith('.apk'): # Android
-                perfherder_data["suites"].append({
-                    "name": "installer size",
-                    "value": installer_size,
-                    "alertChangeType": "absolute",
-                    "alertThreshold": (200 * 1024),
-                    "subtests": size_measurements
-                })
-            else:
-                perfherder_data["suites"].append({
-                    "name": "installer size",
-                    "value": installer_size,
-                    "alertThreshold": 1.0,
-                    "subtests": size_measurements
-                })
+
+        if not c.get('debug_build'):
+            perfherder_data['suites'].extend(self._get_package_metrics())
 
         # Extract compiler warnings count.
         warnings = self.get_output_from_command(
             command=[sys.executable, 'mach', 'warnings-list'],
             cwd=self.query_abs_dirs()['abs_src_dir'],
             env=self.query_build_env(),
             # No need to pollute the log.
             silent=True,