build_manager: do not fetch fuzz_targets when not necessary (#4393)

For non engine fuzzers, we do not need to have the full list of fuzz
targets, since those builds are only containing the APP_NAME. For that
reason, and when the build is fully unpacked, lazily fetch the fuzzing
targets only when requested by the user of the class.

This change will tremendously speed up the unpacking step for some of
our fuzzers, see
https://docs.google.com/document/d/1OepfVcuG2XNXLxgZIXVwgE-uOhX9RO5RfH0TSg_wmPU/.

Co-authored-by: jonathanmetzman <[email protected]>

Author: paulsemel

src/clusterfuzz/_internal/bot/untrusted_runner/build_setup_host.py

@@ -31,25 +31,6 @@ def _clear_env():
   environment.remove_key('FUZZ_TARGET')
 
 
-def _handle_response(build, response):
-  """Handle build setup response."""
-  if not response.result:
-    _clear_env()
-    return False
-
-  _update_env_from_response(response)
-
-  if not environment.get_value('APP_PATH'):
-    fuzzer_directory = environment.get_value('FUZZER_DIR')
-    if fuzzer_directory:
-      build_manager.set_environment_vars([fuzzer_directory])
-
-  environment.set_value('APP_REVISION', build.revision)
-  build.fuzz_targets = list(response.fuzz_targets)
-
-  return True
-
-
 def _update_env_from_response(response):
   """Update environment variables from response."""
   environment.set_value('APP_PATH', response.app_path)
@@ -64,11 +45,37 @@ def _update_env_from_response(response):
 class RemoteRegularBuild(build_manager.RegularBuild):
   """Remote regular build."""
 
+  def __init__(self, *args, **kwargs):
+    super().__init__(*args, **kwargs)
+    self._fuzz_targets = []
+
+  def _handle_response(self, response):
+    """Handle build setup response."""
+    if not response.result:
+      _clear_env()
+      return False
+
+    _update_env_from_response(response)
+
+    if not environment.get_value('APP_PATH'):
+      fuzzer_directory = environment.get_value('FUZZER_DIR')
+      if fuzzer_directory:
+        build_manager.set_environment_vars([fuzzer_directory])
+
+    environment.set_value('APP_REVISION', self.revision)
+    self._fuzz_targets = list(response.fuzz_targets)
+
+    return True
+
   def setup(self):
     request = untrusted_runner_pb2.SetupRegularBuildRequest(  # pylint: disable=no-member
         base_build_dir=self.base_build_dir,
         revision=self.revision,
         build_url=self.build_url,
         fuzz_target=self.fuzz_target,
         build_prefix=self.build_prefix)
-    return _handle_response(self, host.stub().SetupRegularBuild(request))
+    return self._handle_response(host.stub().SetupRegularBuild(request))
+
+  @property
+  def fuzz_targets(self):
+    return self._fuzz_targets

src/clusterfuzz/_internal/build_management/build_manager.py

@@ -330,7 +330,9 @@ def __init__(self,
     self.build_prefix = build_prefix
     self.env_prefix = build_prefix + '_' if build_prefix else ''
     # This is used by users of the class to learn the fuzz targets in the build.
-    self.fuzz_targets = None
+    self._fuzz_targets = None
+    self._unpack_everything = environment.get_value(
+        'UNPACK_ALL_FUZZ_TARGETS_AND_FILES', default_value=False)
     # This is used by users of the class to instruct the class which fuzz
     # target to unpack.
     self.fuzz_target = fuzz_target
@@ -457,15 +459,12 @@ def _download_and_open_build_archive(self, base_build_dir: str,
       shell.remove_file(build_local_archive)
 
   def _open_build_archive(self, base_build_dir: str, build_dir: str,
-                          build_url: str, http_build_url: Optional[str],
-                          unpack_everything: Optional[bool]):
+                          build_url: str, http_build_url: Optional[str]):
     """Gets a handle on a build archive for the current build. Depending on the
     provided parameters, this function might download the build archive into
     the build directory or directly use remote HTTP archive.
 
     Args:
-        unpack_everything: wether we should unpack the whole archive or try
-        selective unpacking.
         base_build_dir: the base build directory.
         build_dir: the current build directory.
         build_url: the build URL.
@@ -483,7 +482,8 @@ def _open_build_archive(self, base_build_dir: str, build_dir: str,
     allow_unpack_over_http = environment.get_value(
         'ALLOW_UNPACK_OVER_HTTP', default_value=False)
     can_unzip_over_http = (
-        allow_unpack_over_http and not unpack_everything and http_build_url and
+        allow_unpack_over_http and not self._unpack_everything and
+        http_build_url and
         build_archive.unzip_over_http_compatible(http_build_url))
 
     if not can_unzip_over_http:
@@ -503,9 +503,6 @@ def _unpack_build(self,
     # Track time taken to unpack builds so that it doesn't silently regress.
     start_time = time.time()
 
-    unpack_everything = environment.get_value(
-        'UNPACK_ALL_FUZZ_TARGETS_AND_FILES')
-
     logs.info(f'Unpacking build from {build_url} into {build_dir}.')
 
     # Free up memory.
@@ -520,15 +517,12 @@ def _unpack_build(self,
 
     try:
       with self._open_build_archive(base_build_dir, build_dir, build_url,
-                                    http_build_url, unpack_everything) as build:
+                                    http_build_url) as build:
         unpack_start_time = time.time()
-        unpack_everything = environment.get_value(
-            'UNPACK_ALL_FUZZ_TARGETS_AND_FILES')
-
-        if not unpack_everything:
+        if not self._unpack_everything:
           # We will never unpack the full build so we need to get the targets
           # from the build archive.
-          self.fuzz_targets = list(build.list_fuzz_targets())
+          self._fuzz_targets = list(build.list_fuzz_targets())
           # We only want to unpack a single fuzz target if unpack_everything is
           # False.
           fuzz_target_to_unpack = self.fuzz_target
@@ -567,9 +561,7 @@ def _unpack_build(self,
       logs.error(f'Unable to unpack build archive {build_url}: {e}')
       return False
 
-    if unpack_everything:
-      self.fuzz_targets = list(self._get_fuzz_targets_from_dir(build_dir))
-    else:
+    if not self._unpack_everything:
       # If this is partial build due to selected build files, then mark it as
       # such so that it is not re-used.
       partial_build_file_path = os.path.join(build_dir, PARTIAL_BUILD_FILE)
@@ -601,6 +593,14 @@ def build_dir(self):
     """The build directory. Usually a subdirectory of base_build_dir."""
     raise NotImplementedError
 
+  @property
+  def fuzz_targets(self):
+    if not self._fuzz_targets and self._unpack_everything:
+      # we can lazily compute that when unpacking the whole archive, since we
+      # know all the fuzzers will be in the build directory.
+      self._fuzz_targets = list(self._get_fuzz_targets_from_dir(self.build_dir))
+    return self._fuzz_targets
+
   def exists(self):
     """Check if build already exists."""
     revision_file = os.path.join(self.build_dir, REVISION_FILE_NAME)
@@ -741,8 +741,8 @@ def setup(self):
       # This list will be incomplete because the directory on disk does not have
       # all fuzz targets. This is fine. The way fuzz_targets are added to db, it
       # does not clobber complete lists.
-      assert self.fuzz_targets is None
-      self.fuzz_targets = list(self._get_fuzz_targets_from_dir(self.build_dir))
+      assert self._fuzz_targets is None
+      self._fuzz_targets = list(self._get_fuzz_targets_from_dir(self.build_dir))
 
     self._setup_application_path(build_update=build_update)
     self._post_setup_success(update_revision=build_update)
@@ -754,7 +754,7 @@ class SplitTargetBuild(RegularBuild):
 
   def setup(self, *args, **kwargs):
     result = super().setup(*args, **kwargs)
-    self.fuzz_targets = list(_split_target_build_list_targets())
+    self._fuzz_targets = list(_split_target_build_list_targets())
     return result
 
 
@@ -793,7 +793,7 @@ def setup(self):
 
     # Select a fuzzer, now that a list is available
     fuzz_targets = fuchsia.undercoat.list_fuzzers(handle)
-    self.fuzz_targets = list(fuzz_targets)
+    self._fuzz_targets = list(fuzz_targets)
     return True
 
 
@@ -972,7 +972,7 @@ def setup(self):
     else:
       logs.info('Build already exists.')
 
-    self.fuzz_targets = list(self._get_fuzz_targets_from_dir(self.build_dir))
+    self._fuzz_targets = list(self._get_fuzz_targets_from_dir(self.build_dir))
     self._setup_application_path(build_update=build_update)
     self._post_setup_success(update_revision=build_update)
     return True