From a40d4d2377b4030879b92831a1482c26b55654c3 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Wed, 17 Oct 2018 16:49:17 +0100 Subject: [PATCH 01/14] env: Default TestEnv res_dir now uses the board name --- lisa/env.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lisa/env.py b/lisa/env.py index d1cf7a90d..ebc97598d 100644 --- a/lisa/env.py +++ b/lisa/env.py @@ -149,9 +149,13 @@ class TestEnv(Loggable): super().__init__() logger = self.get_logger() + board_name = target_conf.get('board', None) + self.tags = [board_name] if board_name else [] if not res_dir: - res_dir_name = datetime.now().strftime('TestEnv_%Y%m%d_%H%M%S.%f') - res_dir = os.path.join(LISA_HOME, RESULT_DIR, res_dir_name) + name = board_name or type(self).__qualname__ + time_str = datetime.now().strftime('%Y%m%d_%H%M%S.%f') + name = '{}-{}'.format(name, time_str) + res_dir = os.path.join(LISA_HOME, RESULT_DIR, name) # That res_dir is for the exclusive use of TestEnv itself, it must not # be used by users of TestEnv @@ -181,9 +185,6 @@ class TestEnv(Loggable): logger.info('Tools to install: %s', tools) self.install_tools(target, tools) - board_name = target_conf.get('board', None) - self.tags = [board_name] if board_name else [] - # Autodetect information from the target, after the TestEnv is # initialized. Expensive computations are deferred so they will only be # computed when actually needed. -- GitLab From dfdcb94e59f68b5b6ab1d5f4f6c7190ae294fe1a Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Wed, 17 Oct 2018 16:59:29 +0100 Subject: [PATCH 02/14] env: close race window when creating res_dir --- lisa/env.py | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/lisa/env.py b/lisa/env.py index ebc97598d..7bf7e0230 100644 --- a/lisa/env.py +++ b/lisa/env.py @@ -358,22 +358,34 @@ class TestEnv(Loggable): created results directory :type symlink: bool """ + logger = self.get_logger() - time_str = datetime.now().strftime('%Y%m%d_%H%M%S.%f') - if not name: - name = time_str - elif append_time: - name = "{}-{}".format(name, time_str) - - res_dir = os.path.join(self._res_dir, name) - - # Compute base installation path - self.get_logger().info('Creating result directory: %s', res_dir) - - try: - os.mkdir(res_dir) - except FileExistsError: - pass + while True: + time_str = datetime.now().strftime('%Y%m%d_%H%M%S.%f') + if not name: + name = time_str + elif append_time: + name = "{}-{}".format(name, time_str) + + res_dir = os.path.join(self._res_dir, name) + + # Compute base installation path + logger.info('Creating result directory: %s', res_dir) + + # It will fail if the folder already exists. In that case, + # append_time should be used to ensure we get a unique name. + try: + os.mkdir(res_dir) + except FileExistsError: + # If the time is used in the name, there is some hope that the + # next time it will succeed + if append_time: + logger.info('Directory already exists, retrying ...') + continue + else: + raise + else: + break if symlink: res_lnk = Path(LISA_HOME, LATEST_LINK) -- GitLab From ca09f9c2516b8950acec22adbb1e1bf5faeb4d8e Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Wed, 17 Oct 2018 17:48:14 +0100 Subject: [PATCH 03/14] exekall: fix symlinking in artifact directory --- lisa/exekall_customize.py | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/lisa/exekall_customize.py b/lisa/exekall_customize.py index 425225a49..6a577c4c2 100644 --- a/lisa/exekall_customize.py +++ b/lisa/exekall_customize.py @@ -176,24 +176,19 @@ class LISAAdaptor(AdaptorBase): # Add symlinks to artifact folders for ExprValue that were used in the # ExprValue graph, but were initially computed for another Expression if isinstance(val, ArtifactStorage): - try: - # If the folder is already a subfolder of our artifacts, we - # don't need to do anything - val.relative_to(testcase_artifact_dir) - # Otherwise, that means that such folder is reachable from our - # parent ExprValue and we want to get a symlink to them - except ValueError: + val = Path(val) + is_subfolder = (testcase_artifact_dir.resolve() in val.resolve().parents) + # The folder is reachable from our ExprValue, but is not a + # subfolder of the testcase_artifact_dir, so we want to get a + # symlink to it + if not is_subfolder: # We get the name of the callable callable_folder = val.parts[-2] folder = testcase_artifact_dir/callable_folder - # TODO: check os.path.relpath # We build a relative path back in the hierarchy to the root of # all artifacts - relative_artifact_dir = Path(*( - '..' for part in - folder.relative_to(artifact_dir).parts - )) + relative_artifact_dir = Path(os.path.relpath(artifact_dir, start=folder)) # The target needs to be a relative symlink, so we replace the # absolute artifact_dir by a relative version of it -- GitLab From 64b665e75497948cfabb82b1e5e5185180f7f141 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Wed, 17 Oct 2018 17:48:33 +0100 Subject: [PATCH 04/14] test_bundle: Use test class name when calling TestEnv.get_res_dir This allows to name the res_dir after the test class when created in a notebook environment. --- lisa/tests/kernel/test_bundle.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lisa/tests/kernel/test_bundle.py b/lisa/tests/kernel/test_bundle.py index 0ef3a87c8..c8271c07d 100644 --- a/lisa/tests/kernel/test_bundle.py +++ b/lisa/tests/kernel/test_bundle.py @@ -280,7 +280,7 @@ class TestBundle(Serializable, abc.ABC): cls.check_from_testenv(te) if not res_dir: - res_dir = te.get_res_dir() + res_dir = te.get_res_dir(cls.__qualname__) bundle = cls._from_testenv(te, res_dir, **kwargs) -- GitLab From a887f4be3066dc68139efaff3110cfb06b49ab65 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Wed, 17 Oct 2018 18:32:31 +0100 Subject: [PATCH 05/14] exekall: fix use of functools.lru_cache() --- tools/exekall/exekall/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/exekall/exekall/utils.py b/tools/exekall/exekall/utils.py index 7a061db3e..18cc0d726 100644 --- a/tools/exekall/exekall/utils.py +++ b/tools/exekall/exekall/utils.py @@ -345,7 +345,8 @@ def unique_type(*param_list): return decorator # Call the given function at most once per set of parameters -once = functools.lru_cache() +def once(callable_): + return functools.lru_cache(maxsize=None, typed=True)(callable_) def iterate_cb(iterator, pre_hook=None, post_hook=None): with contextlib.suppress(StopIteration): -- GitLab From 275a2cd470107f43ee869ee7d6003ad17cf87bd1 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Thu, 18 Oct 2018 16:55:49 +0100 Subject: [PATCH 06/14] bisector: fix test totals counters for ExekallLISATestStep --- tools/bisector | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/tools/bisector b/tools/bisector index 8c13ca1ee..e253aad31 100755 --- a/tools/bisector +++ b/tools/bisector @@ -59,6 +59,7 @@ import urllib.parse import urllib.request import uuid import xml.etree.ElementTree as ET + import ruamel.yaml yaml = ruamel.yaml.YAML(typ='unsafe') @@ -1931,6 +1932,7 @@ class ExekallLISATestStep(ShellStep): ignored_except_set = { tuple(e.strip().rsplit('.')) for e in ignore_excep } + considered_testcase_set = set(testcase) considered_iteration_set = set(iterations) @@ -2158,16 +2160,26 @@ class ExekallLISATestStep(ShellStep): for i in range(1, i_max + 1) ] - counts[issue] += 1 - if show_rates: - if ( - (issue == 'passed' and show_pass_rate) - or - issue_n and not (issue == 'passed' and ignore_non_issue) - ): - table_out( - '{testcase_id}: {pretty_issue} {issue_n}/{iteration_n} ({issue_pc:.1f}%)'.format(**locals()) + if i_set: + counts[issue] += 1 + + if show_rates and ( + (issue == 'passed' and show_pass_rate) + or + issue_n and not ( + issue in ('passed', 'skipped') + and ignore_non_issue + ) + ): + table_out( + '{testcase_id}: {pretty_issue} {issue_n}/{iteration_n} ({issue_pc:.1f}%)'.format( + testcase_id=testcase_id, + pretty_issue=pretty_issue, + issue_n=issue_n, + iteration_n=iteration_n, + issue_pc=issue_pc ) + ) if show_details and not (ignore_non_issue and issue == 'passed'): for entry in filtered_entry_list: @@ -2231,7 +2243,10 @@ class ExekallLISATestStep(ShellStep): 'Failed: {counts[failure]}/{total}, ' 'Undecided: {counts[undecided]}/{total}, ' 'Skipped: {counts[skipped]}/{total}, ' - 'Passed: {counts[passed]}/{total}'.format(**locals()) + 'Passed: {counts[passed]}/{total}'.format( + counts=counts, + total=total, + ) ) # Write out the digest in JSON format so another tool can exploit it -- GitLab From c0a5feee9bedcc59c8e2712cf225bdc94c9dab02 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Mon, 29 Oct 2018 11:29:37 +0000 Subject: [PATCH 07/14] exekall: fix pathlib use for Python < 3.6 --- tools/exekall/exekall/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/exekall/exekall/utils.py b/tools/exekall/exekall/utils.py index 18cc0d726..3b90c8132 100644 --- a/tools/exekall/exekall/utils.py +++ b/tools/exekall/exekall/utils.py @@ -187,7 +187,7 @@ def import_file(python_src, module_name=None, is_package=False): is_package=True ) else: - spec = importlib.util.spec_from_file_location(module_name, python_src, + spec = importlib.util.spec_from_file_location(module_name, str(python_src), submodule_search_locations=submodule_search_locations) if spec is None: raise ValueError('Could not find module "{module}" at {path}'.format( -- GitLab From 8a8c2aeb8e3672b4a0c912b7abd38025712a3976 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Mon, 29 Oct 2018 11:30:08 +0000 Subject: [PATCH 08/14] exekall: small cleanups in the engine --- tools/exekall/exekall/engine.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/tools/exekall/exekall/engine.py b/tools/exekall/exekall/engine.py index 2df5dc3cb..0207afbd4 100644 --- a/tools/exekall/exekall/engine.py +++ b/tools/exekall/exekall/engine.py @@ -423,11 +423,8 @@ class Expression: ) def get_all_values(self): - value_list = list() for result in self.result_list: - value_list.extend(result.value_list) - - return value_list + yield from result.value_list def find_result_list(self, param_expr_val_map): def value_map(expr_value_map): @@ -458,7 +455,7 @@ class Expression: ) def pretty_structure(self, indent=1): - indent_str = 4*" " * indent + indent_str = 4 * ' ' * indent if isinstance(self.op, PrebuiltOperator): op_name = '' @@ -1442,17 +1439,14 @@ class Operator: # mostly initialized. # Special support of return type annotation for classmethod - if ( - inspect.ismethod(self.resolved_callable) and - inspect.isclass(self.resolved_callable.__self__) - ): + if self.is_cls_method: return_type = self.value_type try: # If the return annotation type is an (indirect) base class of # the original annotation, we replace the annotation by the # subclass That allows implementing factory classmethods # easily. - if issubclass(self.resolved_callable.__self__, return_type): + if issubclass(self.unwrapped_callable.__self__, return_type): self.annotations['return'] = self.resolved_callable.__self__ except TypeError: pass -- GitLab From fef4eaf8051cc8137a002a61e9eecd86644559df Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Mon, 29 Oct 2018 13:46:41 +0000 Subject: [PATCH 09/14] conf: fix special YAML constructors --- lisa/utils.py | 2 +- target_conf.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lisa/utils.py b/lisa/utils.py index d8717daf1..2e52cfa65 100644 --- a/lisa/utils.py +++ b/lisa/utils.py @@ -187,7 +187,7 @@ class Serializable: """ varname = loader.construct_scalar(node) assert isinstance(varname, str) - return loader.construct_python_name(varname, node) + return loader.find_python_name(varname, node.start_mark) def to_path(self, filepath, fmt=None): """ diff --git a/target_conf.yml b/target_conf.yml index 752d4aca9..d94ee3ce7 100644 --- a/target_conf.yml +++ b/target_conf.yml @@ -70,7 +70,7 @@ target-conf: platform-info: # Include a preset platform-info file, instead of defining the keys directly here. # Note that you cannot use !include and define keys at the same time. - !include: $LISA_HOME/lisa/platforms/juno_r0.yml + !include $LISA_HOME/lisa/platforms/juno_r0.yml # conf: # rtapp: # # Calibration mapping of CPU numbers to calibration value for rtapp -- GitLab From f3e3a6f0fac8700b6ce597ed51601c33f521c178 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Mon, 29 Oct 2018 13:47:12 +0000 Subject: [PATCH 10/14] env: fix result dir handling --- lisa/env.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lisa/env.py b/lisa/env.py index 7bf7e0230..f413a5ba0 100644 --- a/lisa/env.py +++ b/lisa/env.py @@ -160,7 +160,8 @@ class TestEnv(Loggable): # That res_dir is for the exclusive use of TestEnv itself, it must not # be used by users of TestEnv self._res_dir = res_dir - os.makedirs(self._res_dir, exist_ok=True) + if self._res_dir: + os.makedirs(self._res_dir, exist_ok=True) self.target_conf = target_conf logger.debug('Target configuration %s', self.target_conf) -- GitLab From 164aaae0d0fc77e6c42c8a1ee4cc7a8dcea45a84 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Mon, 29 Oct 2018 14:53:57 +0000 Subject: [PATCH 11/14] env: minor readability improvement --- lisa/env.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lisa/env.py b/lisa/env.py index f413a5ba0..d1b110560 100644 --- a/lisa/env.py +++ b/lisa/env.py @@ -377,6 +377,7 @@ class TestEnv(Loggable): # append_time should be used to ensure we get a unique name. try: os.mkdir(res_dir) + break except FileExistsError: # If the time is used in the name, there is some hope that the # next time it will succeed @@ -385,8 +386,6 @@ class TestEnv(Loggable): continue else: raise - else: - break if symlink: res_lnk = Path(LISA_HOME, LATEST_LINK) -- GitLab From 4551fb184431c3bd95ccd12f4dcb3d0e050f519a Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Mon, 29 Oct 2018 14:55:34 +0000 Subject: [PATCH 12/14] exekall: improve summary readibility Align the IDs for better readibility --- tools/exekall/exekall/customization.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/tools/exekall/exekall/customization.py b/tools/exekall/exekall/customization.py index 513628801..5bb0d3e55 100644 --- a/tools/exekall/exekall/customization.py +++ b/tools/exekall/exekall/customization.py @@ -75,16 +75,27 @@ class AdaptorBase: def process_results(self, result_map): hidden_callable_set = self.hidden_callable_set + + # Get all IDs and compute the maximum length to align the output + result_id_map = { + result: result.get_id( + hidden_callable_set=hidden_callable_set, + full_qual=False, + ) + ':' + for expr, result_list in result_map.items() + for result in result_list + } + + max_id_len = len(max(result_id_map.values(), key=len)) + for expr, result_list in result_map.items(): for result in result_list: msg = self.result_str(result) msg = msg + '\n' if '\n' in msg else msg - out('{id}: {result}'.format( - id=result.get_id( - hidden_callable_set=hidden_callable_set, - full_qual=False, - ), + out('{id:<{max_id_len}} {result}'.format( + id=result_id_map[result], result=msg, + max_id_len=max_id_len, )) @classmethod -- GitLab From 2d7e1779823f09e5d2f2c18efec8157723276ec5 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Mon, 29 Oct 2018 17:39:26 +0000 Subject: [PATCH 13/14] exekall: Compute the tags in the glue Instead of using a "tags" attribute, the glue is now in charge of computing the list of tags for each value. --- lisa/env.py | 1 - lisa/exekall_customize.py | 11 +++- tools/exekall/exekall/customization.py | 10 ++++ tools/exekall/exekall/engine.py | 81 ++++++++++++-------------- tools/exekall/exekall/main.py | 22 +++++-- tools/exekall/exekall/utils.py | 59 ------------------- 6 files changed, 74 insertions(+), 110 deletions(-) diff --git a/lisa/env.py b/lisa/env.py index d1b110560..1501dd40e 100644 --- a/lisa/env.py +++ b/lisa/env.py @@ -150,7 +150,6 @@ class TestEnv(Loggable): logger = self.get_logger() board_name = target_conf.get('board', None) - self.tags = [board_name] if board_name else [] if not res_dir: name = board_name or type(self).__qualname__ time_str = datetime.now().strftime('%Y%m%d_%H%M%S.%f') diff --git a/lisa/exekall_customize.py b/lisa/exekall_customize.py index 6a577c4c2..027040c1f 100644 --- a/lisa/exekall_customize.py +++ b/lisa/exekall_customize.py @@ -27,7 +27,7 @@ from pathlib import Path import xml.etree.ElementTree as ET import traceback -from lisa.env import TargetConf, ArtifactPath +from lisa.env import TestEnv, TargetConf, ArtifactPath from lisa.platforms.platinfo import PlatformInfo from lisa.utils import HideExekallID, Loggable from lisa.tests.kernel.test_bundle import Result, ResultBundle, CannotCreateError @@ -207,6 +207,15 @@ class LISAAdaptor(AdaptorBase): for param, param_expr_val in expr_val.param_value_map.items(): self._finalize_expr_val(param_expr_val, artifact_dir, testcase_artifact_dir) + @classmethod + def get_tag_list(cls, value): + if isinstance(value, TestEnv): + board_name = value.target_conf.get('board') + tags = [board_name] if board_name else [] + else: + tags = super().get_tag_list(value) + + return tags def process_results(self, result_map): super().process_results(result_map) diff --git a/tools/exekall/exekall/customization.py b/tools/exekall/exekall/customization.py index 5bb0d3e55..fa3719870 100644 --- a/tools/exekall/exekall/customization.py +++ b/tools/exekall/exekall/customization.py @@ -16,6 +16,8 @@ # limitations under the License. # +import numbers + from exekall.engine import NoValue, get_name from exekall.utils import out @@ -27,6 +29,14 @@ class AdaptorBase: args = dict() self.args = args + @staticmethod + def get_tag_list(value): + if isinstance(value, numbers.Number): + tags = [str(value)] + else: + tags = [] + return tags + def get_db_loader(self): return None diff --git a/tools/exekall/exekall/engine.py b/tools/exekall/exekall/engine.py index 0207afbd4..d4df2aa55 100644 --- a/tools/exekall/exekall/engine.py +++ b/tools/exekall/exekall/engine.py @@ -253,7 +253,7 @@ class ExpressionWrapper: expr_gen = cls._build_expr(result_op, op_map, cls_map, op_stack = [], non_produced_handler=non_produced_handler, - cycle_handler=cycle_handler + cycle_handler=cycle_handler, ) for expr in expr_gen: if expr.validate_expr(op_map): @@ -353,7 +353,7 @@ class ExpressionWrapper: # Get all the possible ways of calling these operators param_combis = itertools.product(*( cls._build_expr(param_op, op_map, cls_map, - op_stack, non_produced_handler, cycle_handler + op_stack, non_produced_handler, cycle_handler, ) for param_op in op_combi )) @@ -454,28 +454,25 @@ class Expression: id = hex(id(self)) ) - def pretty_structure(self, indent=1): + def pretty_structure(self, full_qual=True, indent=1): indent_str = 4 * ' ' * indent if isinstance(self.op, PrebuiltOperator): op_name = '' - value_type_name = ( - get_name(self.op.value_type, full_qual=True) - # We just call the operator. It is cheap since it is only - # returing a pre-built object - + self._get_value_tag_str(self.op.callable_()) - ) else: op_name = self.op.name - value_type_name = get_name(self.op.value_type, full_qual=True) out = '{op_name} ({value_type_name})'.format( op_name = op_name, - value_type_name = value_type_name, + value_type_name = get_name(self.op.value_type, full_qual=full_qual) +, ) if self.param_map: out += ':\n'+ indent_str + ('\n'+indent_str).join( - '{param}: {desc}'.format(param=param, desc=desc.pretty_structure(indent+1)) + '{param}: {desc}'.format(param=param, desc=desc.pretty_structure( + full_qual=full_qual, + indent=indent+1 + )) for param, desc in self.param_map.items() ) return out @@ -484,17 +481,6 @@ class Expression: for expr_val in self.get_all_values(): yield from expr_val.get_failed_values() - @staticmethod - def _get_value_tag_str(value): - tag_str = '' - try: - if value.tags: - tag_str = '[' + '+'.join(str(v) for v in value.tags) + ']' - except AttributeError: - pass - return tag_str - - def get_id(self, *args, marked_value_set=None, mark_excep=False, hidden_callable_set=None, **kwargs): if hidden_callable_set is None: hidden_callable_set = set() @@ -572,12 +558,14 @@ class Expression: if not param_expr.op.callable_ in hidden_callable_set ) - get_tag = self._get_value_tag_str if with_tags else lambda v: '' - def tags_iter(value_list): if value_list: for expr_val in value_list: - tag = get_tag(expr_val.value) + if with_tags: + tag = expr_val.format_tag_list() + tag = '[{}]'.format(tag) if tag else '' + else: + tag = '' yield (expr_val, tag) # Yield at least once without any tag even if there is no computed # value available @@ -1397,7 +1385,12 @@ class Operator: # True to make all objects reusable by default, False otherwise REUSABLE_DEFAULT = True - def __init__(self, callable_, name=None): + def __init__(self, callable_, name=None, tag_list_getter=None): + + if not tag_list_getter: + tag_list_getter = lambda v: [] + self.tag_list_getter = tag_list_getter + assert callable(callable_) self._name = name self.callable_ = callable_ @@ -1454,7 +1447,7 @@ class Operator: def __repr__(self): return '' - def force_param(self, param_callable_map): + def force_param(self, param_callable_map, tag_list_getter=None): def define_type(param_type): class ForcedType(param_type): # Make ourselves transparent for better reporting @@ -1473,8 +1466,9 @@ class Operator: ForcedType = define_type(param_type) self.annotations[param] = ForcedType prebuilt_op_set.add( - PrebuiltOperator(ForcedType, value_list) - ) + PrebuiltOperator(ForcedType, value_list, + tag_list_getter=tag_list_getter + )) # Make sure the parameter is not optional anymore self.optional_param.discard(param) @@ -1697,18 +1691,7 @@ class PrebuiltOperator(Operator): self.obj_list = obj_list_ self.uuid_list = uuid_list self.obj_type = obj_type - - if id_ is None: - name = self.obj_type - else: - name = id_ - # Get rid of the existing tags, since the name of the operator - # already carries that information. - for obj in self.obj_list: - try: - del obj.tags - except AttributeError: - pass + name = self.obj_type if id_ is None else id_ # Placeholder for the signature def callable_() -> self.obj_type: @@ -1811,7 +1794,8 @@ class SerializableExprValue: for full_qual, with_tags in itertools.product((True, False), repeat=2): self.recorded_id_map[(full_qual, with_tags)] = expr_val.get_id( full_qual = full_qual, - with_tags = with_tags + with_tags = with_tags, + hidden_callable_set=hidden_callable_set, ) self.type_names = [ @@ -1824,7 +1808,7 @@ class SerializableExprValue: for param, param_expr_val in expr_val.param_value_map.items(): param_serialzable = param_expr_val._get_serializable( serialized_map, - hidden_callable_set + hidden_callable_set=hidden_callable_set ) self.param_value_map[param] = param_serialzable @@ -1873,7 +1857,7 @@ def get_name(obj, full_qual=True): class ExprValue: def __init__(self, expr, param_value_map, value=NoValue, value_uuid=None, - excep=NoValue, excep_uuid=None + excep=NoValue, excep_uuid=None, ): self.value = value self.value_uuid = value_uuid @@ -1882,6 +1866,13 @@ class ExprValue: self.expr = expr self.param_value_map = param_value_map + def format_tag_list(self): + tag_list = self.expr.op.tag_list_getter(self.value) + if tag_list: + return '+'.join(str(v) for v in tag_list) + else: + return '' + def _get_serializable(self, serialized_map, *args, **kwargs): if serialized_map is None: serialized_map = dict() diff --git a/tools/exekall/exekall/main.py b/tools/exekall/exekall/main.py index b679bdf92..1d4ec932a 100755 --- a/tools/exekall/exekall/main.py +++ b/tools/exekall/exekall/main.py @@ -253,7 +253,10 @@ the name of the parameter, the start value, stop value and step size.""") callable_pool = utils.get_callable_set(module_set) callable_pool = adaptor.filter_callable_pool(callable_pool) - op_pool = {engine.Operator(callable_) for callable_ in callable_pool} + op_pool = { + engine.Operator(callable_, tag_list_getter=adaptor.get_tag_list) + for callable_ in callable_pool + } op_pool = { op for op in op_pool # Only select operators with non-empty parameter list. This rules out @@ -366,7 +369,8 @@ the name of the parameter, the start value, stop value and step size.""") id_ = serial_list[0].get_id(full_qual=False, with_tags=True) prebuilt_op_pool_list.append( engine.PrebuiltOperator( - type_, serial_list, id_=id_ + type_, serial_list, id_=id_, + tag_list_getter=adaptor.get_tag_list, )) # Make sure that the provided PrebuiltOperator will be the only ones used @@ -382,7 +386,10 @@ the name of the parameter, the start value, stop value and step size.""") for op in op_pool: if op.name == op_name: try: - new_op_pool = op.force_param(param_patch_map) + new_op_pool = op.force_param( + param_patch_map, + tag_list_getter=adaptor.get_tag_list + ) prebuilt_op_pool_list.extend(new_op_pool) except KeyError as e: error('Callable "{callable_}" has no parameter "{param}"'.format( @@ -484,6 +491,10 @@ the name of the parameter, the start value, stop value and step size.""") # Only print once per parameters' tuple @utils.once def handle_non_produced(cls_name, consumer_name, param_name, callable_path): + # When reloading from the DB, we don't want to be annoyed with lots of + # output related to missing PrebuiltOperator + if load_db_path and not verbose: + return info('Nothing can produce instances of {cls} needed for {consumer} (parameter "{param}", along path {path})'.format( cls = cls_name, consumer = consumer_name, @@ -505,7 +516,7 @@ the name of the parameter, the start value, stop value and step size.""") testcase_list = list(engine.ExpressionWrapper.build_expr_list( root_op_list, op_map, cls_map, non_produced_handler = handle_non_produced, - cycle_handler = handle_cycle + cycle_handler = handle_cycle, )) # Only keep the Expression where the outermost (root) operator is defined @@ -547,6 +558,8 @@ the name of the parameter, the start value, stop value and step size.""") db_loader = adaptor.get_db_loader() + out('\nArtifacts dir: {}'.format(artifact_dir)) + for testcase in testcase_list: testcase_short_id = take_first(testcase.get_id( hidden_callable_set=hidden_callable_set, @@ -684,6 +697,7 @@ the name of the parameter, the start value, stop value and step size.""") id=result.get_id( full_qual=False, mark_excep=True, + with_tags=True, ).strip().replace('\n', '\n'+len(prefix)*' '), prefix=prefix, )) diff --git a/tools/exekall/exekall/utils.py b/tools/exekall/exekall/utils.py index 3b90c8132..871b496c2 100644 --- a/tools/exekall/exekall/utils.py +++ b/tools/exekall/exekall/utils.py @@ -265,16 +265,6 @@ def get_module_basename(path): module_name = path.name return module_name -class TaggedNum: - def __init__(self, *args, **kwargs): - self.tags = [str(a) for a in args] - -class Int(int, TaggedNum): - pass - -class Float(float, TaggedNum): - pass - def sweep_number( callable_, param, start, stop=None, step=1): @@ -289,61 +279,12 @@ def sweep_number( stop = start start = 0 - # Swap-in the tagged type if possible - if issubclass(type_, numbers.Integral): - type_ = Int - # Must come in 2nd place, since int is a subclass of numbers.Real - elif issubclass(type_, numbers.Real): - type_ = Float - i = type_(start) step = type_(step) while i <= stop: yield type_(i) i += step -def _make_tagged_type(name, qualname, mod_name, bases): - class new_type(*bases): - def __init__(self, *args, **kwargs): - self.tags = ( - [str(a) for a in args] + - [ - k+'='+str(a) - for k, a in kwargs.items() - ] - ) - try: - super().__init__(*args, **kwargs) - except TypeError: - pass - - new_type.__name__ = name - new_type.__qualname__ = qualname - new_type.__module__ = mod_name - return new_type - -def unique_type(*param_list): - def decorator(f): - annot = engine.get_type_hints(f) - for param in param_list: - type_ = annot[param] - f_name = engine.get_name(f, full_qual=False) - - new_type_name = '{f}_{name}'.format( - f = f_name.replace('.', '_'), - type_name = type_.__name__, - name = param - ) - new_type = _make_tagged_type( - new_type_name, new_type_name, f.__module__, - (type_,) - ) - f.__globals__[new_type_name] = new_type - f.__annotations__[param] = new_type - return f - - return decorator - # Call the given function at most once per set of parameters def once(callable_): return functools.lru_cache(maxsize=None, typed=True)(callable_) -- GitLab From a2a1e58f71b6dd4f1c4d49317095369c5c1f49d6 Mon Sep 17 00:00:00 2001 From: Douglas RAILLARD Date: Tue, 30 Oct 2018 11:46:55 +0000 Subject: [PATCH 14/14] utils: fix serialization and deserialization * Solve issues when pickling generic containers * Fix ArtifactStorage root relocation --- lisa/env.py | 10 ++--- lisa/exekall_customize.py | 26 +++++------ lisa/platforms/platinfo.py | 10 ++--- lisa/utils.py | 76 +++++++++++---------------------- tools/exekall/exekall/engine.py | 36 ++++++++-------- 5 files changed, 66 insertions(+), 92 deletions(-) diff --git a/lisa/env.py b/lisa/env.py index 1501dd40e..c99cb4786 100644 --- a/lisa/env.py +++ b/lisa/env.py @@ -33,7 +33,7 @@ from devlib.platform.gem5 import Gem5SimulationPlatform from lisa.wlgen.rta import RTA from lisa.energy_meter import EnergyMeter -from lisa.utils import Loggable, MultiSrcConf, HideExekallID, resolve_dotted_name, get_all_subclasses, import_all_submodules, TypedList, LISA_HOME +from lisa.utils import Loggable, MultiSrcConf, HideExekallID, resolve_dotted_name, get_all_subclasses, import_all_submodules, LISA_HOME, StrList from lisa.platforms.platinfo import PlatformInfo @@ -65,10 +65,10 @@ class TargetConf(MultiSrcConf, HideExekallID): 'device': str, 'keyfile': str, 'workdir': str, - 'tools': TypedList[str], + 'tools': StrList, 'ftrace': { - 'events': TypedList[str], - 'functions': TypedList[str], + 'events': StrList, + 'functions': StrList, 'buffsize': int, }, 'devlib': { @@ -76,7 +76,7 @@ class TargetConf(MultiSrcConf, HideExekallID): 'class': str, 'args': Mapping, }, - 'excluded-modules': TypedList[str], + 'excluded-modules': StrList, } } diff --git a/lisa/exekall_customize.py b/lisa/exekall_customize.py index 027040c1f..f52ee2e13 100644 --- a/lisa/exekall_customize.py +++ b/lisa/exekall_customize.py @@ -52,7 +52,7 @@ class ArtifactStorage(ArtifactPath, Loggable, HideExekallID): path_str = super().__new__(cls, str(path), *args, **kwargs) # Record the actual root, so we can relocate the path later with an # updated root - path_str.artifact_dir = root + path_str.root = root return path_str def __fspath__(self): @@ -61,31 +61,30 @@ class ArtifactStorage(ArtifactPath, Loggable, HideExekallID): def __reduce__(self): # Serialize the path relatively to the root, so it can be relocated # easily - relative = self.relative_to(self.artifact_dir) - return (type(self), (self.artifact_dir, relative)) + relative = self.relative_to(self.root) + return (type(self), (self.root, relative)) def relative_to(self, path): return os.path.relpath(self, start=path) - def with_artifact_dir(self, artifact_dir): + def with_root(self, root): # Get the path relative to the old root - relative = self.relative_to(self.artifact_dir) + relative = self.relative_to(self.root) - # Swap-in the new artifact_dir and return a new instance - return type(self)(artifact_dir, relative) + # Swap-in the new root and return a new instance + return type(self)(root, relative) @classmethod def from_expr_data(cls, data:ExprData, consumer:Consumer) -> 'ArtifactStorage': """ Factory used when running under `exekall` """ - artifact_dir = Path(data['artifact_dir']).resolve() - root = data['testcase_artifact_dir'] + artifact_dir = Path(data['testcase_artifact_dir']).resolve() consumer_name = get_name(consumer) # Find a non-used directory for i in itertools.count(1): - artifact_dir = Path(root, consumer_name, str(i)) + artifact_dir = Path(artifact_dir, consumer_name, str(i)) if not artifact_dir.exists(): break @@ -97,8 +96,9 @@ class ArtifactStorage(ArtifactPath, Loggable, HideExekallID): path = artifact_dir )) artifact_dir.mkdir(parents=True) - relative = artifact_dir.relative_to(artifact_dir) - return cls(artifact_dir, relative) + root = data['artifact_dir'] + relative = artifact_dir.relative_to(root) + return cls(root, relative) class LISAAdaptor(AdaptorBase): name = 'LISA' @@ -159,7 +159,7 @@ class LISAAdaptor(AdaptorBase): for attr, attr_val in dct.items(): if isinstance(attr_val, ArtifactStorage): setattr(val, attr, - attr_val.with_artifact_dir(artifact_dir) + attr_val.with_root(artifact_dir) ) return db diff --git a/lisa/platforms/platinfo.py b/lisa/platforms/platinfo.py index 9c4f2e628..7c25dc99a 100644 --- a/lisa/platforms/platinfo.py +++ b/lisa/platforms/platinfo.py @@ -21,7 +21,7 @@ from collections import ChainMap from collections.abc import Mapping from numbers import Real -from lisa.utils import HideExekallID, MultiSrcConf, memoized, TypedDict, TypedList, DeferredValue +from lisa.utils import HideExekallID, MultiSrcConf, memoized, DeferredValue, IntRealDict, IntIntDict, StrIntListDict from lisa.energy_model import EnergyModel from lisa.wlgen.rta import RTA @@ -37,10 +37,10 @@ class PlatformInfo(MultiSrcConf, HideExekallID): # we need. STRUCTURE = { 'rtapp': { - 'calib': TypedDict[int, int], + 'calib': IntIntDict, }, 'nrg-model': EnergyModel, - 'cpu-capacities': TypedDict[int, Real], + 'cpu-capacities': IntRealDict, 'kernel-version': KernelVersion, 'abi': str, 'os': str, @@ -48,9 +48,9 @@ class PlatformInfo(MultiSrcConf, HideExekallID): # TODO: remove that once no code depend on it anymore 'topology': Topology, - 'clusters': TypedDict[str, TypedList[int]], + 'clusters': StrIntListDict, 'cpus-count': int, - 'freqs': TypedDict[str, TypedList[int]], + 'freqs': StrIntListDict, } """Some keys have a reserved meaning with an associated type.""" diff --git a/lisa/utils.py b/lisa/utils.py index 2e52cfa65..ec53ef6e9 100644 --- a/lisa/utils.py +++ b/lisa/utils.py @@ -30,6 +30,7 @@ import os import importlib import pkgutil import operator +import numbers import ruamel.yaml from ruamel.yaml import YAML @@ -550,9 +551,9 @@ class MultiSrcConf(SerializableConfABC, Loggable, Mapping): pass # Some classes are able to raise a more detailed exception than # just the boolean return value of __instancecheck__ - elif hasattr(cls, '_instancecheck'): + elif hasattr(cls, 'instancecheck'): try: - cls._instancecheck(val) + cls.instancecheck(val) except ValueError as e: raise_excep(key, val, cls, str(e)) else: @@ -714,7 +715,7 @@ class MultiSrcConf(SerializableConfABC, Loggable, Mapping): k: src_map for k, src_map in key_map.items() if src_map } - state = copy.copy(self.__dict__) + state = copy.copy(super().__getstate__()) state['_key_map'] = key_map return state @@ -828,42 +829,19 @@ class MultiSrcConf(SerializableConfABC, Loggable, Mapping): class GenericContainerMetaBase(type): def __instancecheck__(cls, instance): try: - cls._instancecheck(instance) + cls.instancecheck(instance) except ValueError: return False else: return True -# That is needed to make ruamel.yaml consider these classes as objects, so it -# uses __reduce_ex__ -ruamel.yaml.Representer.add_multi_representer(GenericContainerMetaBase, ruamel.yaml.Representer.represent_object) - class GenericContainerBase: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - type(self)._instancecheck(self) - - @classmethod - def __reduce_ex__(cls, version): - return (cls._build, (cls._type,)) + type(self).instancecheck(self) class GenericMappingMeta(GenericContainerMetaBase, type(Mapping)): - @memoized - def __getitem__(cls, type_): - if type_ is None: - return cls - - class new_cls(cls): - _type = type_ - - suffix = '[{},{}]'.format(*( - t.__qualname__ for t in type_ - )) - new_cls.__qualname__ = cls.__qualname__ + suffix - new_cls.__name__ = cls.__name__ + suffix - return new_cls - - def _instancecheck(cls, instance): + def instancecheck(cls, instance): if not isinstance(instance, Mapping): raise ValueError('not a Mapping') @@ -884,26 +862,10 @@ class GenericMappingMeta(GenericContainerMetaBase, type(Mapping)): )) class TypedDict(GenericContainerBase, dict, metaclass=GenericMappingMeta): - # Workaround issues in ruamel.yaml when it comes to complex setups - @staticmethod - def _build(types): - return TypedDict[types] + pass class GenericSequenceMeta(GenericContainerMetaBase, type(Sequence)): - @memoized - def __getitem__(cls, type_): - if type_ is None: - return cls - - class new_cls(cls): - _type = type_ - - suffix = '[{}]'.format(type_.__qualname__) - new_cls.__qualname__ = cls.__qualname__ + suffix - new_cls.__name__ = cls.__name__ + suffix - return new_cls - - def _instancecheck(cls, instance): + def instancecheck(cls, instance): if not isinstance(instance, Sequence): raise ValueError('not a Sequence') @@ -918,10 +880,22 @@ class GenericSequenceMeta(GenericContainerMetaBase, type(Sequence)): )) class TypedList(GenericContainerBase, list, metaclass=GenericSequenceMeta): - # Workaround issues in ruamel.yaml when it comes to complex setups - @staticmethod - def _build(types): - return TypedList[types] + pass + +class IntIntDict(TypedDict): + _type = (int, int) + +class IntRealDict(TypedDict): + _type = (int, numbers.Real) + +class IntList(TypedList): + _type = int + +class StrList(TypedList): + _type = str + +class StrIntListDict(TypedDict): + _type = (str, IntList) def setup_logging(filepath='logging.conf', level=logging.INFO): """ diff --git a/tools/exekall/exekall/engine.py b/tools/exekall/exekall/engine.py index d4df2aa55..135a52899 100644 --- a/tools/exekall/exekall/engine.py +++ b/tools/exekall/exekall/engine.py @@ -37,6 +37,7 @@ import contextlib import types import pprint import sys +import logging import ruamel.yaml @@ -1330,7 +1331,8 @@ def is_serializable(obj, raise_excep=False): # This may be slow for big objects but it is the only way to be sure # it can actually be serialized pickle.dumps(obj) - except (TypeError, pickle.PickleError): + except (TypeError, pickle.PickleError) as e: + logging.getLogger('serialization test').debug('Cannot serialize instance of %s: %s', type(obj).__qualname__, str(e)) if raise_excep: raise NotSerializableError(obj) return False @@ -1494,16 +1496,17 @@ class Operator: def get_name(self, full_qual=True): if self._name is not None: - if isinstance(self._name, str): - return self._name - # We allow passing in types for example, that will be used as the - # source for the name + # We allow passing in types that will be used as the source for the + # name + if isinstance(self._name, type): + name = get_name(self._name, full_qual) else: - return get_name(self._name, full_qual) - try: - name = get_name(self.callable_, full_qual) - except AttributeError: - name = self._name + name = str(self._name) + else: + try: + name = get_name(self.callable_, full_qual) + except AttributeError: + name = self._name return name @@ -1816,18 +1819,15 @@ class SerializableExprValue: args = (full_qual, with_tags) return self.recorded_id_map[args] - def get_parent_set(self, predicate): - parent_set = set() + def get_parent_set(self, predicate, _parent_set=None): + parent_set = set() if _parent_set is None else _parent_set if predicate(self): parent_set.add(self) - self._get_parent_set(parent_set, predicate) - return parent_set - def _get_parent_set(self, parent_set, predicate): for parent in self.param_value_map.values(): - if predicate(parent): - parent_set.add(parent) - parent._get_parent_set(parent_set, predicate) + parent.get_parent_set(predicate, _parent_set=parent_set) + + return parent_set def get_name(obj, full_qual=True): # Add the module's name in front of the name to get a fully -- GitLab