From 1a5bd1be9c3c8c89ea3ea031edc7484333738fec Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Tue, 12 Sep 2023 13:57:09 +0100 Subject: [PATCH 1/3] lisa.target: Add Target.closing() FEATURE Returns a context manager that will disconnect the target automatically. --- lisa/_cli_tools/lisa_load_kmod.py | 6 ++++++ lisa/_cli_tools/lisa_platinfo_extract.py | 5 +++++ lisa/target.py | 17 +++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/lisa/_cli_tools/lisa_load_kmod.py b/lisa/_cli_tools/lisa_load_kmod.py index 7e5389d38..f30295ad7 100755 --- a/lisa/_cli_tools/lisa_load_kmod.py +++ b/lisa/_cli_tools/lisa_load_kmod.py @@ -42,6 +42,12 @@ def main(): params=params, ) + with target.closing() as target: + return _main(args, target) + + +def _main(args, target): + features = args.feature keep_loaded = not bool(args.cmd) diff --git a/lisa/_cli_tools/lisa_platinfo_extract.py b/lisa/_cli_tools/lisa_platinfo_extract.py index d25af47a1..907fd24dd 100755 --- a/lisa/_cli_tools/lisa_platinfo_extract.py +++ b/lisa/_cli_tools/lisa_platinfo_extract.py @@ -29,6 +29,11 @@ def main(): ) } args, target = Target.from_custom_cli(params=params) + + with target.closing() as target: + return _main(args, target) + +def _main(args, target): plat_info = target.plat_info # Make sure we get all the information we can, even if it means running for diff --git a/lisa/target.py b/lisa/target.py index 06be13189..c50d7bd8a 100644 --- a/lisa/target.py +++ b/lisa/target.py @@ -1303,6 +1303,23 @@ class Target(Loggable, HideExekallID, ExekallTaggable, Configurable): return wrapper_param + def closing(self): + """ + Returns a context manager that will disconnect the target automatically. + """ + # Do not use contextlib.contextmanager() as it would force the __exit__ + # path to run, since destroying a suspended generator raises + # GeneratorExit at the yield point. + class _ClosingCM: + def __enter__(_self): + return self + + def __exit__(_self, *args, **kwargs): + self.disconnect() + + return _ClosingCM() + + class Gem5SimulationPlatformWrapper(Gem5SimulationPlatform): def __init__(self, system, simulator, **kwargs): simulator_args = copy.copy(simulator.get('args', [])) -- GitLab From 409352d2709a275bb0ba258719ef6e8cabce7b92 Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Tue, 12 Sep 2023 14:37:17 +0100 Subject: [PATCH 2/3] lisa.conf: Add FilteredDeferredValue FEATURE Introduce a deferred value type that treats a sentinel value as no value existing for that key. --- lisa/conf.py | 160 ++++++++++++++++++++++++++++--------- lisa/platforms/platinfo.py | 12 +-- 2 files changed, 129 insertions(+), 43 deletions(-) diff --git a/lisa/conf.py b/lisa/conf.py index 56bb6067d..ae9f5c5ce 100644 --- a/lisa/conf.py +++ b/lisa/conf.py @@ -82,6 +82,23 @@ class DeferredValue: return f'' +class FilteredDeferredValue(DeferredValue): + """ + Same as :class:`lisa.conf.DeferredValue` except that the given sentinel + value will be interpreted as no value . + """ + def __init__(self, callback, sentinel=None): + @functools.wraps(callback) + def _callback(): + x = callback() + if x == sentinel: + raise _DeferredInexistentError() + else: + return x + + super().__init__(callback=_callback) + + class DeferredExcep(DeferredValue): """ Specialization of :class:`DeferredValue` to lazily raise an exception. @@ -361,6 +378,14 @@ class KeyComputationRecursionError(ConfigKeyError, RecursionError): """ +class _DeferredInexistentError(Exception): + """ + Raised when a :class:`lisa.conf.DeferredValue` cannot compute the value. It + is then interpreted by the machinery as if no value was provided for that + key. + """ + + class DerivedKeyDesc(KeyDesc): """ Key descriptor describing a key derived from other keys @@ -1234,7 +1259,7 @@ class MultiSrcConf(MultiSrcConfABC, Loggable, Mapping): new._nested_init(*args, **kwargs) return new - def __copy__(self): + def _copy(self, copyf): """ Shallow copy of the nested configuration tree, without duplicating the leaf values. @@ -1253,7 +1278,7 @@ class MultiSrcConf(MultiSrcConfABC, Loggable, Mapping): # make a shallow copy of the attributes so we don't end up sharing # metadata new.__dict__.update( - (key, copy.copy(val)) + (key, copyf(val)) for key, val in self.__dict__.items() if key not in not_copied ) @@ -1262,7 +1287,7 @@ class MultiSrcConf(MultiSrcConfABC, Loggable, Mapping): # levels as one "meta object". It's not really a deepcopy either, since # we don't copy the values themselves. def copy_sublevel(sublevel): - new_sublevel = copy.copy(sublevel) + new_sublevel = copyf(sublevel) # Fixup the parent new_sublevel._parent = new return new_sublevel @@ -1276,6 +1301,16 @@ class MultiSrcConf(MultiSrcConfABC, Loggable, Mapping): return new + def __copy__(self): + """ + Shallow copy of the nested configuration tree, without duplicating the + leaf values. + """ + return self._copy(copy.copy) + + def __deepcopy__(self, memo): + return self._copy(copy.deepcopy) + def to_map(self): """ Export the configuration as a mapping. @@ -1590,9 +1625,14 @@ class MultiSrcConf(MultiSrcConfABC, Loggable, Mapping): key_desc = self._structure[key] val = self._key_map[key][src] validate = True + if isinstance(val, DeferredValue): try: val = val(key_desc=key_desc) + except _DeferredInexistentError: + self.logger.debug(f'Lazy key "{key_desc.qualname}" was computed but no value was produced') + del self._key_map[key][src] + raise # Wrap into a ConfigKeyError so that the user code can easily # handle missing keys, and the original exception is still # available as excep.__cause__ since it was chained with "from" @@ -1622,10 +1662,9 @@ class MultiSrcConf(MultiSrcConfABC, Loggable, Mapping): val = DeferredExcep(excep) validate = False - if validate: - key_desc.validate_val(val) - - self._key_map[key][src] = val + if validate: + key_desc.validate_val(val) + self._key_map[key][src] = val return val def eval_deferred(self, cls=DeferredValue, src=None, resolve_src=True, error='raise'): @@ -1651,19 +1690,28 @@ class MultiSrcConf(MultiSrcConfABC, Loggable, Mapping): :type error: str or None """ for key, src_map in self._key_map.items(): - - if resolve_src and src is None: - resolved_src = self.resolve_src(key) - src_map = {resolved_src: src_map[resolved_src]} + if src is None: + if resolve_src: + try: + src_ = self.resolve_src(key) + except ConfigKeyError: + continue + else: + src_map = {src_: src_map[src_]} + else: + pass + else: + src_map = {src: src_map[src]} for src_, val in src_map.items(): - if src is not None and src != src_: - continue if isinstance(val, cls): - self._eval_deferred_val(src_, key, error=error) + try: + self._eval_deferred_val(src_, key, error=error) + except _DeferredInexistentError: + pass for sublevel in self._sublevel_map.values(): - sublevel.eval_deferred(cls, src, resolve_src, error=error) + sublevel.eval_deferred(cls, src=src, resolve_src=resolve_src, error=error) def __getstate__(self): """ @@ -1731,23 +1779,29 @@ class MultiSrcConf(MultiSrcConfABC, Loggable, Mapping): val = key_desc.compute_val(self, eval_deferred=eval_deferred) src = self.resolve_src(key) else: - # Compute the source to use for that key - if src is None: - src = self.resolve_src(key) + while True: + # Compute the source to use for that key + if src is None: + src = self.resolve_src(key) - try: - val = self._key_map[key][src] - except KeyError: - # pylint: disable=raise-missing-from - key = key_desc.qualname - raise ConfigKeyError( - f'Key "{key}" is not available from source "{src}"', - key=key, - src=src, - ) + try: + val = self._key_map[key][src] + except KeyError: + # pylint: disable=raise-missing-from + key = key_desc.qualname + raise ConfigKeyError( + f'Key "{key}" is not available from source "{src}"', + key=key, + src=src, + ) - if eval_deferred: - val = self._eval_deferred_val(src, key, error='raise') + if eval_deferred: + try: + val = self._eval_deferred_val(src, key, error='raise') + except _DeferredInexistentError: + continue + + break logger = self.logger if not quiet and logger.isEnabledFor(logging.DEBUG): @@ -1791,9 +1845,21 @@ class MultiSrcConf(MultiSrcConfABC, Loggable, Mapping): key = key_desc.qualname raise ValueError(f'Key "{key}" is a nested configuration level, it does not have a source on its own.', key) + def eval_key(src, key): + try: + val = self._eval_deferred_val(src, key, error='raise') + except _DeferredInexistentError: + return (False, None) + else: + return (True, val) + return OrderedDict( - (src, self._eval_deferred_val(src, key, error='raise')) - for src in self._resolve_prio(key) + (src, val) + for src, (add, val) in ( + (src, eval_key(src, key)) + for src in self._resolve_prio(key) + ) + if add ) def pretty_format(self, eval_deferred=False): @@ -1868,8 +1934,29 @@ class MultiSrcConf(MultiSrcConfABC, Loggable, Mapping): def __getitem__(self, key): return self.get_key(key) - def _get_key_names(self): - return sorted(list(self._key_map.keys()) + list(self._sublevel_map.keys())) + def _get_key_names(self, eval_deferred=True): + if eval_deferred: + # Ensure we have an accurate list of keys, not including deferred value + # that turn out to not exist. + self.eval_deferred(cls=FilteredDeferredValue) + + def has_key(key): + try: + self.get_key(key, src=None, eval_deferred=False, quiet=True) + except ConfigKeyError: + return False + else: + return True + + return sorted( + [ + key + for key in self._key_map.keys() + if has_key(key) + ] + list( + self._sublevel_map.keys() + ) + ) def _get_derived_key_names(self): return sorted( @@ -1893,15 +1980,14 @@ class MultiSrcConf(MultiSrcConfABC, Loggable, Mapping): ``collections.abc.Mapping.items()`` to allow not evaluating deferred values if necessary. """ - return ( (k, self.get_key(k, eval_deferred=eval_deferred, quiet=True)) - for k in self.keys() + for k in self._get_key_names(eval_deferred=eval_deferred) ) def _ipython_key_completions_(self): "Allow Jupyter keys completion in interactive notebooks" - regular_keys = set(self.keys()) + regular_keys = set(self._get_key_names(eval_deferred=False)) # For autocompletion purposes, we show the derived keys derived_keys = set(self._get_derived_key_names()) return sorted(regular_keys | derived_keys) diff --git a/lisa/platforms/platinfo.py b/lisa/platforms/platinfo.py index e6018dfe4..7233069c9 100644 --- a/lisa/platforms/platinfo.py +++ b/lisa/platforms/platinfo.py @@ -23,8 +23,8 @@ import typing from lisa.utils import HideExekallID, group_by_value, memoized from lisa.conf import ( - DeferredValue, DeferredExcep, MultiSrcConf, KeyDesc, LevelKeyDesc, - TopLevelKeyDesc, DerivedKeyDesc, ConfigKeyError, + DeferredValue, FilteredDeferredValue, DeferredExcep, MultiSrcConf, KeyDesc, + LevelKeyDesc, TopLevelKeyDesc, DerivedKeyDesc, ConfigKeyError, ) from lisa._generic import SortedSequence from lisa.energy_model import EnergyModel @@ -163,8 +163,8 @@ class PlatformInfo(MultiSrcConf, HideExekallID): 'abi': lambda: target.abi, 'os': lambda: target.os, 'rtapp': { - # Since it is expensive to compute, use an on-demand DeferredValue - 'calib': DeferredValue(RTA.get_cpu_calibrations, target, rta_calib_res_dir) + # Since it is expensive to compute, use an on-demand FilteredDeferredValue + 'calib': FilteredDeferredValue(functools.partial(RTA.get_cpu_calibrations, target, rta_calib_res_dir)) }, 'cpus-count': lambda: target.number_of_cpus, 'numa-nodes-count': lambda: target.number_of_nodes @@ -228,7 +228,7 @@ class PlatformInfo(MultiSrcConf, HideExekallID): 'orig': get_orig_capacities, } - info['kernel']['symbols-address'] = DeferredValue(self._read_kallsyms, target) + info['kernel']['symbols-address'] = FilteredDeferredValue(functools.partial(self._read_kallsyms, target)) return self._add_info(src, info, only_missing=only_missing, filter_none=True, **kwargs) @@ -269,7 +269,7 @@ class PlatformInfo(MultiSrcConf, HideExekallID): if val is None or isinstance(val, DeferredValue): return renamed_val elif deferred: - return DeferredValue(renamed_val) + return FilteredDeferredValue(renamed_val) else: try: return val() -- GitLab From cf5ff042cb56f19a15736d038cd390c2883e23b7 Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Tue, 12 Sep 2023 15:40:53 +0100 Subject: [PATCH 3/3] tests: Add lisa.conf.DeferredValue self-tests --- tests/test_conf.py | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/tests/test_conf.py b/tests/test_conf.py index 49f820b60..92ba0f168 100644 --- a/tests/test_conf.py +++ b/tests/test_conf.py @@ -23,7 +23,7 @@ import typing import pytest -from lisa.conf import MultiSrcConf, KeyDesc, LevelKeyDesc, TopLevelKeyDesc, DerivedKeyDesc, DeferredValue +from lisa.conf import MultiSrcConf, KeyDesc, LevelKeyDesc, TopLevelKeyDesc, DerivedKeyDesc, DeferredValue, FilteredDeferredValue from .utils import StorageTestCase, HOST_PLAT_INFO, HOST_TARGET_CONF """ A test suite for the MultiSrcConf subclasses.""" @@ -80,6 +80,19 @@ class TestTargetConf(TestMultiSrcConfBase): self.conf = copy.copy(HOST_TARGET_CONF) +class TestEmptyConf(TestMultiSrcConfBase): + __test__ = True + + class Conf(MultiSrcConf): + STRUCTURE = TopLevelKeyDesc('lisa-self-test-test-empty-conf', 'lisa self test empty conf', + [] + ) + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.conf = self.Conf() + + def compute_derived(base_conf): return base_conf['foo'] + sum(base_conf['bar']) + base_conf['sublevel']['subkey'] @@ -87,6 +100,8 @@ def compute_derived(base_conf): INTERNAL_STRUCTURE = ( KeyDesc('foo', 'foo help', [int]), KeyDesc('bar', 'bar help', [typing.Sequence[int]]), + KeyDesc('bar-deferred', 'bar help', [typing.Sequence[int]]), + KeyDesc('bar-filter-deferred', 'bar help', [typing.Sequence[int]]), KeyDesc('multitypes', 'multitypes help', [typing.Sequence[int], str, None]), LevelKeyDesc('sublevel', 'sublevel help', ( KeyDesc('subkey', 'subkey help', [int]), @@ -113,20 +128,23 @@ class TestConfWithDefault(MultiSrcConf): DEFAULT_SRC = { 'bar': [0, 1, 2], + 'bar-deferred': DeferredValue(lambda *_: [0, 1, 2]), + 'bar-filter-deferred': FilteredDeferredValue(lambda *_: None), } class TestMultiSrcConf(TestMultiSrcConfBase): def test_add_src_one_key(self): conf = copy.deepcopy(self.conf) - conf_src = {'foo': 22} + assert dict(conf) == dict(self.conf) + goal = dict(self.conf) + conf_src = {'foo': 22} conf.add_src('mysrc', conf_src) - goal = dict(self.conf) goal.update(conf_src) - assert dict(conf) == goal + assert dict(conf) == goal assert conf.resolve_src('foo') == 'mysrc' def test_disallowed_val(self): @@ -219,6 +237,14 @@ class TestTestConfWithDefault(TestMultiSrcConf): def test_default_src(self): ref = dict(TestConfWithDefault.DEFAULT_SRC) + + # Evaluate the deferred key + ref['bar-deferred'] = ref['bar-deferred']() + + # Remove the filtered deferred key, that will rightfully not be + # included when the conf is converted to dict() + del ref['bar-filter-deferred'] + # A freshly built object still has all the level keys, even if it has # no leaves ref['sublevel'] = {} -- GitLab