From d5ad066d2d028fb3fee02f6ffa908718c10923d2 Mon Sep 17 00:00:00 2001 From: Karl Simonsen Date: Fri, 8 Apr 2022 18:34:27 -0700 Subject: [PATCH] AIBench: runBinaryBenchmark(): Don't give critical logging error for known user errors. (#487) Summary: Pull Request resolved: https://github.com/facebook/FAI-PEP/pull/487 Perfetto should not give a critical error for know errors. Leave any critical logging to the place that first sees the exception. Reviewed By: MarkAndersonIX Differential Revision: D35303625 fbshipit-source-id: 5f9886400462681c0d6dd3bd3dd15aeff5db6354 --- .../platforms/android/android_platform.py | 66 +++++++++++-------- benchmarking/profilers/perfetto/perfetto.py | 55 ++++++++-------- benchmarking/run_lab.py | 1 - benchmarking/utils/utilities.py | 16 ++++- 4 files changed, 79 insertions(+), 59 deletions(-) diff --git a/benchmarking/platforms/android/android_platform.py b/benchmarking/platforms/android/android_platform.py index 288b5c95..69bb0735 100644 --- a/benchmarking/platforms/android/android_platform.py +++ b/benchmarking/platforms/android/android_platform.py @@ -21,7 +21,12 @@ from profilers.profilers import getProfilerByUsage from six import string_types from utils.custom_logger import getLogger -from utils.utilities import getRunStatus, setRunStatus +from utils.utilities import ( + getRunStatus, + setRunStatus, + BenchmarkArgParseException, + BenchmarkUnsupportedDeviceException, +) class AndroidPlatform(PlatformBase): @@ -48,6 +53,7 @@ def __init__(self, tempdir, adb, args, usb_controller=None): self.type = "android" self.setPlatform(platform) self.setPlatformHash(adb.device) + self.device_label = self.getMangledName() self.usb_controller = usb_controller self._setLogCatSize() self.app = None @@ -242,39 +248,41 @@ def runBinaryBenchmark(self, cmd, *args, **kwargs): if enable_profiling: profiler = platform_args["profiling_args"]["profiler"] profiling_types = platform_args["profiling_args"]["types"] - if profiler == "simpleperf": - assert profiling_types == [ - "cpu" - ], "Only cpu profiling is supported for SimplePerf" - try: + profiler_exception_message = f"An error has occurred when running {profiler} profiler on device {self.device_label}." + try: + if profiler == "simpleperf": + assert profiling_types == [ + "cpu" + ], "Only cpu profiling is supported for SimplePerf" # attempt to run with cpu profiling, else fallback to standard run return self._runBenchmarkWithSimpleperf( cmd, log_to_screen_only, **platform_args ) - except Exception: - # if this has not succeeded for some reason reset run status and run without profiling. - getLogger().critical( - f"An error has occurred when running Simpleperf profiler on device {self.platform} {self.platform_hash}.", - exc_info=True, - ) - elif profiler == "perfetto": - assert ( - "cpu" not in profiling_types - ), "cpu profiling is not yet implemented for Perfetto" - try: - # attempt Perfetto profiling + elif profiler == "perfetto": + assert ( + "cpu" not in profiling_types + ), "cpu profiling is not yet implemented for Perfetto" + # attempt Perfetto profiling, else fallback to standard run return self._runBenchmarkWithPerfetto( cmd, log_to_screen_only, **platform_args ) - except Exception: - # if this has not succeeded for some reason reset run status and run without profiling. - getLogger().critical( - f"An error has occurred when running Perfetto profiler on device {self.platform} {self.platform_hash}.", - exc_info=True, + else: + raise BenchmarkArgParseException( + f"Ignoring unsupported profiler setting: {profiler}: {profiling_types}.", ) - else: - getLogger().error( - f"Ignoring unsupported profiler setting: {profiler}: {profiling_types}.", + except BenchmarkUnsupportedDeviceException: + getLogger().exception( + profiler_exception_message, + ) + except BenchmarkArgParseException: + getLogger().exception( + "An error occurred while parsing profiler arguments.", + ) + except Exception: + # if this has not succeeded for some reason reset run status and run without profiling. + getLogger().critical( + profiler_exception_message, + exc_info=True, ) # Run without profiling @@ -310,8 +318,8 @@ def _runBenchmarkWithSimpleperf( def _runBenchmarkWithPerfetto(self, cmd, log_to_screen_only: bool, **platform_args): # attempt Perfetto profiling if not self.util.isRootedDevice(silent=True): - raise RuntimeError( - "Attempted to perform Perfetto profiling on unrooted device {self.util.device}." + raise BenchmarkUnsupportedDeviceException( + f"Attempted to perform perfetto profiling on unrooted device {self.device_label}." ) with Perfetto( @@ -371,7 +379,7 @@ def currentPower(self): return int(result_line.split(": ")[-1]) except Exception: getLogger().critical( - f"Could not read battery level for device {self.platform} {self.platform_hash}", + f"Could not read battery level for device {self.device_label}.", exc_info=True, ) return -1 diff --git a/benchmarking/profilers/perfetto/perfetto.py b/benchmarking/profilers/perfetto/perfetto.py index 69c881a2..14b413ae 100644 --- a/benchmarking/profilers/perfetto/perfetto.py +++ b/benchmarking/profilers/perfetto/perfetto.py @@ -28,6 +28,7 @@ from profilers.profiler_base import ProfilerBase from profilers.utilities import generate_perf_filename, upload_profiling_reports from utils.custom_logger import getLogger +from utils.utilities import BenchmarkUnsupportedDeviceException PROCESS_KEY = "perfetto" @@ -73,14 +74,14 @@ def __init__( self.options = options or {} self.android_version: int = int(platform.rel_version.split(".")[0]) self.adb = platform.util - self.valid = True + self.valid = False self.perfetto_pid = None self.all_heaps = ( f"all_heaps: {self.options.get('all_heaps', 'false')}" if self.android_version >= 12 else "" ) - self.basename = generate_perf_filename(model_name, self.adb.device) + self.basename = generate_perf_filename(model_name, self.platform.platform_hash) self.trace_file_name = f"{self.basename}.perfetto-trace" self.trace_file_device = f"{self.DEVICE_DIRECTORY}/{self.trace_file_name}" self.config_file = f"{self.basename}.{self.CONFIG_FILE}" @@ -124,22 +125,20 @@ def __exit__(self, type, value, traceback): def _start(self): """Begin Perfetto profiling on platform.""" - try: - if self.android_version < 10: - getLogger().error( - f"Attempt to run Perfetto on {self.platform.type} {self.platform.rel_version} device {self.adb.device} ignored." - ) - self.valid = False - return None - - if not self.is_rooted_device: - getLogger().error( - f"Attempt to run Perfetto on unrooted device {self.adb.device} ignored." - ) - self.valid = False - return None + self.valid = False + if self.android_version < 10: + raise BenchmarkUnsupportedDeviceException( + f"Attempt to run perfetto on {self.platform.type} {self.platform.rel_version} device {self.platform.device_label} ignored." + ) + if not self.is_rooted_device: + raise BenchmarkUnsupportedDeviceException( + f"Attempt to run perfetto on unrooted device {self.platform.device_label} ignored." + ) - getLogger().info(f"Collect Perfetto data on device {self.adb.device}") + try: + getLogger().info( + f"Collect perfetto data on device {self.platform.device_label}." + ) self._enablePerfetto() # Generate and upload custom config file @@ -155,13 +154,15 @@ def _start(self): # call Perfetto output = self._perfetto() - if output != 1 and output[0] != "1": - self.perfetto_pid = output[0] + except Exception as e: + raise RuntimeError(f"Perfetto profiling failed to start:\n{e}.") + else: + if output == 1 or output == [] or output[0] == "1": + raise RuntimeError("Perfetto profiling could not be started.") + + self.perfetto_pid = output[0] + self.valid = True return output - except Exception: - self.valid = False - getLogger().exception("Perfetto profiling could not be started.") - return None def getResults(self): if self.valid: @@ -181,7 +182,7 @@ def _finish(self): # if we ran perfetto, signal it to stop profiling if self._signalPerfetto(): getLogger().info( - f"Looking for Perfetto data on device {self.adb.device}" + f"Looking for Perfetto data on device {self.platform.device_label}." ) self._copyPerfDataToHost() self._generateReport() @@ -236,7 +237,7 @@ def _restoreState(self): def _signalPerfetto(self) -> bool: # signal perfetto to stop profiling and await results - getLogger().info("Stopping Perfetto profiling.") + getLogger().info("Stopping perfetto profiling.") result = None if self.perfetto_pid is not None: sigint_cmd = [ @@ -290,7 +291,7 @@ def _enablePerfetto(self): retry=1, ) - # Enable Perfetto if not enabled yet. + # Enable Perfetto if not yet enabled. getprop_tracing_enabled = self.adb.getprop( self.TRACING_PROPERTY, default=["0"], @@ -390,7 +391,7 @@ def _setup_perfetto_config( def _perfetto(self): """Run perfetto on platform with benchmark process id.""" - getLogger().info(f"Calling Perfetto: {self.perfetto_cmd}") + getLogger().info(f"Calling perfetto: {self.perfetto_cmd}") output = self.platform.util.shell(self.perfetto_cmd) getLogger().info(f"Perfetto returned: {output}.") startup_time: float = 2.0 if self.all_heaps != "false" else 0.2 diff --git a/benchmarking/run_lab.py b/benchmarking/run_lab.py index 360dae55..e17cd07e 100644 --- a/benchmarking/run_lab.py +++ b/benchmarking/run_lab.py @@ -24,7 +24,6 @@ import signal import stat import tempfile -import threading import time from concurrent.futures import ProcessPoolExecutor as Pool from io import StringIO diff --git a/benchmarking/utils/utilities.py b/benchmarking/utils/utilities.py index 2387ad7f..310ec9de 100644 --- a/benchmarking/utils/utilities.py +++ b/benchmarking/utils/utilities.py @@ -39,18 +39,30 @@ EXTERNAL_STATUS_MASK = 0xFF -class DownloadException(Exception): +class BenchmarkException(Exception): + """Base class for all benchmark exceptions.""" + + pass + + +class DownloadException(BenchmarkException): """Raised where exception occurs when downloading benchmark files.""" pass -class BenchmarkArgParseException(Exception): +class BenchmarkArgParseException(BenchmarkException): """Raised where benchmark arguments could not be parsed or are invalid.""" pass +class BenchmarkUnsupportedDeviceException(BenchmarkException): + """Raised where benchmark arguments specify an invalid device.""" + + pass + + def check_is_json(json_str): try: json.loads(json_str)