From 2e7df0ae082dc8848722b07adcd6ba537ac1f0de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=B6hm?= Date: Sun, 24 Sep 2023 13:25:19 +0200 Subject: [PATCH 01/12] Fixes patch provider typing --- .../varats/provider/patch/patch_provider.py | 24 +++++++++++-------- varats-core/varats/utils/filesystem_util.py | 3 ++- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/varats-core/varats/provider/patch/patch_provider.py b/varats-core/varats/provider/patch/patch_provider.py index d466a629b..318da442a 100644 --- a/varats-core/varats/provider/patch/patch_provider.py +++ b/varats-core/varats/provider/patch/patch_provider.py @@ -49,7 +49,7 @@ def __init__( self.tags: tp.Optional[tp.Set[str]] = tags @staticmethod - def from_yaml(yaml_path: Path): + def from_yaml(yaml_path: Path) -> 'Patch': """Creates a Patch from a YAML file.""" yaml_dict = yaml.safe_load(yaml_path.read_text()) @@ -71,7 +71,9 @@ def from_yaml(yaml_path: Path): # Update repository to have all upstream changes fetch_repository(project_git_path) - def parse_revisions(rev_dict: tp.Dict) -> tp.Set[CommitHash]: + def parse_revisions( + rev_dict: tp.Dict[str, tp.Any] + ) -> tp.Set[CommitHash]: res: tp.Set[CommitHash] = set() if "single_revision" in rev_dict: @@ -102,10 +104,11 @@ def parse_revisions(rev_dict: tp.Dict) -> tp.Set[CommitHash]: return res + include_revisions: tp.Set[CommitHash] if "include_revisions" in yaml_dict: include_revisions = parse_revisions(yaml_dict["include_revisions"]) else: - include_revisions: tp.Set[CommitHash] = set( + include_revisions = set( get_all_revisions_between( get_initial_commit(project_git_path).hash, "", ShortCommitHash, project_git_path @@ -137,7 +140,7 @@ def __str__(self) -> str: return str_representation - def __hash__(self): + def __hash__(self) -> int: if self.tags: return hash((self.shortname, str(self.path), tuple(self.tags))) @@ -148,7 +151,7 @@ class PatchSet: """A PatchSet is a storage container for project specific patches that can easily be accessed via the tags of a patch.""" - def __init__(self, patches: tp.Set[Patch]): + def __init__(self, patches: tp.Union[tp.Set[Patch], tp.FrozenSet[Patch]]): self.__patches: tp.FrozenSet[Patch] = frozenset(patches) def __iter__(self) -> tp.Iterator[Patch]: @@ -160,7 +163,7 @@ def __contains__(self, value: tp.Any) -> bool: def __len__(self) -> int: return len(self.__patches) - def __getitem__(self, tags: tp.Union[str, tp.Iterable[str]]): + def __getitem__(self, tags: tp.Union[str, tp.Iterable[str]]) -> 'PatchSet': """ Overrides the bracket operator of a PatchSet. @@ -286,7 +289,7 @@ def get_patches_for_revision(self, revision: CommitHash) -> PatchSet: @classmethod def create_provider_for_project( cls: tp.Type[ProviderType], project: tp.Type[Project] - ): + ) -> 'PatchProvider': """ Creates a provider instance for the given project. @@ -302,7 +305,7 @@ def create_provider_for_project( @classmethod def create_default_provider( cls: tp.Type[ProviderType], project: tp.Type[Project] - ): + ) -> 'PatchProvider': """ Creates a default provider instance that can be used with any project. @@ -315,10 +318,11 @@ def create_default_provider( @classmethod def _get_patches_repository_path(cls) -> Path: - return Path(target_prefix()) / cls.patches_source.local + # pathlib doesn't have type annotations for '/' + return tp.cast(Path, Path(target_prefix()) / cls.patches_source.local) @classmethod - def _update_local_patches_repo(cls): + def _update_local_patches_repo(cls) -> None: lock_path = Path(target_prefix()) / "patch_provider.lock" with lock_file(lock_path): diff --git a/varats-core/varats/utils/filesystem_util.py b/varats-core/varats/utils/filesystem_util.py index bc44c3265..258fb5e27 100644 --- a/varats-core/varats/utils/filesystem_util.py +++ b/varats-core/varats/utils/filesystem_util.py @@ -18,7 +18,8 @@ def __init__(self, folder: tp.Union[Path, str]) -> None: @contextmanager -def lock_file(lock_path: Path, lock_mode: int = fcntl.LOCK_EX) -> tp.Generator: +def lock_file(lock_path: Path, + lock_mode: int = fcntl.LOCK_EX) -> tp.Generator[None, None, None]: open_mode = os.O_RDWR | os.O_CREAT | os.O_TRUNC lock_fd = os.open(lock_path, open_mode) try: From d3ef7b6d7cd504422dcf43ee00782a95186a1075 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=B6hm?= Date: Sun, 24 Sep 2023 13:45:36 +0200 Subject: [PATCH 02/12] Draft workload patch filtering --- .../varats/experiment/workload_util.py | 33 +++------- varats-core/varats/project/project_util.py | 62 ++++++++++++++++--- varats/varats/projects/c_projects/xz.py | 2 +- 3 files changed, 62 insertions(+), 35 deletions(-) diff --git a/varats-core/varats/experiment/workload_util.py b/varats-core/varats/experiment/workload_util.py index 4566e2ee7..c0f4d4f63 100644 --- a/varats-core/varats/experiment/workload_util.py +++ b/varats-core/varats/experiment/workload_util.py @@ -20,7 +20,7 @@ ) from varats.experiment.experiment_util import get_extra_config_options -from varats.project.project_util import ProjectBinaryWrapper +from varats.project.project_util import ProjectBinaryWrapper, VCommand from varats.project.varats_project import VProject from varats.report.report import KeyedReportAggregate, ReportTy from varats.utils.exceptions import auto_unwrap @@ -96,31 +96,16 @@ def workload_commands( # Filter commands that have required args set. extra_options = set(get_extra_config_options(project)) - def requires_any_filter(prj_cmd: ProjectCommand) -> bool: - if hasattr( - prj_cmd.command, "requires_any" - ) and prj_cmd.command.requires_any: - args = set(prj_cmd.command._args).union(extra_options) - return bool(args.intersection(prj_cmd.command.requires_any)) + def filter_by_config(prj_cmd: ProjectCommand) -> bool: + # TODO: pass applied patches + if isinstance(prj_cmd.command, VCommand): + return prj_cmd.command.can_be_executed_by(extra_options, set()) return True - def requires_all_filter(prj_cmd: ProjectCommand) -> bool: - if hasattr( - prj_cmd.command, "requires_all" - ) and prj_cmd.command.requires_all: - args = set(prj_cmd.command._args).union(extra_options) - return bool(prj_cmd.command.requires_all.issubset(args)) - return True - - available_cmds = filter( - requires_all_filter, filter(requires_any_filter, project_cmds) - ) - - return list( - filter( - lambda prj_cmd: prj_cmd.path.name == binary.name, available_cmds - ) - ) + return [ + cmd for cmd in project_cmds + if cmd.path.name == binary.name and filter_by_config(cmd) + ] def create_workload_specific_filename( diff --git a/varats-core/varats/project/project_util.py b/varats-core/varats/project/project_util.py index 3028c438b..8a2965904 100644 --- a/varats-core/varats/project/project_util.py +++ b/varats-core/varats/project/project_util.py @@ -15,6 +15,9 @@ from varats.utils.settings import bb_cfg +if tp.TYPE_CHECKING: + from varats.provider.patch.patch_provider import Patch + LOG = logging.getLogger(__name__) @@ -390,8 +393,14 @@ class VCommand(Command): # type: ignore [misc] Wrapper around benchbuild's Command class. Attributes: - requires_any: sufficient args that must be available for successful execution. - requires_all: all args that must be available for successful execution. + requires_any_args: any of these command line args must be available for + successful execution. + requires_all_args: all of these command line args must be available for + successful execution. + requires_any_patch: any of these patch feature-tags must be available for + successful execution. + requires_all_patch: all of these patch feature-tags must be available for + successful execution. """ _requires: tp.Set[str] @@ -399,19 +408,52 @@ class VCommand(Command): # type: ignore [misc] def __init__( self, *args: tp.Any, - requires_any: tp.Optional[tp.Set[str]] = None, - requires_all: tp.Optional[tp.Set[str]] = None, + requires_any_args: tp.Optional[tp.Set[str]] = None, + requires_all_args: tp.Optional[tp.Set[str]] = None, + requires_any_patch: tp.Optional[tp.Set[str]] = None, + requires_all_patch: tp.Optional[tp.Set[str]] = None, **kwargs: tp.Union[str, tp.List[str]], ) -> None: super().__init__(*args, **kwargs) - self._requires_any = requires_any if requires_any else set() - self._requires_all = requires_all if requires_all else set() + self._requires_any_args = requires_any_args or set() + self._requires_all_args = requires_all_args or set() + self._requires_any_patch = requires_any_patch or set() + self._requires_all_patch = requires_all_patch or set() + + @property + def requires_any_args(self) -> tp.Set[str]: + return self._requires_any_args + + @property + def requires_all_args(self) -> tp.Set[str]: + return self._requires_all_args @property - def requires_any(self) -> tp.Set[str]: - return self._requires_any + def requires_any_patch(self) -> tp.Set[str]: + return self._requires_any_patch @property - def requires_all(self) -> tp.Set[str]: - return self._requires_all + def requires_all_patch(self) -> tp.Set[str]: + return self._requires_all_patch + + def can_be_executed_by( + self, extra_args: tp.Set[str], applied_patches: tp.Set['Patch'] + ) -> bool: + all_args = set(self._args).union(extra_args) + # TODO: extract tags from patches + all_patch_tags = {"" for patch in applied_patches} + + return bool(( + not self.requires_any_args or + all_args.intersection(self.requires_any_args) + ) and ( + not self.requires_all_args or + self.requires_all_args.issubset(all_args) + ) and ( + not self.requires_any_patch or + all_patch_tags.intersection(self.requires_any_patch) + ) and ( + not self.requires_all_patch or + self.requires_all_patch.issubset(all_patch_tags) + )) diff --git a/varats/varats/projects/c_projects/xz.py b/varats/varats/projects/c_projects/xz.py index 3299a0e08..3debddd18 100644 --- a/varats/varats/projects/c_projects/xz.py +++ b/varats/varats/projects/c_projects/xz.py @@ -111,7 +111,7 @@ class Xz(VProject): output=SourceRoot("geo-maps/countries-land-250m.geo.json"), label="countries-land-250m", creates=["geo-maps/countries-land-250m.geo.json.xz"], - requires_all={"--compress"}, + requires_all_args={"--compress"}, ) ], } From d450cdee4ee56112e412590e4368a90a76a558d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=B6hm?= Date: Sun, 24 Sep 2023 15:35:44 +0200 Subject: [PATCH 03/12] Add support for feature tags to patches --- .../varats/provider/patch/patch_provider.py | 43 ++++++++++++++++--- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/varats-core/varats/provider/patch/patch_provider.py b/varats-core/varats/provider/patch/patch_provider.py index 318da442a..15aea0512 100644 --- a/varats-core/varats/provider/patch/patch_provider.py +++ b/varats-core/varats/provider/patch/patch_provider.py @@ -38,7 +38,8 @@ def __init__( description: str, path: Path, valid_revisions: tp.Optional[tp.Set[CommitHash]] = None, - tags: tp.Optional[tp.Set[str]] = None + tags: tp.Optional[tp.Set[str]] = None, + feature_tags: tp.Optional[tp.Set[str]] = None ): self.project_name: str = project_name self.shortname: str = shortname @@ -47,6 +48,7 @@ def __init__( self.valid_revisions: tp.Set[ CommitHash] = valid_revisions if valid_revisions else set() self.tags: tp.Optional[tp.Set[str]] = tags + self.feature_tags: tp.Optional[tp.Set[str]] = feature_tags @staticmethod def from_yaml(yaml_path: Path) -> 'Patch': @@ -62,9 +64,8 @@ def from_yaml(yaml_path: Path) -> 'Patch': # the yaml info file. path = yaml_path.parent / path - tags = None - if "tags" in yaml_dict: - tags = yaml_dict["tags"] + tags = yaml_dict.get("tags") + feature_tags = yaml_dict.get("feature_tags") project_git_path = get_local_project_git_path(project_name) @@ -121,7 +122,8 @@ def parse_revisions( ) return Patch( - project_name, shortname, description, path, include_revisions, tags + project_name, shortname, description, path, include_revisions, tags, + feature_tags ) def __repr__(self) -> str: @@ -141,10 +143,13 @@ def __str__(self) -> str: return str_representation def __hash__(self) -> int: + hash_args = [self.shortname, self.path] if self.tags: - return hash((self.shortname, str(self.path), tuple(self.tags))) + hash_args += tuple(self.tags) + if self.feature_tags: + hash_args += tuple(self.feature_tags) - return hash((self.shortname, str(self.path))) + return hash(tuple(hash_args)) class PatchSet: @@ -216,6 +221,30 @@ def all_of(self, tags: tp.Union[str, tp.Iterable[str]]) -> "PatchSet": """ return self[tags] + def any_of_features(self, feature_tags: tp.Iterable[str]) -> "PatchSet": + """Returns a patch set with patches containing at least one of the given + feature tags.""" + tag_set = set(feature_tags) + result: tp.Set[Patch] = set() + for patch in self: + if patch.feature_tags and patch.feature_tags.intersection(tag_set): + result.add(patch) + + return PatchSet(result) + + def all_of_features( + self, feature_tags: tp.Union[str, tp.Iterable[str]] + ) -> "PatchSet": + """Returns a patch set with patches containing all the given feature + tags.""" + tag_set = set(feature_tags) + result: tp.Set[Patch] = set() + for patch in self: + if patch.feature_tags and tag_set.issubset(patch.feature_tags): + result.add(patch) + + return PatchSet(result) + def __hash__(self) -> int: return hash(self.__patches) From 0580c2fae83d489d35ca80587d6bf3d1208d5b51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=B6hm?= Date: Sun, 24 Sep 2023 15:37:01 +0200 Subject: [PATCH 04/12] Add PatchConfiguration --- varats-core/varats/base/configuration.py | 47 ++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/varats-core/varats/base/configuration.py b/varats-core/varats/base/configuration.py index 0c667dd2e..1387040dd 100644 --- a/varats-core/varats/base/configuration.py +++ b/varats-core/varats/base/configuration.py @@ -414,3 +414,50 @@ def get_config_value(self, option_name: str) -> tp.Optional[tp.Any]: def unfreeze(self) -> Configuration: return self + + +class PatchConfiguration(Configuration): + """Configuration class for projects where configuring is done by applying a + patch.""" + + def __init__(self, patch_feature_tags: tp.Set[str]): + self.__patch_feature_tags: tp.Set[ConfigurationOption] = { + ConfigurationOptionImpl(tag, tag) for tag in patch_feature_tags + } + + @staticmethod + def create_configuration_from_str(config_str: str) -> Configuration: + patch_feature_tags = json.loads(config_str) + return PatchConfiguration(patch_feature_tags) + + def add_config_option(self, option: ConfigurationOption) -> None: + self.__patch_feature_tags.add(option) + + def set_config_option(self, option_name: str, value: tp.Any) -> None: + self.__patch_feature_tags = { + option for option in self.__patch_feature_tags + if option.name != option_name + } + self.add_config_option(ConfigurationOptionImpl(option_name, value)) + + def get_config_value(self, option_name: str) -> tp.Optional[tp.Any]: + return any( + filter( + lambda option: option.name == option_name, + self.__patch_feature_tags + ) + ) + + def options(self) -> tp.List[ConfigurationOption]: + return list(self.__patch_feature_tags) + + def dump_to_string(self) -> str: + return ", ".join( + map(lambda option: str(option.value), self.__patch_feature_tags) + ) + + def freeze(self) -> FrozenConfiguration: + return FrozenConfiguration(deepcopy(self)) + + def unfreeze(self) -> Configuration: + return self From 95d304bc4f5b02592394334f12271dfa2dd7f64c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=B6hm?= Date: Sun, 24 Sep 2023 15:38:42 +0200 Subject: [PATCH 05/12] Add workload filter for patch based configuration --- .../varats/experiment/experiment_util.py | 67 ++++++++++++++----- .../varats/experiment/workload_util.py | 11 +-- varats-core/varats/paper/case_study.py | 15 ++++- varats-core/varats/project/project_util.py | 21 ++++-- 4 files changed, 89 insertions(+), 25 deletions(-) diff --git a/varats-core/varats/experiment/experiment_util.py b/varats-core/varats/experiment/experiment_util.py index 110782502..04eff65c6 100644 --- a/varats-core/varats/experiment/experiment_util.py +++ b/varats-core/varats/experiment/experiment_util.py @@ -10,7 +10,6 @@ from collections import defaultdict from pathlib import Path from types import TracebackType -from typing import Protocol, runtime_checkable from benchbuild import source from benchbuild.experiment import Experiment @@ -23,11 +22,16 @@ from plumbum.commands.base import BoundCommand import varats.revision.revisions as revs -from varats.base.configuration import PlainCommandlineConfiguration +from varats.base.configuration import ( + PlainCommandlineConfiguration, + PatchConfiguration, + Configuration, +) from varats.paper.paper_config import get_paper_config from varats.project.project_util import ProjectBinaryWrapper from varats.project.sources import FeatureSource from varats.project.varats_project import VProject +from varats.provider.patch.patch_provider import PatchSet, PatchProvider from varats.report.report import ( BaseReport, FileStatusExtension, @@ -696,20 +700,12 @@ def get_current_config_id(project: VProject) -> tp.Optional[int]: return None -def get_extra_config_options(project: VProject) -> tp.List[str]: - """ - Get extra program options that were specified in the particular - configuration of \a Project. - - Args: - project: to get the extra options for - - Returns: - list of command line options as string - """ +def get_config( + project: VProject, config_type: tp.Type[Configuration] +) -> tp.Optional[Configuration]: config_id = get_current_config_id(project) if config_id is None: - return [] + return None paper_config = get_paper_config() case_studies = paper_config.get_case_studies(cs_name=project.name) @@ -722,7 +718,7 @@ def get_extra_config_options(project: VProject) -> tp.List[str]: case_study = case_studies[0] config_map = load_configuration_map_for_case_study( - paper_config, case_study, PlainCommandlineConfiguration + paper_config, case_study, config_type ) config = config_map.get_configuration(config_id) @@ -732,4 +728,45 @@ def get_extra_config_options(project: VProject) -> tp.List[str]: "Requested config id was not in the map, but should be" ) + return config + + +def get_extra_config_options(project: VProject) -> tp.List[str]: + """ + Get extra program options that were specified in the particular + configuration of \a Project. + + Args: + project: to get the extra options for + + Returns: + list of command line options as string + """ + config = get_config(project, PlainCommandlineConfiguration) + if not config: + return [] return list(map(lambda option: option.value, config.options())) + + +def get_config_patches(project: VProject) -> PatchSet: + """ + Get required patches for the particular configuration of \a Project. + + Args: + project: to get the patches for + + Returns: + list of patches + """ + config = get_config(project, PatchConfiguration) + if not config: + return PatchSet(set()) + + patch_provider = PatchProvider.create_provider_for_project(project) + revision = ShortCommitHash(project.revision.primary.version) + feature_tags = {opt.value for opt in config.options()} + patches = patch_provider.get_patches_for_revision(revision).all_of_features( + feature_tags + ) + + return patches diff --git a/varats-core/varats/experiment/workload_util.py b/varats-core/varats/experiment/workload_util.py index c0f4d4f63..abf90b094 100644 --- a/varats-core/varats/experiment/workload_util.py +++ b/varats-core/varats/experiment/workload_util.py @@ -19,7 +19,10 @@ Command, ) -from varats.experiment.experiment_util import get_extra_config_options +from varats.experiment.experiment_util import ( + get_extra_config_options, + get_config_patches, +) from varats.project.project_util import ProjectBinaryWrapper, VCommand from varats.project.varats_project import VProject from varats.report.report import KeyedReportAggregate, ReportTy @@ -93,13 +96,13 @@ def workload_commands( ) ] - # Filter commands that have required args set. + # Filter commands that have required args and patches set. extra_options = set(get_extra_config_options(project)) + patches = get_config_patches(project) def filter_by_config(prj_cmd: ProjectCommand) -> bool: - # TODO: pass applied patches if isinstance(prj_cmd.command, VCommand): - return prj_cmd.command.can_be_executed_by(extra_options, set()) + return prj_cmd.command.can_be_executed_by(extra_options, patches) return True return [ diff --git a/varats-core/varats/paper/case_study.py b/varats-core/varats/paper/case_study.py index 627e96d01..32a7d389f 100644 --- a/varats-core/varats/paper/case_study.py +++ b/varats-core/varats/paper/case_study.py @@ -6,7 +6,11 @@ import benchbuild as bb -from varats.base.configuration import Configuration +from varats.base.configuration import ( + Configuration, + PlainCommandlineConfiguration, + PatchConfiguration, +) from varats.base.sampling_method import ( NormalSamplingMethod, SamplingMethodBase, @@ -214,6 +218,11 @@ class CaseStudy(): - a set of revisions """ + CONFIG_FILE_INDICES: tp.Dict[tp.Type[Configuration], int] = { + PlainCommandlineConfiguration: 1, + PatchConfiguration: 2 + } + def __init__( self, project_name: str, @@ -580,7 +589,9 @@ def load_configuration_map_from_case_study_file( version_header.raise_if_not_type("CaseStudy") version_header.raise_if_version_is_less_than(1) - next(documents) # Skip case study yaml-doc + config_index = CaseStudy.CONFIG_FILE_INDICES[concrete_config_type] + for _ in range(config_index): + next(documents) return create_configuration_map_from_yaml_doc( next(documents), concrete_config_type diff --git a/varats-core/varats/project/project_util.py b/varats-core/varats/project/project_util.py index 8a2965904..4a836f99d 100644 --- a/varats-core/varats/project/project_util.py +++ b/varats-core/varats/project/project_util.py @@ -16,7 +16,7 @@ from varats.utils.settings import bb_cfg if tp.TYPE_CHECKING: - from varats.provider.patch.patch_provider import Patch + from varats.provider.patch.patch_provider import Patch, PatchSet LOG = logging.getLogger(__name__) @@ -438,11 +438,24 @@ def requires_all_patch(self) -> tp.Set[str]: return self._requires_all_patch def can_be_executed_by( - self, extra_args: tp.Set[str], applied_patches: tp.Set['Patch'] + self, extra_args: tp.Set[str], applied_patches: 'PatchSet' ) -> bool: + """ + Checks whether this command can be executed with the give configuration. + + Args: + extra_args: additional command line arguments that will be passed to + the command + applied_patches: patches that were applied to create the executable + + Returns: + whether this command can be executed + """ all_args = set(self._args).union(extra_args) - # TODO: extract tags from patches - all_patch_tags = {"" for patch in applied_patches} + all_patch_tags: tp.Set[str] = set() + for patch in applied_patches: + if patch.feature_tags: + all_patch_tags.update(patch.feature_tags) return bool(( not self.requires_any_args or From e59470b8535b26efdbed3d2c259b0f157b1bb88c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=B6hm?= Date: Sun, 24 Sep 2023 16:32:04 +0200 Subject: [PATCH 06/12] Refactor config map loading --- .../SynthSAContextSensitivity_0.case_study | 1 + .../test_config_ids/xz_0.case_study | 1 + tests/paper/test_case_study.py | 1 + .../varats/mapping/configuration_map.py | 1 + varats-core/varats/paper/case_study.py | 26 ++++++++----------- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/TEST_INPUTS/paper_configs/test_config_ids/SynthSAContextSensitivity_0.case_study b/tests/TEST_INPUTS/paper_configs/test_config_ids/SynthSAContextSensitivity_0.case_study index 2a872480b..dd8b44c42 100644 --- a/tests/TEST_INPUTS/paper_configs/test_config_ids/SynthSAContextSensitivity_0.case_study +++ b/tests/TEST_INPUTS/paper_configs/test_config_ids/SynthSAContextSensitivity_0.case_study @@ -14,6 +14,7 @@ stages: version: 0 ... --- +config_type: PlainCommandlineConfiguration 0: '["--compress", "--mem", "10", "8"]' 1: '["--compress", "--mem", "300", "8"]' ... diff --git a/tests/TEST_INPUTS/paper_configs/test_config_ids/xz_0.case_study b/tests/TEST_INPUTS/paper_configs/test_config_ids/xz_0.case_study index e1101d9be..a544fbfbc 100644 --- a/tests/TEST_INPUTS/paper_configs/test_config_ids/xz_0.case_study +++ b/tests/TEST_INPUTS/paper_configs/test_config_ids/xz_0.case_study @@ -10,5 +10,6 @@ stages: config_ids: [1] version: 0 --- +config_type: PlainCommandlineConfiguration 0: '["--foo", "--bar"]' 1: '["--foo"]' \ No newline at end of file diff --git a/tests/paper/test_case_study.py b/tests/paper/test_case_study.py index 6d8412ee9..6a18fbbf3 100644 --- a/tests/paper/test_case_study.py +++ b/tests/paper/test_case_study.py @@ -48,6 +48,7 @@ commit_id: 494 ... --- +config_type: ConfigurationImpl 0: '{"foo": true, "bar": false, "bazz": "bazz-value", "buzz": "None"}' 1: '{}' 2: '{}' diff --git a/varats-core/varats/mapping/configuration_map.py b/varats-core/varats/mapping/configuration_map.py index f472c7d00..71a71122e 100644 --- a/varats-core/varats/mapping/configuration_map.py +++ b/varats-core/varats/mapping/configuration_map.py @@ -141,6 +141,7 @@ def create_configuration_map_from_yaml_doc( """ new_config_map = ConfigurationMap() + yaml_doc.pop("config_type", None) for config_id in sorted(yaml_doc): parsed_config = concrete_config_type.create_configuration_from_str( diff --git a/varats-core/varats/paper/case_study.py b/varats-core/varats/paper/case_study.py index 32a7d389f..673ec5523 100644 --- a/varats-core/varats/paper/case_study.py +++ b/varats-core/varats/paper/case_study.py @@ -6,11 +6,7 @@ import benchbuild as bb -from varats.base.configuration import ( - Configuration, - PlainCommandlineConfiguration, - PatchConfiguration, -) +from varats.base.configuration import Configuration from varats.base.sampling_method import ( NormalSamplingMethod, SamplingMethodBase, @@ -173,7 +169,7 @@ def get_config_ids_for_revision(self, revision: CommitHash) -> tp.List[int]: Returns a list of all configuration IDs specified for this revision. Args: - revision: i.e., a commit hash registed in this ``CSStage`` + revision: i.e., a commit hash registered in this ``CSStage`` Returns: list of config IDs """ @@ -218,11 +214,6 @@ class CaseStudy(): - a set of revisions """ - CONFIG_FILE_INDICES: tp.Dict[tp.Type[Configuration], int] = { - PlainCommandlineConfiguration: 1, - PatchConfiguration: 2 - } - def __init__( self, project_name: str, @@ -589,12 +580,17 @@ def load_configuration_map_from_case_study_file( version_header.raise_if_not_type("CaseStudy") version_header.raise_if_version_is_less_than(1) - config_index = CaseStudy.CONFIG_FILE_INDICES[concrete_config_type] - for _ in range(config_index): - next(documents) + next(documents) # skip case study document + while True: + document = next(documents) + if not document: + raise AssertionError("Configuration missing from case study file.") + + if document["config_type"] == concrete_config_type.__name__: + break return create_configuration_map_from_yaml_doc( - next(documents), concrete_config_type + document, concrete_config_type ) From e5a7d435624658dbac326028429c997d550d6708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=B6hm?= Date: Sun, 24 Sep 2023 16:33:46 +0200 Subject: [PATCH 07/12] Add config type header to docs --- docs/source/tutorials/configuration_specific_experiments.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/source/tutorials/configuration_specific_experiments.rst b/docs/source/tutorials/configuration_specific_experiments.rst index 16479538c..77f804b6a 100644 --- a/docs/source/tutorials/configuration_specific_experiments.rst +++ b/docs/source/tutorials/configuration_specific_experiments.rst @@ -21,6 +21,7 @@ One just needs to extend the case-study file of a project with a yaml document t .. code-block:: yaml --- + config_type: PlainCommandlineConfiguration 0: '["--foo", "--bar"]' 1: '["--foo"]' ... From 75fee71bfcbc76e85b3bcd952d0c978f31f34383 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=B6hm?= Date: Sun, 24 Sep 2023 17:01:31 +0200 Subject: [PATCH 08/12] PatchSet feature tag selection tests --- tests/provider/test_patch_provider.py | 125 ++++++++++++++++++++++---- 1 file changed, 110 insertions(+), 15 deletions(-) diff --git a/tests/provider/test_patch_provider.py b/tests/provider/test_patch_provider.py index 9acebb32e..9b55604e7 100644 --- a/tests/provider/test_patch_provider.py +++ b/tests/provider/test_patch_provider.py @@ -184,57 +184,120 @@ def setUpClass(cls) -> None: "Test-ABCD", "", path=Path("test.patch"), - tags={"A", "B", "C", "D"} + tags={"A", "B", "C", "D"}, + feature_tags={"F_A", "F_B", "F_C", "F_D"} ), - Patch("TEST", "Test-A", "", path=Path("test.patch"), tags={"A"}), - Patch("TEST", "Test-B", "", path=Path("test.patch"), tags={"B"}), - Patch("TEST", "Test-C", "", path=Path("test.patch"), tags={"C"}), - Patch("TEST", "Test-D", "", path=Path("test.patch"), tags={"D"}), Patch( - "TEST", "Test-AB", "", path=Path("test.patch"), tags={"A", "B"} + "TEST", + "Test-A", + "", + path=Path("test.patch"), + tags={"A"}, + feature_tags={"F_A"} + ), + Patch( + "TEST", + "Test-B", + "", + path=Path("test.patch"), + tags={"B"}, + feature_tags={"F_B"} + ), + Patch( + "TEST", + "Test-C", + "", + path=Path("test.patch"), + tags={"C"}, + feature_tags={"F_C"} ), Patch( - "TEST", "Test-AC", "", path=Path("test.patch"), tags={"A", "C"} + "TEST", + "Test-D", + "", + path=Path("test.patch"), + tags={"D"}, + feature_tags={"F_D"} + ), + Patch( + "TEST", + "Test-AB", + "", + path=Path("test.patch"), + tags={"A", "B"}, + feature_tags={"F_A", "F_B"} + ), + Patch( + "TEST", + "Test-AC", + "", + path=Path("test.patch"), + tags={"A", "C"}, + feature_tags={"F_A", "F_C"} ), Patch( - "TEST", "Test-AD", "", path=Path("test.patch"), tags={"A", "D"} + "TEST", + "Test-AD", + "", + path=Path("test.patch"), + tags={"A", "D"}, + feature_tags={"F_A", "F_D"} ), Patch( - "TEST", "Test-BC", "", path=Path("test.patch"), tags={"B", "C"} + "TEST", + "Test-BC", + "", + path=Path("test.patch"), + tags={"B", "C"}, + feature_tags={"F_B", "F_C"} ), Patch( - "TEST", "Test-BD", "", path=Path("test.patch"), tags={"B", "D"} + "TEST", + "Test-BD", + "", + path=Path("test.patch"), + tags={"B", "D"}, + feature_tags={"F_B", "F_D"} ), Patch( - "TEST", "Test-CD", "", path=Path("test.patch"), tags={"C", "D"} + "TEST", + "Test-CD", + "", + path=Path("test.patch"), + tags={"C", "D"}, + feature_tags={"F_C", "F_D"} ), Patch( "TEST", "Test-ABC", "", path=Path("test.patch"), - tags={"A", "B", "C"} + tags={"A", "B", "C"}, + feature_tags={"F_A", "F_B", "F_C"} ), Patch( "TEST", "Test-ABD", "", path=Path("test.patch"), - tags={"A", "B", "D"} + tags={"A", "B", "D"}, + feature_tags={"F_A", "F_B", "F_D"} ), Patch( "TEST", "Test-ACD", "", path=Path("test.patch"), - tags={"A", "C", "D"} + tags={"A", "C", "D"}, + feature_tags={"F_A", "F_C", "F_D"} ), Patch( "TEST", "Test-BCD", "", path=Path("test.patch"), - tags={"B", "C", "D"} + tags={"B", "C", "D"}, + feature_tags={"F_B", "F_C", "F_D"} ), } @@ -311,6 +374,38 @@ def test_any_of_multiple_tags(self): for patch in patches: any([tag in patch.tags for tag in tags]) + def test_all_of_single_feature_tag(self): + for tag in {"F_A", "F_B", "F_C", "F_D"}: + patches = self.patchSet.all_of_features([tag]) + self.assertEqual(8, len(patches)) + + def test_all_of_multiple_feature_tags(self): + tags_count = {("F_A", "F_B"): 4, + ("F_C", "F_B"): 4, + ("F_D", "F_B"): 4, + ("F_A", "F_B", "F_C"): 2, + ("F_A", "F_B", "F_C", "F_D"): 1} + + for tags in tags_count: + patches = self.patchSet.all_of_features(tags) + self.assertEqual(tags_count[tags], len(patches)) + + def test_any_of_single_feature_tag(self): + for tag in {"F_A", "F_B", "F_C", "F_D"}: + patches = self.patchSet.any_of_features([tag]) + self.assertEqual(8, len(patches)) + + def test_any_of_multiple_feature_tags(self): + tags_count = {("F_A", "F_B"): 12, + ("F_C", "F_B"): 12, + ("F_D", "F_B"): 12, + ("F_A", "F_B", "F_C"): 14, + ("F_A", "F_B", "F_C", "F_D"): 15} + + for tags in tags_count: + patches = self.patchSet.any_of_features(tags) + self.assertEqual(tags_count[tags], len(patches)) + def test_patchset_intersection(self): patches = self.patchSet["A"] & self.patchSet["B"] From 902829063b694ed42be0f927d5868f12f709103a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=B6hm?= Date: Sun, 24 Sep 2023 17:23:51 +0200 Subject: [PATCH 09/12] Resolve import cycle by moving VCommand to a new module --- .../varats/experiment/workload_util.py | 3 +- varats-core/varats/project/project_util.py | 88 ------------------ varats-core/varats/project/varats_command.py | 92 +++++++++++++++++++ varats/varats/projects/c_projects/xz.py | 2 +- 4 files changed, 95 insertions(+), 90 deletions(-) create mode 100644 varats-core/varats/project/varats_command.py diff --git a/varats-core/varats/experiment/workload_util.py b/varats-core/varats/experiment/workload_util.py index abf90b094..38b82720f 100644 --- a/varats-core/varats/experiment/workload_util.py +++ b/varats-core/varats/experiment/workload_util.py @@ -23,7 +23,8 @@ get_extra_config_options, get_config_patches, ) -from varats.project.project_util import ProjectBinaryWrapper, VCommand +from varats.project.project_util import ProjectBinaryWrapper +from varats.project.varats_command import VCommand from varats.project.varats_project import VProject from varats.report.report import KeyedReportAggregate, ReportTy from varats.utils.exceptions import auto_unwrap diff --git a/varats-core/varats/project/project_util.py b/varats-core/varats/project/project_util.py index 4a836f99d..a4c27d74d 100644 --- a/varats-core/varats/project/project_util.py +++ b/varats-core/varats/project/project_util.py @@ -7,7 +7,6 @@ import benchbuild as bb import pygit2 -from benchbuild.command import Command from benchbuild.source import Git from benchbuild.utils.cmd import git from plumbum import local @@ -15,9 +14,6 @@ from varats.utils.settings import bb_cfg -if tp.TYPE_CHECKING: - from varats.provider.patch.patch_provider import Patch, PatchSet - LOG = logging.getLogger(__name__) @@ -386,87 +382,3 @@ def copy_renamed_git_to_dest(src_dir: Path, dest_dir: Path) -> None: for name in dirs: if name == ".gitted": os.rename(os.path.join(root, name), os.path.join(root, ".git")) - - -class VCommand(Command): # type: ignore [misc] - """ - Wrapper around benchbuild's Command class. - - Attributes: - requires_any_args: any of these command line args must be available for - successful execution. - requires_all_args: all of these command line args must be available for - successful execution. - requires_any_patch: any of these patch feature-tags must be available for - successful execution. - requires_all_patch: all of these patch feature-tags must be available for - successful execution. - """ - - _requires: tp.Set[str] - - def __init__( - self, - *args: tp.Any, - requires_any_args: tp.Optional[tp.Set[str]] = None, - requires_all_args: tp.Optional[tp.Set[str]] = None, - requires_any_patch: tp.Optional[tp.Set[str]] = None, - requires_all_patch: tp.Optional[tp.Set[str]] = None, - **kwargs: tp.Union[str, tp.List[str]], - ) -> None: - - super().__init__(*args, **kwargs) - self._requires_any_args = requires_any_args or set() - self._requires_all_args = requires_all_args or set() - self._requires_any_patch = requires_any_patch or set() - self._requires_all_patch = requires_all_patch or set() - - @property - def requires_any_args(self) -> tp.Set[str]: - return self._requires_any_args - - @property - def requires_all_args(self) -> tp.Set[str]: - return self._requires_all_args - - @property - def requires_any_patch(self) -> tp.Set[str]: - return self._requires_any_patch - - @property - def requires_all_patch(self) -> tp.Set[str]: - return self._requires_all_patch - - def can_be_executed_by( - self, extra_args: tp.Set[str], applied_patches: 'PatchSet' - ) -> bool: - """ - Checks whether this command can be executed with the give configuration. - - Args: - extra_args: additional command line arguments that will be passed to - the command - applied_patches: patches that were applied to create the executable - - Returns: - whether this command can be executed - """ - all_args = set(self._args).union(extra_args) - all_patch_tags: tp.Set[str] = set() - for patch in applied_patches: - if patch.feature_tags: - all_patch_tags.update(patch.feature_tags) - - return bool(( - not self.requires_any_args or - all_args.intersection(self.requires_any_args) - ) and ( - not self.requires_all_args or - self.requires_all_args.issubset(all_args) - ) and ( - not self.requires_any_patch or - all_patch_tags.intersection(self.requires_any_patch) - ) and ( - not self.requires_all_patch or - self.requires_all_patch.issubset(all_patch_tags) - )) diff --git a/varats-core/varats/project/varats_command.py b/varats-core/varats/project/varats_command.py new file mode 100644 index 000000000..314a1ee55 --- /dev/null +++ b/varats-core/varats/project/varats_command.py @@ -0,0 +1,92 @@ +"""Custom version of benchbuild's Command for use with the VaRA-Tool-Suite.""" +import typing as tp + +from benchbuild.command import Command + +if tp.TYPE_CHECKING: + import varats.provider.patch.patch_provider as patch_provider + + +class VCommand(Command): # type: ignore [misc] + """ + Wrapper around benchbuild's Command class. + + Attributes: + requires_any_args: any of these command line args must be available for + successful execution. + requires_all_args: all of these command line args must be available for + successful execution. + requires_any_patch: any of these patch feature-tags must be available for + successful execution. + requires_all_patch: all of these patch feature-tags must be available for + successful execution. + """ + + _requires: tp.Set[str] + + def __init__( + self, + *args: tp.Any, + requires_any_args: tp.Optional[tp.Set[str]] = None, + requires_all_args: tp.Optional[tp.Set[str]] = None, + requires_any_patch: tp.Optional[tp.Set[str]] = None, + requires_all_patch: tp.Optional[tp.Set[str]] = None, + **kwargs: tp.Union[str, tp.List[str]], + ) -> None: + + super().__init__(*args, **kwargs) + self._requires_any_args = requires_any_args or set() + self._requires_all_args = requires_all_args or set() + self._requires_any_patch = requires_any_patch or set() + self._requires_all_patch = requires_all_patch or set() + + @property + def requires_any_args(self) -> tp.Set[str]: + return self._requires_any_args + + @property + def requires_all_args(self) -> tp.Set[str]: + return self._requires_all_args + + @property + def requires_any_patch(self) -> tp.Set[str]: + return self._requires_any_patch + + @property + def requires_all_patch(self) -> tp.Set[str]: + return self._requires_all_patch + + def can_be_executed_by( + self, extra_args: tp.Set[str], + applied_patches: 'patch_provider.PatchSet' + ) -> bool: + """ + Checks whether this command can be executed with the give configuration. + + Args: + extra_args: additional command line arguments that will be passed to + the command + applied_patches: patches that were applied to create the executable + + Returns: + whether this command can be executed + """ + all_args = set(self._args).union(extra_args) + all_patch_tags: tp.Set[str] = set() + for patch in applied_patches: + if patch.feature_tags: + all_patch_tags.update(patch.feature_tags) + + return bool(( + not self.requires_any_args or + all_args.intersection(self.requires_any_args) + ) and ( + not self.requires_all_args or + self.requires_all_args.issubset(all_args) + ) and ( + not self.requires_any_patch or + all_patch_tags.intersection(self.requires_any_patch) + ) and ( + not self.requires_all_patch or + self.requires_all_patch.issubset(all_patch_tags) + )) diff --git a/varats/varats/projects/c_projects/xz.py b/varats/varats/projects/c_projects/xz.py index 3debddd18..3d1a580ed 100644 --- a/varats/varats/projects/c_projects/xz.py +++ b/varats/varats/projects/c_projects/xz.py @@ -18,13 +18,13 @@ from varats.paper.paper_config import PaperConfigSpecificGit from varats.project.project_domain import ProjectDomains from varats.project.project_util import ( - VCommand, ProjectBinaryWrapper, get_local_project_git_path, BinaryType, verify_binaries, ) from varats.project.sources import FeatureSource +from varats.project.varats_command import VCommand from varats.project.varats_project import VProject from varats.utils.git_util import ( ShortCommitHash, From 7cd84fa194d62bf9b664d1f069b7e5b973ad7ab8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=B6hm?= Date: Mon, 25 Sep 2023 10:53:36 +0200 Subject: [PATCH 10/12] Add helper function to apply configuration patches --- .../varats/experiment/experiment_util.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/varats-core/varats/experiment/experiment_util.py b/varats-core/varats/experiment/experiment_util.py index 04eff65c6..b4361a27c 100644 --- a/varats-core/varats/experiment/experiment_util.py +++ b/varats-core/varats/experiment/experiment_util.py @@ -27,6 +27,7 @@ PatchConfiguration, Configuration, ) +from varats.experiment.steps.patch import ApplyPatch from varats.paper.paper_config import get_paper_config from varats.project.project_util import ProjectBinaryWrapper from varats.project.sources import FeatureSource @@ -770,3 +771,21 @@ def get_config_patches(project: VProject) -> PatchSet: ) return patches + + +def get_config_patch_steps(project: VProject) -> tp.MutableSequence[Step]: + """ + Get a list of actions that apply all configuration patches to the project. + + Args: + project: the project to be configured + + Returns: + the actions that configure the project + """ + return list( + map( + lambda patch: ApplyPatch(project, patch), + get_config_patches(project) + ) + ) From d684ba1a38b6f22023e34f29a792e50e0cb1ddc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=B6hm?= Date: Mon, 25 Sep 2023 16:36:24 +0200 Subject: [PATCH 11/12] Fix config map edge cases --- .../varats/experiment/experiment_util.py | 5 ----- varats-core/varats/paper/case_study.py | 19 ++++++++++--------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/varats-core/varats/experiment/experiment_util.py b/varats-core/varats/experiment/experiment_util.py index b4361a27c..bad60ba6f 100644 --- a/varats-core/varats/experiment/experiment_util.py +++ b/varats-core/varats/experiment/experiment_util.py @@ -724,11 +724,6 @@ def get_config( config = config_map.get_configuration(config_id) - if config is None: - raise AssertionError( - "Requested config id was not in the map, but should be" - ) - return config diff --git a/varats-core/varats/paper/case_study.py b/varats-core/varats/paper/case_study.py index 673ec5523..3fb087596 100644 --- a/varats-core/varats/paper/case_study.py +++ b/varats-core/varats/paper/case_study.py @@ -581,17 +581,18 @@ def load_configuration_map_from_case_study_file( version_header.raise_if_version_is_less_than(1) next(documents) # skip case study document - while True: - document = next(documents) - if not document: - raise AssertionError("Configuration missing from case study file.") + try: + while True: + document = next(documents) - if document["config_type"] == concrete_config_type.__name__: - break + if document["config_type"] == concrete_config_type.__name__: + break - return create_configuration_map_from_yaml_doc( - document, concrete_config_type - ) + return create_configuration_map_from_yaml_doc( + document, concrete_config_type + ) + except StopIteration: + return ConfigurationMap() def store_case_study(case_study: CaseStudy, case_study_location: Path) -> None: From 1978199746ddff4c2c208a2f74aa9b561853e4be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20B=C3=B6hm?= Date: Wed, 27 Sep 2023 11:07:18 +0200 Subject: [PATCH 12/12] Split any(filter()) into two statements to aid type checking --- varats-core/varats/base/configuration.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/varats-core/varats/base/configuration.py b/varats-core/varats/base/configuration.py index 1387040dd..cdf6cb8e5 100644 --- a/varats-core/varats/base/configuration.py +++ b/varats-core/varats/base/configuration.py @@ -441,12 +441,11 @@ def set_config_option(self, option_name: str, value: tp.Any) -> None: self.add_config_option(ConfigurationOptionImpl(option_name, value)) def get_config_value(self, option_name: str) -> tp.Optional[tp.Any]: - return any( - filter( - lambda option: option.name == option_name, - self.__patch_feature_tags - ) + filtered_options = filter( + lambda option: (option.name == option_name), + self.__patch_feature_tags ) + return any(filtered_options) def options(self) -> tp.List[ConfigurationOption]: return list(self.__patch_feature_tags)