From 11acbfa825a1d67efecff750933332246b544ed7 Mon Sep 17 00:00:00 2001 From: Robin Rehrmann Date: Tue, 3 Jun 2025 16:10:47 +0200 Subject: [PATCH 1/6] Polishing the interface Some of the command-line flags do not provide enough information, e.g., what is the default time-out? Others don't work as expected, e.g., --list-passes fails, when no TEST_CASE is provided. In addition to fixing those, this change tries to hint about possible errors, right in the beginning. Such as: * Not enough disk space available. The default parallel setting (`-n`/ `--n`) does not take into account the disk space. For large input files of several GB, we may run out of disk, before we run out of CPU. Therefore, this change provides a parallel setting calculation that takes disk space into account. If the user sets parallelism through the command line, the satisfaction of disk space requirements are checked. A warning is printed on the command line, if such test fails, but the execution continues. * Interestingness test check already exceeds the timeout. This change measures the initial check of the interestingness test. If that already fails the given timeout, a warning is issued, but the execution continues. * Creating backups may fail. When *.orig files already exist, TEST_CASEs are not copied. This change prints a warning, if this is the case. All warnings described above may be switched off by setting `-w` (similar to the gcc `-w` flag). --- cvise.py | 70 +++++++++++++++++-------------- cvise/passes/clangbinarysearch.py | 2 +- cvise/utils/testing.py | 60 ++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 33 deletions(-) diff --git a/cvise.py b/cvise.py index f33cd9820..42d22e70e 100755 --- a/cvise.py +++ b/cvise.py @@ -85,29 +85,6 @@ def get_available_pass_groups(): return group_names -def get_available_cores(): - try: - # try to detect only physical cores, ignore HyperThreading - # in order to speed up parallel execution - core_count = psutil.cpu_count(logical=False) - if not core_count: - core_count = psutil.cpu_count(logical=True) - # respect affinity - try: - affinity = len(psutil.Process().cpu_affinity()) - assert affinity >= 1 - except AttributeError: - return core_count - - if core_count: - core_count = min(core_count, affinity) - else: - core_count = affinity - return core_count - except NotImplementedError: - return 1 - - EPILOG_TEXT = f""" available shortcuts: S - skip execution of the current pass @@ -118,6 +95,7 @@ def get_available_cores(): """ if __name__ == '__main__': + default_timeout_value = 300 parser = argparse.ArgumentParser( description='C-Vise', formatter_class=argparse.RawDescriptionHelpFormatter, @@ -127,8 +105,8 @@ def get_available_cores(): '--n', '-n', type=int, - default=get_available_cores(), - help='Number of cores to use; C-Vise tries to automatically pick a good setting but its choice may be too low or high for your situation', + default=-1, + help=f'Number of cores to use; C-Vise tries to automatically pick a good setting but its choice may be too low or high for your situation. Usually defaults to {testing.TestManager.get_available_cores()}. May be less on large TEST_CASEs.', ) parser.add_argument( '--tidy', @@ -161,6 +139,12 @@ def get_available_cores(): action='store_true', help='Print debug information (alias for --log-level=DEBUG)', ) + parser.add_argument( + '-w', + action='store_false', + default=True, + help='Do not double-check command-line arguments.', + ) parser.add_argument( '--log-level', type=str, @@ -218,8 +202,8 @@ def get_available_cores(): '--timeout', type=int, nargs='?', - default=300, - help='Interestingness test timeout in seconds', + default=-1, + help=f'Interestingness test timeout in seconds. Defaults to {default_timeout_value}.', ) parser.add_argument('--no-cache', action='store_true', help="Don't cache behavior of passes") parser.add_argument( @@ -238,7 +222,7 @@ def get_available_cores(): '--pass-group', type=str, choices=get_available_pass_groups(), - help='Set of passes used during the reduction', + help='Set of passes used during the reduction.', ) passes_group.add_argument('--pass-group-file', type=str, help='JSON file defining a custom pass group') parser.add_argument( @@ -262,7 +246,7 @@ def get_available_cores(): action='store_true', help='Enable all renaming passes (that are disabled by default)', ) - parser.add_argument('--list-passes', action='store_true', help='Print all available passes and exit') + parser.add_argument('--list-passes', action='store_true', help='Print all available passes and exit. Works with --not-c (showing available passes not specific to C and C++), --renaming (showing available renaming passes), and --sllooww (showing availalbe passes, activated with the --sllooww option).') parser.add_argument( '--version', action='version', @@ -274,7 +258,7 @@ def get_available_cores(): '-c', help='Use shell commands instead of an interestingness test case', ) - parser.add_argument('--shell', default='bash', help='Use selected shell for the --commands option') + parser.add_argument('--shell', default='bash', help='Use selected shell for the --commands option. Defaults to "bash"') parser.add_argument( '--to-utf8', action='store_true', @@ -296,10 +280,17 @@ def get_available_cores(): '--stopping-threshold', default=1.0, type=float, - help='CVise will stop reducing a test case once it has reduced by this fraction of its original size. Between 0.0 and 1.0.', + help='CVise will stop reducing a test case once it has reduced by this fraction of its original size. Between 0.0 and 1.0. Defaults to 1.0', ) - args = parser.parse_args() + # TEST_CASE may not be required, when `--list-passes` is provided. But the parser doesn't know. + # Thus, we insert a dummy element so that the parser won't complain. + if '--list-passes' in sys.argv: + cmd_args = sys.argv[1:] + cmd_args.append("/dev/null") + args = parser.parse_args(cmd_args) + else: + args = parser.parse_args() log_config = {} @@ -358,13 +349,21 @@ def get_available_cores(): ) if args.list_passes: logging.info('Available passes:') + logging.info('==============') logging.info('INITIAL PASSES') + logging.info('==============') for p in pass_group['first']: logging.info(str(p)) + logging.info('') + logging.info('==============') logging.info('MAIN PASSES') + logging.info('==============') for p in pass_group['main']: logging.info(str(p)) + logging.info('') + logging.info('==============') logging.info('CLEANUP PASSES') + logging.info('==============') for p in pass_group['last']: logging.info(str(p)) sys.exit(0) @@ -408,6 +407,12 @@ def get_available_cores(): logging.info(f'Using temporary interestingness test: {script.name}') args.interestingness_test = script.name + if args.timeout <= 0: + args.timeout = default_timeout_value + else: + from cvise.passes import ClangBinarySearchPass + ClangBinarySearchPass.QUERY_TIMEOUT = args.timeout + test_manager = testing.TestManager( pass_statistic, args.interestingness_test, @@ -426,6 +431,7 @@ def get_available_cores(): args.start_with_pass, args.skip_after_n_transforms, args.stopping_threshold, + args.w, ) reducer = CVise(test_manager, args.skip_interestingness_test_check) diff --git a/cvise/passes/clangbinarysearch.py b/cvise/passes/clangbinarysearch.py index 69cca32c8..a9acbfff2 100644 --- a/cvise/passes/clangbinarysearch.py +++ b/cvise/passes/clangbinarysearch.py @@ -65,7 +65,7 @@ def count_instances(self, test_case): proc = subprocess.run(cmd, text=True, capture_output=True, timeout=self.QUERY_TIMEOUT) except subprocess.TimeoutExpired: logging.warning( - f'clang_delta --query-instances (--std={self.clang_delta_std}) {self.QUERY_TIMEOUT}s timeout reached' + f'clang_delta --query-instances (--std={self.clang_delta_std}) {self.QUERY_TIMEOUT}s timeout reached. Cmd: {cmd}' ) return 0 except subprocess.SubprocessError as e: diff --git a/cvise/utils/testing.py b/cvise/utils/testing.py index 00460c877..a7ce7b7b5 100644 --- a/cvise/utils/testing.py +++ b/cvise/utils/testing.py @@ -14,6 +14,7 @@ import tempfile import traceback import concurrent.futures +import time from cvise.cvise import CVise from cvise.passes.abstract import PassResult, ProcessEventNotifier, ProcessEventType @@ -161,6 +162,7 @@ def __init__( start_with_pass, skip_after_n_transforms, stopping_threshold, + print_warnings, ): self.test_script = Path(test_script).absolute() self.timeout = timeout @@ -180,6 +182,7 @@ def __init__( self.start_with_pass = start_with_pass self.skip_after_n_transforms = skip_after_n_transforms self.stopping_threshold = stopping_threshold + self.print_warnings = print_warnings for test_case in test_cases: test_case = Path(test_case) @@ -206,6 +209,23 @@ def __init__( == 0 ) + if self.parallel_tests <= 0: + logging.debug("Trying to find a good estimate for parallel tests.") + available_cores = self.get_available_cores() + available_space_per_process = max(int(self.get_free_temp_space()/self.total_file_size)-1, 1) + # i.e., parallel_tests = min(available_cores, available_space_per_process) + if available_cores < available_space_per_process: + logging.debug(f'Setting parallelism to {available_cores}, because we have less available cores than space in {self.get_tempdirname()}') + self.parallel_tests = available_cores + else: + logging.debug(f'Setting parallelism to {available_space_per_process}, because we have less space available in {self.get_tempdirname()}, than we have available cores.') + self.parallel_tests = available_space_per_process + elif self.print_warnings: + free_space = self.get_free_temp_space() + if self.orig_total_file_size * self.parallel_tests >= free_space: + tempdir = self.get_tempdirname() + logging.warning(f'You chose an input set of {self.orig_total_file_size} Bytes and a parallelism of {self.parallel_tests} interestingness tests. This may require {self.orig_total_file_size * self.parallel_tests} Bytes of available disk space. However, your temp directory ({tempdir}) only has {free_space} Bytes available. Please consider using less parallel interestingness tests (we propose {int(free_space/self.orig_total_file_size)} at max), a different temp directory by setting the TMPDIR environment variable, or freeing space in {tempdir}, to not run into No Disk Space Available errors!') + def create_root(self): pass_name = str(self.current_pass).replace('::', '-') self.root = tempfile.mkdtemp(prefix=f'{self.TEMP_PREFIX}{pass_name}-') @@ -219,6 +239,10 @@ def restore_mode(self): for test_case in self.test_cases: test_case.chmod(self.test_cases_modes[test_case]) + @staticmethod + def get_tempdirname(): + return os.getenv('TMPDIR') or '/tmp' + @classmethod def is_valid_test(cls, test_script): for mode in {os.F_OK, os.X_OK}: @@ -226,6 +250,11 @@ def is_valid_test(cls, test_script): return False return True + def get_free_temp_space(self): + tempdir = os.getenv('TMPDIR') or '/tmp' + usage = shutil.disk_usage(tempdir) + return usage.free + @property def total_file_size(self): return self.get_file_size(self.test_cases) @@ -251,6 +280,29 @@ def get_line_count(files): lines += len([line for line in f.readlines() if line and not line.isspace()]) return lines + @staticmethod + def get_available_cores(): + try: + # try to detect only physical cores, ignore HyperThreading + # in order to speed up parallel execution + core_count = psutil.cpu_count(logical=False) + if not core_count: + core_count = psutil.cpu_count(logical=True) + # respect affinity + try: + affinity = len(psutil.Process().cpu_affinity()) + assert affinity >= 1 + except AttributeError: + return core_count + + if core_count: + core_count = min(core_count, affinity) + else: + core_count = affinity + return core_count + except NotImplementedError: + return 1 + def backup_test_cases(self): for f in self.test_cases: orig_file = Path(f'{f}.orig') @@ -258,6 +310,8 @@ def backup_test_cases(self): if not orig_file.exists(): # Copy file and preserve attributes shutil.copy2(f, orig_file) + elif self.print_warnings: + logger.warning(f'Could not create backup of {f}, as {orig_file} already exists.') @staticmethod def check_file_permissions(path, modes, error): @@ -336,10 +390,16 @@ def check_sanity(self, verbose=False): test_env = TestEnvironment(None, 0, self.test_script, folder, list(self.test_cases)[0], self.test_cases, None) logging.debug(f'sanity check tmpdir = {test_env.folder}') + time_start = time.monotonic() returncode = test_env.run_test(verbose) + time_stop = time.monotonic() if returncode == 0: rmfolder(folder) logging.debug('sanity check successful') + if self.print_warnings: + time_diff = time_stop - time_start + if time_diff > self.timeout: + logging.warning(f'Timeout is set to {self.timeout} seconds. However, the sanity check already took {time_diff:.6f} seconds. Please consider increasing the timeout. Otherwise, you may run into timeout issues, later!') else: if not self.save_temps: rmfolder(folder) From 05dd9d340653ef118dd83da97fb073d2717fb3a0 Mon Sep 17 00:00:00 2001 From: Robin Rehrmann Date: Wed, 18 Mar 2026 15:41:21 +0100 Subject: [PATCH 2/6] Adding a Clear pass This pass clears a file and checks, whether that file is required for reproducing the observed bug, at all. This may be helpful, when building libraries with hundreds of files. --- cvise/cvise.py | 2 ++ cvise/passes/clear.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 cvise/passes/clear.py diff --git a/cvise/cvise.py b/cvise/cvise.py index 178d87c29..33ac5f5ca 100644 --- a/cvise/cvise.py +++ b/cvise/cvise.py @@ -5,6 +5,7 @@ from cvise.passes.abstract import AbstractPass from cvise.passes.balanced import BalancedPass from cvise.passes.blank import BlankPass +from cvise.passes.clear import ClearPass from cvise.passes.clang import ClangPass from cvise.passes.clangbinarysearch import ClangBinarySearchPass from cvise.passes.clex import ClexPass @@ -41,6 +42,7 @@ class Info: 'blank': BlankPass, 'clang': ClangPass, 'clangbinarysearch': ClangBinarySearchPass, + 'clear': ClearPass, 'clex': ClexPass, 'comments': CommentsPass, 'gcda-binary': GCDABinaryPass, diff --git a/cvise/passes/clear.py b/cvise/passes/clear.py new file mode 100644 index 000000000..75f228002 --- /dev/null +++ b/cvise/passes/clear.py @@ -0,0 +1,31 @@ +import os +import shutil +import tempfile + +from cvise.passes.abstract import AbstractPass, PassResult + + +class ClearPass(AbstractPass): + def check_prerequisites(self): + return True + + def new(self, test_case, _=None): + return 0 + + def advance(self, test_case, state): + return state + 1 + + def advance_on_success(self, test_case, state): + return state + + @staticmethod + def __transform(test_case): + if os.path.getsize(test_case) == 0: + return False + tmp = os.path.dirname(test_case) + tmp_file = tempfile.NamedTemporaryFile(mode='w+', delete=False, dir=tmp) + shutil.move(tmp_file.name, test_case) + return True + + def transform(self, test_case, state, process_event_notifier): + return (PassResult.OK if self.__transform(test_case) else PassResult.STOP, state) From 762892d7d7bbfd29d7e8af0ff727044750b1f690 Mon Sep 17 00:00:00 2001 From: Robin Rehrmann Date: Wed, 18 Mar 2026 15:51:12 +0100 Subject: [PATCH 3/6] Adding max timeout count to clangbinarysearch When clangbinarysearch fails due to a timeout, current implementation simply retries. For large projects, this may take minutes to hours. This change introduces a max timeout count (currently set to 20) and a current timeout count. When clangbinarysearch failed `max timeout count` times, due to a timeout, it stops. --- cvise/passes/clangbinarysearch.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cvise/passes/clangbinarysearch.py b/cvise/passes/clangbinarysearch.py index a9acbfff2..d36e834d1 100644 --- a/cvise/passes/clangbinarysearch.py +++ b/cvise/passes/clangbinarysearch.py @@ -11,6 +11,8 @@ class ClangBinarySearchPass(AbstractPass): QUERY_TIMEOUT = 10 + QUERY_MAX_TIMEOUTS = 30 + QUERY_CUR_TIMEOUT = 0 def check_prerequisites(self): return self.check_external_program('clang_delta') @@ -51,6 +53,8 @@ def advance_on_success(self, test_case, state): return state def count_instances(self, test_case): + if self.QUERY_CUR_TIMEOUT >= self.QUERY_MAX_TIMEOUTS: + return 0 assert self.clang_delta_std args = [ self.external_programs['clang_delta'], @@ -64,8 +68,9 @@ def count_instances(self, test_case): try: proc = subprocess.run(cmd, text=True, capture_output=True, timeout=self.QUERY_TIMEOUT) except subprocess.TimeoutExpired: + self.QUERY_CUR_TIMEOUT += 1 logging.warning( - f'clang_delta --query-instances (--std={self.clang_delta_std}) {self.QUERY_TIMEOUT}s timeout reached. Cmd: {cmd}' + f'[{self.QUERY_CUR_TIMEOUT:{len(str(self.QUERY_MAX_TIMEOUTS))}}/{self.QUERY_MAX_TIMEOUTS:{len(str(self.QUERY_MAX_TIMEOUTS))}}] clang_delta --query-instances (--std={self.clang_delta_std}) {self.QUERY_TIMEOUT}s timeout reached. Cmd: {cmd}' ) return 0 except subprocess.SubprocessError as e: From 8047fae71813ca73bc42e232e48cfb05b844a57d Mon Sep 17 00:00:00 2001 From: Robin Rehrmann Date: Wed, 18 Mar 2026 16:06:27 +0100 Subject: [PATCH 4/6] Introducing KeyboardInterrupt Exception By throwin an exception on KeyboardInterrupt, cvise can still print out statistics, when user decides to cancel the run. --- cvise/utils/error.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cvise/utils/error.py b/cvise/utils/error.py index cae01b97f..ed25ea763 100644 --- a/cvise/utils/error.py +++ b/cvise/utils/error.py @@ -90,6 +90,11 @@ def __str__(self): return 'Could not find a directory with definitions for pass groups!' +class KeyboardInterruption(CViseError): + def __str__(self): + return 'Got Ctrl+C' + + class PassBugError(CViseError): MSG = """*************************************************** From 42050643474d7fcc5f52f81816e9a5855e226030 Mon Sep 17 00:00:00 2001 From: Robin Rehrmann Date: Wed, 18 Mar 2026 16:07:39 +0100 Subject: [PATCH 5/6] Adding progress to statistics When printing an overview at the end of the run, it makes sense to also add the progress per run (in bytes). Thus, user may explicitly exclude passes that take a lot of time, but make little progress. --- cvise/utils/statistics.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cvise/utils/statistics.py b/cvise/utils/statistics.py index dffd676ff..1c31cb2cb 100644 --- a/cvise/utils/statistics.py +++ b/cvise/utils/statistics.py @@ -8,6 +8,7 @@ def __init__(self, pass_name): self.worked = 0 self.failed = 0 self.totally_executed = 0 + self.improvement = 0 class PassStatistic: @@ -39,6 +40,10 @@ def add_success(self, pass_): pass_name = repr(pass_) self.stats[pass_name].worked += 1 + def add_improvement(self, pass_, bytes_): + pass_name = repr(pass_) + self.stats[pass_name].improvement += bytes_ + def add_failure(self, pass_): pass_name = repr(pass_) self.stats[pass_name].failed += 1 From 79708440d7544a78d3b0f96b84571f064d1fdc9a Mon Sep 17 00:00:00 2001 From: Robin Rehrmann Date: Wed, 18 Mar 2026 16:17:00 +0100 Subject: [PATCH 6/6] Adding parameters and fixing bug Adding a parameter for min_improvement: Currently, only max_improvement is supported. However, in large projects it may be undesired to waste days on a pass that only makes a few byte with each pass. With min_improvement, pass which make little progress are skipped faster and passes that make larger progress are executed sooner. Also there seems to be a bug when accounting for the improvement of a pass on a file. I fixed that. --- cvise.py | 10 +++- cvise/utils/testing.py | 127 ++++++++++++++++++++++++++++++++++------- 2 files changed, 113 insertions(+), 24 deletions(-) diff --git a/cvise.py b/cvise.py index 42d22e70e..997c909b1 100755 --- a/cvise.py +++ b/cvise.py @@ -217,6 +217,12 @@ def get_available_pass_groups(): type=int, help='Largest improvement in file size from a single transformation that C-Vise should accept (useful only to slow C-Vise down)', ) + parser.add_argument( + '--min-improvement', + metavar='BYTES', + type=int, + help='Minimum improvement in file size from a single transformation that C-Vise should accept', + ) passes_group = parser.add_mutually_exclusive_group() passes_group.add_argument( '--pass-group', @@ -409,9 +415,6 @@ def get_available_pass_groups(): if args.timeout <= 0: args.timeout = default_timeout_value - else: - from cvise.passes import ClangBinarySearchPass - ClangBinarySearchPass.QUERY_TIMEOUT = args.timeout test_manager = testing.TestManager( pass_statistic, @@ -426,6 +429,7 @@ def get_available_pass_groups(): args.die_on_pass_bug, args.print_diff, args.max_improvement, + args.min_improvement, args.no_give_up, args.also_interesting, args.start_with_pass, diff --git a/cvise/utils/testing.py b/cvise/utils/testing.py index a7ce7b7b5..3696a36fd 100644 --- a/cvise/utils/testing.py +++ b/cvise/utils/testing.py @@ -15,6 +15,7 @@ import traceback import concurrent.futures import time +import random from cvise.cvise import CVise from cvise.passes.abstract import PassResult, ProcessEventNotifier, ProcessEventType @@ -24,6 +25,7 @@ from cvise.utils.error import InvalidTestCaseError from cvise.utils.error import PassBugError from cvise.utils.error import ZeroSizeError +from cvise.utils.error import KeyboardInterruption from cvise.utils.misc import is_readable_file from cvise.utils.readkey import KeyLogger import pebble @@ -62,6 +64,7 @@ def __init__( all_test_cases, transform, pid_queue=None, + min_improvement=0, ): self.state = state self.folder = folder @@ -76,6 +79,7 @@ def __init__( self.test_case = test_case self.base_size = test_case.stat().st_size self.all_test_cases = all_test_cases + self.min_improvement = min_improvement # Copy files to the created folder for test_case in all_test_cases: @@ -109,6 +113,10 @@ def run(self): self.result = result if self.result != PassResult.OK: return self + if self.size_improvement < 0: + logging.debug(f'Order {self.order:02d}: Negative improvement on file {self.test_case}: {self.size_improvement} B') + self.result = PassResult.INVALID + return self # run test script self.exitcode = self.run_test(False) @@ -157,6 +165,7 @@ def __init__( die_on_pass_bug, print_diff, max_improvement, + min_improvement, no_give_up, also_interesting, start_with_pass, @@ -177,6 +186,7 @@ def __init__( self.die_on_pass_bug = die_on_pass_bug self.print_diff = print_diff self.max_improvement = max_improvement + self.min_improvement = min_improvement if min_improvement else 1 self.no_give_up = no_give_up self.also_interesting = also_interesting self.start_with_pass = start_with_pass @@ -347,25 +357,25 @@ def report_pass_bug(self, test_env, problem): logging.warning(f'{self.current_pass} has encountered a non fatal bug: {problem}') crash_dir = self.get_extra_dir(self.BUG_DIR_PREFIX, self.MAX_CRASH_DIRS) - + if crash_dir is None: return False - + crash_dir.mkdir() test_env.dump(crash_dir) - + if not self.die_on_pass_bug: logging.debug( f'Please consider tarring up {crash_dir} and creating an issue at https://github.com/marxin/cvise/issues and we will try to fix the bug.' ) - + with (crash_dir / 'PASS_BUG_INFO.TXT').open(mode='w') as info_file: info_file.write(f'Package: {CVise.Info.PACKAGE_STRING}\n') info_file.write(f'Git version: {CVise.Info.GIT_VERSION}\n') info_file.write(f'LLVM version: {CVise.Info.LLVM_VERSION}\n') info_file.write(f'System: {str(platform.uname())}\n') info_file.write(PassBugError.MSG.format(self.current_pass, problem, test_env.state, crash_dir)) - + if self.die_on_pass_bug: raise PassBugError(self.current_pass, problem, test_env.state, crash_dir) else: @@ -438,6 +448,8 @@ def kill_pid_queue(self): child.kill() except psutil.NoSuchProcess: pass + except psutil.AccessDenied: + pass except psutil.NoSuchProcess: pass @@ -510,9 +522,13 @@ def check_pass_result(self, test_env): if self.max_improvement is not None and test_env.size_improvement > self.max_improvement: logging.debug(f'Too large improvement: {test_env.size_improvement} B') return PassCheckingOutcome.IGNORE + if self.min_improvement is not None and test_env.size_improvement < self.min_improvement: + logging.debug(f'Too small improvement on file {test_env.test_case}: {round((test_env.size_improvement / test_env.base_size)*100, 2)}%, i.e., {test_env.size_improvement} B') + return PassCheckingOutcome.ACCEPT # Report bug if transform did not change the file if filecmp.cmp(self.current_test_case, test_env.test_case_path): if not self.silent_pass_bug: + logging.debug(f'pass failed to modify the variant') if not self.report_pass_bug(test_env, 'pass failed to modify the variant'): return PassCheckingOutcome.QUIT_LOOP return PassCheckingOutcome.IGNORE @@ -530,9 +546,11 @@ def check_pass_result(self, test_env): self.report_pass_bug(test_env, 'pass error') return PassCheckingOutcome.QUIT_LOOP - if not self.no_give_up and test_env.order > self.GIVEUP_CONSTANT: + if not self.no_give_up and test_env.order > min(self.GIVEUP_CONSTANT, self.skip_after_n_transforms if + self.skip_after_n_transforms is not None else self.GIVEUP_CONSTANT): if not self.giveup_reported: - self.report_pass_bug(test_env, 'pass got stuck') + if self.skip_after_n_transforms is None: + self.report_pass_bug(test_env, 'pass got stuck') self.giveup_reported = True return PassCheckingOutcome.QUIT_LOOP return PassCheckingOutcome.IGNORE @@ -570,6 +588,7 @@ def run_parallel_tests(self): self.test_cases, self.current_pass.transform, self.pid_queue, + self.min_improvement, ) future = pool.schedule(test_env.run, timeout=self.timeout) self.temporary_folders[future] = folder @@ -609,11 +628,38 @@ def run_pass(self, pass_): if not self.skip_key_off: logger = KeyLogger() + num_too_small_improvements = 0 + num_failures = 0 + self.skip = False try: - for test_case in self.sorted_test_cases: + curr_i=0 + num_testcases = len(self.test_cases) + test_case_order = [] + + shuffled_test_cases = list(self.test_cases) + random.shuffle(shuffled_test_cases) + stop_val = 15 + stop_val = self.current_pass.max_transforms if self.current_pass.max_transforms is not None else stop_val + stop_val = self.skip_after_n_transforms if self.skip_after_n_transforms is not None else stop_val + for test_case in shuffled_test_cases: + if self.skip: + logging.info(f'Skipping the rest of {self.current_pass} after {curr_i}/{num_testcases} runs, because of user request.') + break + # Too many files did not make enough progress at all with this pass -> bail out + if num_too_small_improvements >= stop_val: + logging.info(f'Cancelling {self.current_pass} after {curr_i}/{num_testcases} runs, because it does not make enough progress.') + break + if num_failures >= stop_val: + # Do not bail out too early + if (curr_i / (num_failures*2)) < 1: + logging.info(f'Cancelling {self.current_pass} after {curr_i}/{num_testcases} runs, because it had {num_failures} failures.') + break + # File is too small -> We're not going to make enough progress, here + if test_case.stat().st_size < self.min_improvement: + logging.debug(f'Skipping {test_case}, because its size {test_case.stat().st_size} Bytes is less than the required min improvement of {self.min_improvement} Bytes') + continue + curr_i += 1 self.current_test_case = test_case - starting_test_case_size = test_case.stat().st_size - success_count = 0 if self.get_file_size([test_case]) == 0: continue @@ -629,31 +675,38 @@ def run_pass(self, pass_): logging.info(f'cache hit for {test_case}') continue + starting_test_case_size = test_case.stat().st_size # create initial state self.state = self.current_pass.new(self.current_test_case, self.check_sanity) - self.skip = False + success_count = 0 + num_success=0 while self.state is not None and not self.skip: # Ignore more key presses after skip has been detected if not self.skip_key_off and not self.skip: key = logger.pressed_key() - if key == 's': - self.skip = True - self.log_key_event('skipping the rest of this pass') - elif key == 'd': - self.log_key_event('toggle print diff') - self.print_diff = not self.print_diff - + if key is not None: + if key.lower() == 's': + self.skip = True + self.log_key_event('skipping the rest of this pass') + break + elif key.lower() == 'd': + self.log_key_event('toggle print diff') + self.print_diff = not self.print_diff + + previous_test_case_size = test_case.stat().st_size success_env = self.run_parallel_tests() self.kill_pid_queue() if success_env: + num_failures = max(0, num_failures-1) self.process_result(success_env) success_count += 1 # if the file increases significantly, bail out the current pass test_case_size = self.current_test_case.stat().st_size - if test_case_size >= MAX_PASS_INCREASEMENT_THRESHOLD * starting_test_case_size: + self.pass_statistic.add_improvement(self.current_pass, previous_test_case_size - test_case_size) + if test_case_size >= MAX_PASS_INCREASEMENT_THRESHOLD * previous_test_case_size: logging.info( f'skipping the rest of the pass (huge file increasement ' f'{MAX_PASS_INCREASEMENT_THRESHOLD * 100}%)' @@ -663,7 +716,11 @@ def run_pass(self, pass_): self.release_folders() self.futures.clear() if not success_env: - break + logging.debug(f'{self.current_pass} failed for {test_case}') + num_failures += 1 + num_success -= 1 + if num_success < 0: + break # skip after N transformations if requested if (self.skip_after_n_transforms and success_count >= self.skip_after_n_transforms) or ( @@ -672,6 +729,32 @@ def run_pass(self, pass_): logging.info(f'skipping after {success_count} successful transformations') break + # This testcase does not make enough progress for the current iteration. Now, we decide: + # (1) Did the current pass not make enough progress at all on this file? + # E.g., We could have very well met the criteria twice, but failed it thrice - in + # that case, we certainly don't want to count this as not making enough progress. + # (2) Did the current pass make enough progress in this iteration? + # + # In the first case, we might need to continue with the next pass, in the second case, + # only with the next testcase + if (previous_test_case_size - test_case_size) < self.min_improvement: + # Here, we compare the size of the testcase before the pass, to decide whether we + # should count this as not making enough progress overall. + if (starting_test_case_size - test_case_size) < self.min_improvement: + # (1) Does the pass make progress at all? + # -> If not continue with next pass + num_too_small_improvements+=1 + # (2) Does the pass make progress for this test case? + # -> If not continue with next test case + num_success -= 1 + if num_success < 0: + logging.info('Skipping due to small improvement.') + break + else: + num_too_small_improvements = max(0, num_too_small_improvements-1) + num_success += 1 + + # Cache result of this pass if not self.no_cache: with open(test_case, mode='rb') as tmp_file: @@ -685,8 +768,10 @@ def run_pass(self, pass_): self.remove_root() except KeyboardInterrupt: logging.info('Exiting now ...') + self.release_folders() + self.futures.clear() self.remove_root() - sys.exit(1) + raise KeyboardInterruption() def process_result(self, test_env): if self.print_diff: