Bug 1636886 - Only download conditioned-profiles once in Raptor. r=tarek,perftest-reviewers,Bebe
authorGregory Mierzwinski <gmierz2@outlook.com>
Thu, 14 May 2020 11:20:42 +0000
changeset 593604 4dd20d7e4ed3864c31e5182d458b4d66012ca228
parent 593603 9685e26fe77d10bbb01174b755c4698b9be161f8
child 593605 80a0ea17c9f8c6db0b01918d5d3e7b2f4b90c3be
push id13186
push userffxbld-merge
push dateMon, 01 Jun 2020 09:52:46 +0000
treeherdermozilla-beta@3e7c70a1e4a1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstarek, perftest-reviewers, Bebe
bugs1636886
milestone78.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1636886 - Only download conditioned-profiles once in Raptor. r=tarek,perftest-reviewers,Bebe This patch makes it so that we only download and extract the conditioned-profile once and have tests use copies of that original copy. It also adds some debugging logs to the condprof `download_file` function to debug an issue where we seem to hit a cache-miss when we have already downloaded the artifact. Differential Revision: https://phabricator.services.mozilla.com/D75176
testing/condprofile/condprof/util.py
testing/raptor/raptor/perftest.py
--- a/testing/condprofile/condprof/util.py
+++ b/testing/condprofile/condprof/util.py
@@ -197,29 +197,41 @@ def download_file(url, target=None):
     if not present:
         logger.info("Cannot find %r" % url)
         raise ArchiveNotFound(url)
 
     etag = headers.get("ETag")
     if target is None:
         target = url.split("/")[-1]
 
+    logger.info("Checking for existence of: %s" % target)
     if os.path.exists(target):
         # XXX for now, reusing downloads without checking them
         # when we don't have an .etag file
         if etag is None or not os.path.exists(target + ".etag"):
+            logger.info("No existing etag downloads.")
             return target
         with open(target + ".etag") as f:
             current_etag = f.read()
         if etag == current_etag:
             logger.info("Already Downloaded.")
             # should at least check the size?
             return target
         else:
             logger.info("Changed!")
+    else:
+        logger.info("Could not find an existing archive.")
+        # Add some debugging logs for the directory content
+        try:
+            archivedir = os.path.dirname(target)
+            logger.info(
+                "Content in cache directory %s: %s" % (archivedir, os.listdir(archivedir))
+            )
+        except Exception:
+            logger.info("Failed to list cache directory contents")
 
     logger.info("Downloading %s" % url)
     req = requests.get(url, stream=True, timeout=DOWNLOAD_TIMEOUT)
     total_length = int(req.headers.get("content-length"))
     target_dir = os.path.dirname(target)
     if target_dir != "" and not os.path.exists(target_dir):
         logger.info("Creating dir %s" % target_dir)
         os.makedirs(target_dir)
--- a/testing/raptor/raptor/perftest.py
+++ b/testing/raptor/raptor/perftest.py
@@ -206,19 +206,31 @@ either Raptor or browsertime."""
         tempdir = tempfile.mkdtemp()
         self._dirs_to_remove.append(tempdir)
         return tempdir
 
     @property
     def is_localhost(self):
         return self.config.get("host") in ("localhost", "127.0.0.1")
 
+    @property
+    def conditioned_profile_copy(self):
+        """Returns a copy of the original conditioned profile that was created."""
+        condprof_copy = os.path.join(self._get_temp_dir(), "profile")
+        shutil.copytree(self.conditioned_profile_dir, condprof_copy)
+        LOG.info("Created a conditioned-profile copy: %s" % condprof_copy)
+        return condprof_copy
+
     def get_conditioned_profile(self):
         """Downloads a platform-specific conditioned profile, using the
         condprofile client API; returns a self.conditioned_profile_dir"""
+        if self.conditioned_profile_dir:
+            # We already have a directory, so provide a copy that
+            # will get deleted after it's done with
+            return self.conditioned_profile_copy
 
         # create a temp file to help ensure uniqueness
         temp_download_dir = self._get_temp_dir()
         LOG.info(
             "Making temp_download_dir from inside get_conditioned_profile {}".format(
                 temp_download_dir
             )
         )
@@ -280,30 +292,29 @@ either Raptor or browsertime."""
                     temp_download_dir,
                     platform,
                     profile_scenario
                 )
             )
             raise OSError
 
         LOG.info(
-            "self.conditioned_profile_dir is now set: {}".format(
+            "Original self.conditioned_profile_dir is now set: {}".format(
                 self.conditioned_profile_dir
             )
         )
-        return self.conditioned_profile_dir
+        return self.conditioned_profile_copy
 
     def build_browser_profile(self):
         if not self.using_condprof or self.config['app'] in ['chrome', 'chromium', 'chrome-m']:
             self.profile = create_profile(self.profile_class)
         else:
-            self.get_conditioned_profile()
             # use mozprofile to create a profile for us, from our conditioned profile's path
             self.profile = create_profile(
-                self.profile_class, profile=self.conditioned_profile_dir
+                self.profile_class, profile=self.get_conditioned_profile()
             )
         # Merge extra profile data from testing/profiles
         with open(os.path.join(self.profile_data_dir, "profiles.json"), "r") as fh:
             base_profiles = json.load(fh)["raptor"]
 
         for profile in base_profiles:
             path = os.path.join(self.profile_data_dir, profile)
             LOG.info("Merging profile: {}".format(path))