From db6d08269316da87ac704d7483bdbf905e772e8c Mon Sep 17 00:00:00 2001 From: "Brandon T. Willard" Date: Tue, 4 Jun 2024 13:45:47 -0500 Subject: [PATCH 1/4] ENH: Replace use of Result.env in profiling code --- asv/commands/profiling.py | 17 ++++++++++++----- asv/results.py | 2 +- test/test_profile.py | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/asv/commands/profiling.py b/asv/commands/profiling.py index fc32dc8cf..c413101db 100644 --- a/asv/commands/profiling.py +++ b/asv/commands/profiling.py @@ -140,15 +140,22 @@ def run(cls, conf, benchmark, revision=None, gui=None, output=None, conf.results_dir, machine_name): if hash_equal(commit_hash, result.commit_hash): if result.has_profile(benchmark): - env_matched = any(result.env.name == env.name - for env in environments) + # Only take the first one + env_matched = next( + ( + env + for env in environments + if result.env_name == env.name + ), + None, + ) if env_matched: - if result.env.name not in checked_out: + if result.env_name not in checked_out: # We need to checkout the correct commit so that # the line numbers in the profile data match up with # what's in the source tree. - result.env.checkout_project(repo, commit_hash) - checked_out.add(result.env.name) + env_matched.checkout_project(repo, commit_hash) + checked_out.add(result.env_name) profile_data = result.get_profile(benchmark) break diff --git a/asv/results.py b/asv/results.py index 579127070..c309d3f56 100644 --- a/asv/results.py +++ b/asv/results.py @@ -573,7 +573,7 @@ def has_profile(self, benchmark_name): """ Does the given benchmark data have profiling information? """ - return benchmark_name in self._profiles + return self._profiles.get(benchmark_name) def save(self, result_dir): """ diff --git a/test/test_profile.py b/test/test_profile.py index ecaab0b0d..fb7f6af41 100644 --- a/test/test_profile.py +++ b/test/test_profile.py @@ -1,5 +1,7 @@ import re +from asv import util + from . import tools @@ -17,3 +19,33 @@ def test_profile_python_same(capsys, basic_conf): # Check that it did not clone or install assert "Cloning" not in text assert "Installing" not in text + + +def test_profile_python_commit(capsys, basic_conf): + + tmpdir, local, conf, machine_file = basic_conf + + # Create initial results with no profile results + tools.run_asv_with_conf(conf, 'run', "--quick", "--bench=time_secondary.track_value", + f'{util.git_default_branch()}^!', _machine_file=machine_file) + text, err = capsys.readouterr() + + assert "Installing" in text + + # Query the previous empty results results; there should be no issues here + tools.run_asv_with_conf(conf, 'profile', "time_secondary.track_value", + f'{util.git_default_branch()}', _machine_file=machine_file) + text, err = capsys.readouterr() + + assert "Profile data does not already exist" in text + + tools.run_asv_with_conf(conf, 'run', "--quick", "--profile", + "--bench=time_secondary.track_value", + f'{util.git_default_branch()}^!', _machine_file=machine_file) + + # Profile results should be present now + tools.run_asv_with_conf(conf, 'profile', "time_secondary.track_value", + f'{util.git_default_branch()}', _machine_file=machine_file) + text, err = capsys.readouterr() + + assert "Profile data does not already exist" not in text From 24deb7f02a54314503f40e78a87051f2454bf453 Mon Sep 17 00:00:00 2001 From: Rohit Goswami Date: Mon, 12 Aug 2024 03:58:21 -0700 Subject: [PATCH 2/4] MAINT: Lint and refactor --- asv/commands/profiling.py | 17 ++++++++--------- asv/plugins/conda.py | 2 +- asv/util.py | 11 +++++++++++ 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/asv/commands/profiling.py b/asv/commands/profiling.py index c413101db..ee3290e84 100644 --- a/asv/commands/profiling.py +++ b/asv/commands/profiling.py @@ -135,20 +135,16 @@ def run(cls, conf, benchmark, revision=None, gui=None, output=None, # First, we see if we already have the profile in the results # database + env = None if not force and commit_hash: for result in iter_results_for_machine( conf.results_dir, machine_name): if hash_equal(commit_hash, result.commit_hash): if result.has_profile(benchmark): # Only take the first one - env_matched = next( - ( - env - for env in environments - if result.env_name == env.name - ), - None, - ) + env_matched = util.get_matching_environment(environments, result) + raise(ValueError(f"Matching environment found: {env_matched.name}")) + if env_matched: if result.env_name not in checked_out: # We need to checkout the correct commit so that @@ -157,6 +153,7 @@ def run(cls, conf, benchmark, revision=None, gui=None, output=None, env_matched.checkout_project(repo, commit_hash) checked_out.add(result.env_name) profile_data = result.get_profile(benchmark) + env = env_matched break if profile_data is None: @@ -171,7 +168,9 @@ def run(cls, conf, benchmark, revision=None, gui=None, output=None, "An explicit revision may not be specified when " "using an existing environment.") - env = environments[0] + if env is None: + # Fallback + env = environments[0] if env.python != "{0}.{1}".format(*sys.version_info[:2]): raise util.UserError( diff --git a/asv/plugins/conda.py b/asv/plugins/conda.py index a31f2e04e..2819f4a67 100644 --- a/asv/plugins/conda.py +++ b/asv/plugins/conda.py @@ -154,7 +154,7 @@ def _setup(self): self._run_conda(['env', 'create', '-f', env_file_name, '-p', self._path, "--yes"], env=env) - else: # Backward compatbility + else: # Backward compatibility self._run_conda(['env', 'create', '-f', env_file_name, '-p', self._path, '--force'], env=env) diff --git a/asv/util.py b/asv/util.py index d8755a381..5c5fcf80b 100644 --- a/asv/util.py +++ b/asv/util.py @@ -1414,3 +1414,14 @@ def construct_pip_call(pip_caller, parsed_declaration: ParsedPipDeclaration): ON_PYPY = True else: ON_PYPY = False + +def get_matching_environment(environments, result=None): + return next( + ( + env + for env in environments + if (result is None or result.env_name == env.name) + and env.python == "{0}.{1}".format(*sys.version_info[:2]) + ), + None, + ) From 79f3f0d4a141978abf76c7b9a94ad4b7a28af448 Mon Sep 17 00:00:00 2001 From: Rohit Goswami Date: Mon, 12 Aug 2024 03:58:29 -0700 Subject: [PATCH 3/4] TST: Fallback failures on PyPy --- test/test_profile.py | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/test/test_profile.py b/test/test_profile.py index fb7f6af41..463d9f70f 100644 --- a/test/test_profile.py +++ b/test/test_profile.py @@ -1,5 +1,7 @@ import re +import pytest + from asv import util from . import tools @@ -33,19 +35,31 @@ def test_profile_python_commit(capsys, basic_conf): assert "Installing" in text # Query the previous empty results results; there should be no issues here - tools.run_asv_with_conf(conf, 'profile', "time_secondary.track_value", - f'{util.git_default_branch()}', _machine_file=machine_file) - text, err = capsys.readouterr() - - assert "Profile data does not already exist" in text - - tools.run_asv_with_conf(conf, 'run', "--quick", "--profile", - "--bench=time_secondary.track_value", - f'{util.git_default_branch()}^!', _machine_file=machine_file) + if not util.ON_PYPY: + tools.run_asv_with_conf(conf, 'profile', "time_secondary.track_value", + f'{util.git_default_branch()}', _machine_file=machine_file) + text, err = capsys.readouterr() + + assert "Profile data does not already exist" in text + + tools.run_asv_with_conf(conf, 'run', "--quick", "--profile", + "--bench=time_secondary.track_value", + f'{util.git_default_branch()}^!', _machine_file=machine_file) + else: + # The ASV main process doesn't use PyPy + with pytest.raises(util.UserError): + tools.run_asv_with_conf(conf, 'profile', "time_secondary.track_value", + f'{util.git_default_branch()}', _machine_file=machine_file) # Profile results should be present now - tools.run_asv_with_conf(conf, 'profile', "time_secondary.track_value", - f'{util.git_default_branch()}', _machine_file=machine_file) - text, err = capsys.readouterr() - - assert "Profile data does not already exist" not in text + if not util.ON_PYPY: + tools.run_asv_with_conf(conf, 'profile', "time_secondary.track_value", + f'{util.git_default_branch()}', _machine_file=machine_file) + text, err = capsys.readouterr() + + assert "Profile data does not already exist" not in text + else: + # The ASV main process doesn't use PyPy + with pytest.raises(util.UserError): + tools.run_asv_with_conf(conf, 'profile', "time_secondary.track_value", + f'{util.git_default_branch()}', _machine_file=machine_file) From e40eb18626b68a95a81af570c07883407dfe277a Mon Sep 17 00:00:00 2001 From: Rohit Goswami Date: Mon, 12 Aug 2024 06:05:01 -0700 Subject: [PATCH 4/4] MAINT: Fix debug typo --- asv/commands/profiling.py | 1 - 1 file changed, 1 deletion(-) diff --git a/asv/commands/profiling.py b/asv/commands/profiling.py index ee3290e84..94736126b 100644 --- a/asv/commands/profiling.py +++ b/asv/commands/profiling.py @@ -143,7 +143,6 @@ def run(cls, conf, benchmark, revision=None, gui=None, output=None, if result.has_profile(benchmark): # Only take the first one env_matched = util.get_matching_environment(environments, result) - raise(ValueError(f"Matching environment found: {env_matched.name}")) if env_matched: if result.env_name not in checked_out: