From 078546418a937095941387abf22bd0d4370536a5 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Wed, 8 May 2019 12:17:07 +0100 Subject: [PATCH 1/8] lisa.utils: Add LayeredMapping This is a mapping that allows accessing a base read-only layer, with a read-write layer on top. All key writes are directed against the top read-write layer. --- lisa/utils.py | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/lisa/utils.py b/lisa/utils.py index 447f94ad1..869ee0230 100644 --- a/lisa/utils.py +++ b/lisa/utils.py @@ -17,7 +17,7 @@ import abc import copy -from collections.abc import Mapping, Sequence +from collections.abc import Mapping, MutableMapping, Sequence from collections import OrderedDict import contextlib import inspect @@ -755,4 +755,53 @@ def non_recursive_property(f): return property(wrapper) +class LayeredMapping(MutableMapping): + """ + A layered mutable mapping that allows setting values in a ``top`` layer, + while values from ``base`` layer cannot be modified. + + :param base: Base layer that will be read-only + :type base: collections.abc.Mapping + + :param top: Top layer that will be read-write and have priority over + ``base``. + :type base: collections.abc.MutableMapping + """ + def __init__(self, base, top=None): + self.base = base + self.top = top if top is not None else {} + + @property + def _merged(self): + merged = dict(self.base) + merged.update(self.top) + return merged + + def __getitem__(self, key): + return self._merged[key] + + def __setitem__(self, key, val): + self.top[key] = val + + def __delitem__(self, key): + del self.top[key] + + def __iter__(self): + return iter(self._merged) + + def __len__(self): + return len(self._merged) + + def __str__(self): + return str(self._merged) + + def __copy__(self): + # Shallow copy of underlying dict, so that they can be modified + # independently. Otherwise, any mutation on the LayeredMapping would + # impact the original top layer. + return LayeredMapping( + base=copy.copy(self.base), + top=copy.copy(self.top), + ) + # vim :set tabstop=4 shiftwidth=4 textwidth=80 expandtab -- GitLab From c6f070065f5cb66ade900965b6ccbb37b50b724a Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Fri, 10 May 2019 14:12:15 +0100 Subject: [PATCH 2/8] tests: load_tracking: Fix test_freq_invariance Fix a typo in variable name that made the test always pass. --- lisa/tests/scheduler/load_tracking.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lisa/tests/scheduler/load_tracking.py b/lisa/tests/scheduler/load_tracking.py index 6236a729c..4b02f81bb 100644 --- a/lisa/tests/scheduler/load_tracking.py +++ b/lisa/tests/scheduler/load_tracking.py @@ -604,7 +604,7 @@ class Invariance(TestBundle, LoadTrackingHelpers): freq_list.append(item.freq) # Only test util, as it should be more robust res = item.test_task_util_avg() - passed &= bool(res) + group_passed &= bool(res) name = '{}@{}'.format(cpu, item.freq) metrics[name] = res.metrics -- GitLab From 6fe08085b928737a799b7bb001d30de4cd110272 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Wed, 8 May 2019 12:18:10 +0100 Subject: [PATCH 3/8] lisa.tests.base: Add AggregatedResultBundle Allows aggregating multiple ResultBundle, and derive both results and metrics from the original ResultBundle. Extra metrics can still be added on top if needed. --- lisa/tests/base.py | 139 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 120 insertions(+), 19 deletions(-) diff --git a/lisa/tests/base.py b/lisa/tests/base.py index 45931917f..4d64d4524 100644 --- a/lisa/tests/base.py +++ b/lisa/tests/base.py @@ -35,7 +35,10 @@ from lisa.analysis.tasks import TasksAnalysis from lisa.trace import Trace, requires_events from lisa.wlgen.rta import RTA -from lisa.utils import Serializable, memoized, ArtifactPath, non_recursive_property +from lisa.utils import ( + Serializable, memoized, ArtifactPath, non_recursive_property, + LayeredMapping +) from lisa.trace import FtraceCollector, FtraceConf class TestMetric: @@ -91,7 +94,39 @@ class Result(enum.Enum): """Return the name in lower case""" return self.name.lower() -class ResultBundle: + +class ResultBundleBase: + """ + Base class for all result bundles. + + .. note:: ``__init__`` is not provided as some classes uses properties to + provide some of the attributes. + """ + + def __bool__(self): + return self.result is Result.PASSED + + def __str__(self): + return self.result.name + ': ' + ', '.join( + '{}={}'.format(key, val) + for key, val in self.metrics.items()) + + def add_metric(self, name, data, units=None): + """ + Lets you append several test :class:`TestMetric` to the bundle. + + :Parameters: :class:`TestMetric` parameters + """ + self.metrics[name] = TestMetric(data, units) + + def display_and_exit(self) -> type(None): + print("Test result: {}".format(self)) + if self: + sys.exit(0) + else: + sys.exit(1) + +class ResultBundle(ResultBundleBase): """ Bundle for storing test results @@ -131,28 +166,94 @@ class ResultBundle: result = Result.PASSED if cond else Result.FAILED return cls(result, *args, **kwargs) - def __bool__(self): - return self.result is Result.PASSED +class AggregatedResultBundle(ResultBundleBase): + """ + Aggregates many :class:`ResultBundle` into one. - def __str__(self): - return self.result.name + ': ' + ', '.join( - '{}={}'.format(key, val) - for key, val in self.metrics.items()) + :param result_bundles: List of :class:`ResultBundle` to aggregate. + :type result_bundles: list(ResultBundle) - def add_metric(self, name, data, units=None): - """ - Lets you append several test :class:`TestMetric` to the bundle. + :param name_metric: Metric to use as the "name" of each result bundle. + The value of that metric will be used as top-level key in the + aggregated metrics. If not provided, the index in the + ``result_bundles`` list will be used. + :type name_metric: str - :Parameters: :class:`TestMetric` parameters - """ - self.metrics[name] = TestMetric(data, units) + :param result: Optionally, force the ``self.result`` attribute to that + value. This is useful when the way of combining the result bundles is + not the default one, without having to make a whole new subclass. + :type result: Result - def display_and_exit(self) -> type(None): - print("Test result: {}".format(self)) - if self: - sys.exit(0) + This is useful for some tests that are naturally decomposed in subtests. + + .. note:: Metrics of aggregated bundles will always be shown, but can be + augmented with new metrics using the usual API. + """ + def __init__(self, result_bundles, name_metric=None, result=None): + self.result_bundles = result_bundles + self.name_metric = name_metric + self.extra_metrics = {} + self._forced_result = result + + @property + def result(self): + forced_result = self._forced_result + if forced_result is not None: + return forced_result + + def predicate(combinator, result): + return combinator( + res_bundle.result is result + for res_bundle in self.result_bundles + ) + + if predicate(all, Result.UNDECIDED): + return Result.UNDECIDED + elif predicate(any, Result.FAILED): + return Result.FAILED + elif predicate(any, Result.PASSED): + return Result.PASSED else: - sys.exit(1) + return Result.UNDECIDED + + @result.setter + def _(self, result): + self._forced_result = result + + @property + def metrics(self): + def get_name(res_bundle, i): + if self.name_metric: + return res_bundle.metrics[self.name_metric] + else: + return str(i) + + names = { + res_bundle: get_name(res_bundle, i) + for i, res_bundle in enumerate(self.result_bundles) + } + + def get_metrics(res_bundle): + metrics = copy.copy(res_bundle.metrics) + # Since we already show it at the top-level, we can remove it from + # the nested level to remove some clutter + metrics.pop(self.name_metric, None) + return metrics + + base = { + names[res_bundle]: get_metrics(res_bundle) + for res_bundle in self.result_bundles + } + + if 'failed' not in base: + base['failed'] = TestMetric([ + names[res_bundle] + for res_bundle in self.result_bundles + if res_bundle.result is Result.FAILED + ]) + top = self.extra_metrics + return LayeredMapping(base, top) + class CannotCreateError(RuntimeError): """ -- GitLab From 1fd0a964942d03acb3acc0b7783d85ca3c1ca34a Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Wed, 8 May 2019 14:06:06 +0100 Subject: [PATCH 4/8] lisa: Change default exekall goal to ResultBundleBase In order to run tests returning AggregatedResultBundle by default, make sure to set the base class as the goal. --- lisa/exekall_customize.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lisa/exekall_customize.py b/lisa/exekall_customize.py index 342b3d885..f31b5edd6 100644 --- a/lisa/exekall_customize.py +++ b/lisa/exekall_customize.py @@ -284,7 +284,7 @@ comparison. Can be repeated.""") @staticmethod def get_default_type_goal_pattern_set(): - return {'*.ResultBundle'} + return {'*.ResultBundleBase'} @classmethod def reload_db(cls, db, path=None): -- GitLab From 75cab07d70d09b35bb5b8a5ea5dead99eaa7e43b Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Wed, 8 May 2019 15:59:59 +0100 Subject: [PATCH 5/8] lisa: Refer to ResulBundleBase when appropriate Since ResultBundle is not the only class anymore, use ResulBundleBase in conjunction with isinstance() and issubclass(). --- lisa/exekall_customize.py | 4 ++-- lisa/regression.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lisa/exekall_customize.py b/lisa/exekall_customize.py index f31b5edd6..cb08f1ca5 100644 --- a/lisa/exekall_customize.py +++ b/lisa/exekall_customize.py @@ -30,7 +30,7 @@ from lisa.trace import FtraceCollector, FtraceConf from lisa.platforms.platinfo import PlatformInfo from lisa.utils import HideExekallID, Loggable, ArtifactPath, get_subclasses, groupby, Serializable from lisa.conf import MultiSrcConf -from lisa.tests.base import TestBundle, ResultBundle, Result +from lisa.tests.base import TestBundle, ResultBundleBase, Result from lisa.tests.scheduler.load_tracking import InvarianceItem from lisa.regression import compute_regressions @@ -396,7 +396,7 @@ comparison. Can be repeated.""") return 20 val = expr_val.value - if isinstance(val, ResultBundle): + if isinstance(val, ResultBundleBase): if val.result is Result.FAILED: return 10 return 0 diff --git a/lisa/regression.py b/lisa/regression.py index 124f4d702..41c906c2c 100644 --- a/lisa/regression.py +++ b/lisa/regression.py @@ -22,7 +22,7 @@ import itertools from collections import OrderedDict, namedtuple from lisa.utils import groupby, memoized -from lisa.tests.base import Result, ResultBundle +from lisa.tests.base import Result, ResultBundleBase import scipy.stats @@ -81,7 +81,7 @@ class RegressionResult: :type alpha: float """ def coerce_to_bool(x, res): - if isinstance(x, ResultBundle): + if isinstance(x, ResultBundleBase): return x.result is res # handle other types as well, as long as they can be # converted to bool -- GitLab From e7228c24d4a2626f7466f8afc81e8a37bcebe374 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Fri, 10 May 2019 15:34:00 +0100 Subject: [PATCH 6/8] lisa.tests.base: Improve ResultBundle.metrics formatting Handle nested mappings in a nicer way, so we get good output for AggregatedResultBundle. --- lisa/tests/base.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/lisa/tests/base.py b/lisa/tests/base.py index 4d64d4524..4b754e0b7 100644 --- a/lisa/tests/base.py +++ b/lisa/tests/base.py @@ -107,9 +107,18 @@ class ResultBundleBase: return self.result is Result.PASSED def __str__(self): - return self.result.name + ': ' + ', '.join( - '{}={}'.format(key, val) - for key, val in self.metrics.items()) + + def format_val(val): + # Handle recursive mappings, like metrics of AggregatedResultBundle + if isinstance(val, Mapping): + return '{' + ', '.join( + '{}={}'.format(key, format_val(val)) + for key, val in val.items() + ) + '}' + else: + return str(val) + + return self.result.name + ': ' + format_val(self.metrics) def add_metric(self, name, data, units=None): """ -- GitLab From 56db77d6805d5e39716a6892872c865825087116 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Wed, 8 May 2019 12:19:02 +0100 Subject: [PATCH 7/8] load_tracking: Use AggregatedResultBundle Use AggregatedResultBundle in Invariance tests. --- lisa/tests/scheduler/load_tracking.py | 111 +++++++++++++------------- 1 file changed, 54 insertions(+), 57 deletions(-) diff --git a/lisa/tests/scheduler/load_tracking.py b/lisa/tests/scheduler/load_tracking.py index 4b02f81bb..027732f73 100644 --- a/lisa/tests/scheduler/load_tracking.py +++ b/lisa/tests/scheduler/load_tracking.py @@ -15,6 +15,7 @@ # limitations under the License. # +import copy import os from collections import OrderedDict import itertools @@ -30,8 +31,8 @@ from bart.sched.SchedAssert import SchedAssert from trappy.stats.Topology import Topology from lisa.tests.base import ( - TestMetric, Result, ResultBundle, TestBundle, RTATestBundle, - CannotCreateError + TestMetric, Result, ResultBundle, AggregatedResultBundle, TestBundle, + RTATestBundle, CannotCreateError ) from lisa.target import Target from lisa.utils import ArtifactPath, groupby @@ -337,8 +338,6 @@ class InvarianceItem(LoadTrackingBase): expected_data = {} trace_data = {} - freq_str = '@{}'.format(self.freq) if self.freq is not None else '' - capacity = self.plat_info['cpu-capacities'][self.cpu] # Scale the capacity linearly according to the frequency @@ -352,11 +351,12 @@ class InvarianceItem(LoadTrackingBase): if not ok: passed = False - metric_name = 'cpu{}{}'.format(self.cpu, freq_str) - expected_data[metric_name] = TestMetric(exp_util) - trace_data[metric_name] = TestMetric(signal_mean) + expected_data[name] = TestMetric(exp_util) + trace_data[name] = TestMetric(signal_mean) + freq_str = '@{}'.format(self.freq) if self.freq is not None else '' bundle = ResultBundle.from_bool(passed) + bundle.add_metric("cpu", '{}{}'.format(self.cpu, freq_str)) bundle.add_metric("Expected signals", expected_data) bundle.add_metric("Trace signals", trace_data) return bundle @@ -507,7 +507,7 @@ class Invariance(TestBundle, LoadTrackingHelpers): # InvarianceItem with the result merged. @InvarianceItem.test_task_util_avg.used_events - def test_task_util_avg(self, allowed_error_pct=15) -> ResultBundle: + def test_task_util_avg(self, allowed_error_pct=15) -> AggregatedResultBundle: """ Aggregated version of :meth:`InvarianceItem.test_task_util_avg` """ @@ -518,7 +518,7 @@ class Invariance(TestBundle, LoadTrackingHelpers): return self._test_all_freq(item_test) @InvarianceItem.test_task_load_avg.used_events - def test_task_load_avg(self, allowed_error_pct=15) -> ResultBundle: + def test_task_load_avg(self, allowed_error_pct=15) -> AggregatedResultBundle: """ Aggregated version of :meth:`InvarianceItem.test_task_load_avg` """ @@ -534,24 +534,14 @@ class Invariance(TestBundle, LoadTrackingHelpers): :attr:`~lisa.tests.base.Result.UNDECIDED` is ignored. """ - item_res_bundles = { - '{}@{}'.format(item.cpu, item.freq): item_test(item) + item_res_bundles = [ + item_test(item) for item in self.invariance_items - } - - overall_bundle = ResultBundle.from_bool(all(item_res_bundles.values())) - for name, bundle in item_res_bundles.items(): - overall_bundle.add_metric(name, bundle.metrics) - - overall_bundle.add_metric('failed cpu@freq', [ - name for name, bundle in item_res_bundles.items() - if bundle.result is Result.FAILED - ]) - - return overall_bundle + ] + return AggregatedResultBundle(item_res_bundles, 'cpu') @InvarianceItem.test_task_util_avg.used_events - def test_cpu_invariance(self) -> ResultBundle: + def test_cpu_invariance(self) -> AggregatedResultBundle: """ Check that items using the max freq on each CPU is passing util avg test. @@ -560,8 +550,7 @@ class Invariance(TestBundle, LoadTrackingHelpers): .. seealso:: :class:`InvarianceItem.test_task_util_avg` """ - metrics = {} - passed = True + res_list = [] for cpu, item_group in groupby(self.invariance_items, key=lambda x: x.cpu): item_group = list(item_group) # combine all frequencies of that CPU class, although they should @@ -577,15 +566,9 @@ class Invariance(TestBundle, LoadTrackingHelpers): for item in max_freq_items: # Only test util, as it should be more robust res = item.test_task_util_avg() - passed &= bool(res) - metrics.setdefault(cpu, []).append(res.metrics) + res_list.append(res) - res = ResultBundle.from_bool(passed) - for cpu, submetrics in metrics.items(): - for submetric in submetrics: - res.add_metric(cpu, submetric) - - return res + return AggregatedResultBundle(res_list, 'cpu') @InvarianceItem.test_task_util_avg.used_events def test_freq_invariance(self) -> ResultBundle: @@ -594,35 +577,49 @@ class Invariance(TestBundle, LoadTrackingHelpers): .. seealso:: :class:`InvarianceItem.test_task_util_avg` """ + logger = self.get_logger() - metrics = {} - passed = False - for cpu, item_group in groupby(self.invariance_items, key=lambda x: x.cpu): - group_passed = True - freq_list = [] - for item in item_group: - freq_list.append(item.freq) - # Only test util, as it should be more robust - res = item.test_task_util_avg() - group_passed &= bool(res) - name = '{}@{}'.format(cpu, item.freq) - metrics[name] = res.metrics - logger.info('Util avg invariance {res} for CPU {cpu} at frequencies: {freq_list}'.format( - res='passed' if group_passed else 'failed', + def make_group_bundle(cpu, item_group): + bundle = AggregatedResultBundle( + [ + # Only test util, as it should be more robust + item.test_task_util_avg() + for item in item_group + ], + # each item's "cpu" metric also contains the frequency + name_metric='cpu', + ) + # At that level, we only report the CPU, since nested bundles cover + # different frequencies + bundle.add_metric('cpu', cpu) + + logger.info('Util avg invariance {res} for CPU {cpu}'.format( + res=bundle.result.lower_name, cpu=cpu, - freq_list=freq_list, )) + return bundle - # At least one group must pass - passed |= group_passed - - res = ResultBundle.from_bool(passed) - for cpu, submetric in metrics.items(): - res.add_metric(cpu, submetric) - - return res + group_result_bundles = [ + make_group_bundle(cpu, item_group) + for cpu, item_group in groupby(self.invariance_items, key=lambda x: x.cpu) + ] + # The combination differs from the AggregatedResultBundle default one: + # we consider as passed as long as at least one of the group has + # passed, instead of forcing all of them to pass. + if any(result_bundle.result is Result.PASSED for result_bundle in group_result_bundles): + overall_result = Result.PASSED + elif all(result_bundle.result is Result.UNDECIDED for result_bundle in group_result_bundles): + overall_result = Result.UNDECIDED + else: + overall_result = Result.FAILED + + return AggregatedResultBundle( + group_result_bundles, + name_metric='cpu', + result=overall_result + ) class PELTTask(LoadTrackingBase): """ -- GitLab From acc250456a09704df729e16987c45c395350a525 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Mon, 3 Jun 2019 17:18:51 +0100 Subject: [PATCH 8/8] tests: sched_android: Use AggregatedResultBundle Make use of the common aggregation logic. --- lisa/tests/staging/sched_android.py | 45 +++++++++++------------------ 1 file changed, 17 insertions(+), 28 deletions(-) diff --git a/lisa/tests/staging/sched_android.py b/lisa/tests/staging/sched_android.py index 585e04001..7770202c5 100644 --- a/lisa/tests/staging/sched_android.py +++ b/lisa/tests/staging/sched_android.py @@ -20,7 +20,7 @@ import os.path import abc from lisa.wlgen.rta import RTA, Periodic -from lisa.tests.base import TestBundle, Result, ResultBundle, RTATestBundle +from lisa.tests.base import TestBundle, Result, ResultBundle, RTATestBundle, AggregatedResultBundle from lisa.trace import Trace, FtraceCollector, FtraceConf, requires_events from lisa.target import Target from lisa.utils import ArtifactPath @@ -108,7 +108,8 @@ class SchedTuneBase(TestBundle): @abc.abstractmethod def _create_test_bundles(cls, target, res_dir, ftrace_coll): """ - Collects and yields a ResultBundle per test item. + Collects and yields a :class:`lisa.tests.base.ResultBundle` per test + item. """ pass @@ -128,19 +129,6 @@ class SchedTuneBase(TestBundle): item_cls.__name__, boost, prefer_idle)) return item_cls.from_target(target, boost, prefer_idle, res_dir=item_dir, ftrace_coll=ftrace_coll) - def _merge_res_bundles(self, res_bundles): - """ - Merge a set of result bundles - """ - overall_bundle = ResultBundle.from_bool(all(res_bundles.values())) - for name, bundle in res_bundles.items(): - overall_bundle.add_metric(name, bundle.metrics) - - overall_bundle.add_metric('failed', [ - name for name, bundle in res_bundles.items() - if bundle.result is Result.FAILED - ]) - return overall_bundle class SchedTuneFreqItem(SchedTuneItemBase): """ @@ -216,6 +204,7 @@ class SchedTuneFreqItem(SchedTuneItemBase): res = ResultBundle.from_bool(distance < freq_margin_pct) res.add_metric("target freq", target_freq, 'kHz') res.add_metric("average freq", avg_freq, 'kHz') + res.add_metric("boost", boost, '%') return res @@ -234,16 +223,15 @@ class SchedTuneFrequencyTest(SchedTuneBase): yield cls._create_test_bundle_item(target, res_dir, ftrace_coll, SchedTuneFreqItem, boost, False) - def test_stune_frequency(self, freq_margin_pct=10) -> ResultBundle: + def test_stune_frequency(self, freq_margin_pct=10) -> AggregatedResultBundle: """ .. seealso:: :meth:`SchedTuneFreqItem.test_stune_frequency` """ - res_bundles = { - 'boost{}'.format(b.boost): b.test_stune_frequency(freq_margin_pct) - for b in self.test_bundles - } - return self._merge_res_bundles(res_bundles) - + item_res_bundles = [ + item.test_stune_frequency(freq_margin_pct) + for item in self.test_bundles + ] + return AggregatedResultBundle(item_res_bundles, 'boost') class SchedTunePlacementItem(SchedTuneItemBase): """ @@ -292,6 +280,7 @@ class SchedTunePlacementItem(SchedTuneItemBase): pct_ko = time_ko * 100 / total_time res = ResultBundle.from_bool(pct_ko < bad_cpu_margin_pct) res.add_metric("time spent on inappropriate CPUs", pct_ko, '%') + res.add_metric("boost", boost, '%') return res @@ -311,14 +300,14 @@ class SchedTunePlacementTest(SchedTuneBase): yield cls._create_test_bundle_item(target, res_dir, ftrace_coll, SchedTunePlacementItem, boost, True) - def test_stune_task_placement(self, margin_pct=10) -> ResultBundle: + def test_stune_task_placement(self, margin_pct=10) -> AggregatedResultBundle: """ .. seealso:: :meth:`SchedTunePlacementItem.test_stune_task_placement` """ - res_bundles = { - 'boost{}'.format(b.boost): b.test_stune_task_placement(margin_pct) - for b in self.test_bundles - } - return self._merge_res_bundles(res_bundles) + item_res_bundles = [ + item.test_stune_task_placement(margin_pct) + for item in self.test_bundles + ] + return AggregatedResultBundle(item_res_bundles, 'boost') # vim :set tabstop=4 shiftwidth=4 textwidth=80 expandtab -- GitLab