From 731c201e7f2102d65c67c5e22078bcdf58d5a6dc Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Mon, 18 Feb 2019 16:24:57 +0000 Subject: [PATCH 1/2] lisa.trace: cleanup dead code Remove misleading assignements and checks that make the user think an attribute can be None, when it is always assigned a value by design. --- lisa/trace.py | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/lisa/trace.py b/lisa/trace.py index b05362387..6f2bd2301 100644 --- a/lisa/trace.py +++ b/lisa/trace.py @@ -88,9 +88,6 @@ class Trace(Loggable): # The platform information used to run the experiments self.plat_info = plat_info - # TRAPpy Trace object - self.ftrace = None - # Trace format self.trace_format = trace_format @@ -237,7 +234,7 @@ class Trace(Loggable): # Check for events available on the parsed trace self._check_available_events() - if len(self.available_events) == 0: + if not self.available_events: if has_function_stats: logger.info('Trace contains only functions stats') return @@ -270,7 +267,7 @@ class Trace(Loggable): logger = self.get_logger() for val in self.ftrace.get_filters(key): obj = getattr(self.ftrace, val) - if len(obj.data_frame): + if not obj.data_frame.empty: self.available_events.append(val) logger.debug('Events found on trace:') for evt in self.available_events: @@ -451,13 +448,12 @@ class Trace(Loggable): :param event: Trace event name :type event: str """ - if self.data_dir is None: - raise ValueError("trace data not (yet) loaded") - if self.ftrace and hasattr(self.ftrace, event): + try: return getattr(self.ftrace, event).data_frame - raise ValueError('Event [{}] not supported. ' - 'Supported events are: {}' - .format(event, self.available_events)) + except AttributeError: + raise ValueError('Event [{}] not supported. ' + 'Supported events are: {}' + .format(event, self.available_events)) def df_functions_stats(self, functions=None): """ @@ -474,8 +470,6 @@ class Trace(Loggable): to report :type functions: str or list(str) """ - if not hasattr(self, '_functions_stats_df'): - return None df = self._functions_stats_df if not functions: return df @@ -699,7 +693,7 @@ class Trace(Loggable): # make sure fake cpu_frequency events are never interleaved with # OS generated events else: - if len(devlib_freq) > 0: + if not devlib_freq.empty: # Frequencies injection is done in a per-cluster based. # This is based on the assumption that clusters are -- GitLab From 2ddf5dd7e52d5378236a0f3a6c6defe4feebded9 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Mon, 18 Feb 2019 16:30:27 +0000 Subject: [PATCH 2/2] lisa.trace: rename misleading attribute Rename `data_dir` attribute into `trace_path`. `plots_dir` can already be controlled as a separate `__init__` parameter. Update the transition guide as well. --- doc/users_guide.rst | 1 + ipynb/examples/typical_experiment.ipynb | 6 +++-- .../kernel_functions_profiling.ipynb | 3 ++- lisa/tests/base.py | 3 ++- lisa/trace.py | 22 +++++++------------ lisa/wa_results_collector.py | 2 +- tests/test_trace.py | 2 +- 7 files changed, 19 insertions(+), 20 deletions(-) diff --git a/doc/users_guide.rst b/doc/users_guide.rst index b7d96ddf5..ff502ad4d 100644 --- a/doc/users_guide.rst +++ b/doc/users_guide.rst @@ -324,6 +324,7 @@ but we did rename/move things to make them more coherent. * Removed last occurences of camelCase * Removed big.LITTLE assumptions and made the code only rely on CPU capacities or frequency domains, where relevant. +* Constructor now only takes trace files as input, not folders anymore. * ``Trace.data_frame`` is gone: **LISA legacy**:: diff --git a/ipynb/examples/typical_experiment.ipynb b/ipynb/examples/typical_experiment.ipynb index 48884ca6d..c3c588258 100644 --- a/ipynb/examples/typical_experiment.ipynb +++ b/ipynb/examples/typical_experiment.ipynb @@ -731,7 +731,8 @@ "metadata": {}, "outputs": [], "source": [ - "trace = Trace(wload.res_dir, te.plat_info, events=events)" + "trace_path = os.path.join(wload.res_dir, 'trace.dat')\n", + "trace = Trace(trace_path, te.plat_info, events=events)" ] }, { @@ -771,7 +772,8 @@ "outputs": [], "source": [ "plat_info = PlatformInfo.from_yaml_map(plat_info_path)\n", - "trace = Trace(wload.res_dir, plat_info, events=events)" + "trace_path = os.path.join(wload.res_dir, 'trace.dat')\n", + "trace = Trace(trace_path, plat_info, events=events)" ] }, { diff --git a/ipynb/profiling/kernel_functions_profiling.ipynb b/ipynb/profiling/kernel_functions_profiling.ipynb index 7ddf3e057..6dffd6e2a 100644 --- a/ipynb/profiling/kernel_functions_profiling.ipynb +++ b/ipynb/profiling/kernel_functions_profiling.ipynb @@ -623,7 +623,8 @@ "metadata": {}, "outputs": [], "source": [ - "trace = Trace(wload.res_dir, te.plat_info, events=events)" + "trace_path = os.path.join(wload.res_dir, 'trace.dat')\n", + "trace = Trace(trace_path, te.plat_info, events=events)" ] }, { diff --git a/lisa/tests/base.py b/lisa/tests/base.py index 0257a04b3..688837a52 100644 --- a/lisa/tests/base.py +++ b/lisa/tests/base.py @@ -376,7 +376,8 @@ class RTATestBundle(TestBundle, abc.ABC): allows updating the underlying path before it is actually loaded to match a different folder structure. """ - return Trace(self.res_dir, self.plat_info, events=self.ftrace_conf["events"]) + path = os.path.join(self.res_dir, 'trace.dat') + return Trace(path, self.plat_info, events=self.ftrace_conf["events"]) def __init__(self, res_dir, plat_info, rtapp_profile): super().__init__(res_dir, plat_info) diff --git a/lisa/trace.py b/lisa/trace.py index 6f2bd2301..9b7dcd150 100644 --- a/lisa/trace.py +++ b/lisa/trace.py @@ -43,8 +43,8 @@ class Trace(Loggable): """ The Trace object is the LISA trace events parser. - :param data_dir: folder containing all trace data - :type data_dir: str + :param trace_path: File containing the trace + :type trace_path: str :param events: events to be parsed (all the events by default) :type events: str or list(str) @@ -72,7 +72,7 @@ class Trace(Loggable): """ def __init__(self, - data_dir, + trace_path, plat_info=None, events=None, window=(0, None), @@ -117,21 +117,15 @@ class Trace(Loggable): self.freq_coherency = True # Folder containing trace - self.data_dir = data_dir - - # By deafult, use the trace dir to save plots - self.plots_dir = plots_dir - if self.plots_dir is None: - # In case we're passed the trace.dat - if os.path.isfile(data_dir): - self.plots_dir = os.path.dirname(data_dir) - else: - self.plots_dir = data_dir + self.trace_path = trace_path + + # By default, use the trace dir to save plots + self.plots_dir = plots_dir if plots_dir else os.path.dirname(trace_path) self.plots_prefix = plots_prefix self._register_trace_events(events) - self._parse_trace(self.data_dir, window, trace_format) + self._parse_trace(self.trace_path, window, trace_format) # Import here to avoid a circular dependency issue at import time # with lisa.analysis.base diff --git a/lisa/wa_results_collector.py b/lisa/wa_results_collector.py index 627c379c3..14a37ef94 100644 --- a/lisa/wa_results_collector.py +++ b/lisa/wa_results_collector.py @@ -423,7 +423,7 @@ class WaResultsCollector(Loggable): df = trace.analysis.frequency.df_cluster_frequency_residency(cluster) if df is None or df.empty: logger.warning("Can't get cluster freq residency from %s", - trace.data_dir) + trace.trace_path) else: df = df.reset_index() avg_freq = (df.frequency * df.time).sum() / df.time.sum() diff --git a/tests/test_trace.py b/tests/test_trace.py index 5a7fbfd86..6f402c33d 100644 --- a/tests/test_trace.py +++ b/tests/test_trace.py @@ -296,7 +296,7 @@ class TestTrace(StorageTestCase): columns = ['comm', 'pid', 'load', 'util', '__cpu'] for column in columns: msg = 'Task signals parsed from {} missing {} column'.format( - trace.data_dir, column) + trace.trace_path, column) self.assertIn(column, lt_df, msg=msg) # Pick an arbitrary PID to try plotting signals for. -- GitLab