Skip to content

Commit

Permalink
Make time processing optional, abbreviate home with proper Pathlib APIs
Browse files Browse the repository at this point in the history
This commit contains two main fixes:
1) Time values are now processed conditionally on whether a "time_unit" key is found in the benchmark object.
2) The `abbrev_home` function now collapses the home directory with only pathlib's APIs.
  • Loading branch information
nicholasjng committed Feb 8, 2022
1 parent 466a054 commit 597ed41
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 28 deletions.
34 changes: 18 additions & 16 deletions pybm/commands/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class RunCommand(CLICommand):
def __init__(self):
super(RunCommand, self).__init__(name="run")
self.config = PybmConfig.load()
self.use_legacy_checkout: bool = self.config.get_value("git.legacycheckout")

def add_arguments(self):
self.parser.add_argument(
Expand Down Expand Up @@ -56,8 +55,8 @@ def add_arguments(self):
action="store_true",
default=False,
help="Run benchmarks in checkout mode in environment 'root'. Here, instead "
"of persisted git worktrees, different refs are benchmarked using "
"`git checkout` commands.",
"of persisted git worktrees, different refs are benchmarked using `git "
"checkout` commands.",
)
self.parser.add_argument(
"--all",
Expand All @@ -73,14 +72,14 @@ def add_arguments(self):
default=None,
dest="source_ref",
metavar="<git-ref>",
help="Source benchmark targets from a different git reference.",
help="Source benchmark targets from a different git reference <git-ref>.",
)
self.parser.add_argument(
"--repetitions",
type=int,
default=5,
metavar="<N>",
help="Number of repetitions for the target benchmarks.",
help="Number of times to repeat the target benchmarks.",
)
self.parser.add_argument(
"--filter",
Expand All @@ -98,8 +97,8 @@ def add_arguments(self):
dest="benchmark_context",
metavar="<context>",
help="Additional global context, given as strings in the format "
"--context='key'='value'. Keys must be unique. Supplying two or more "
"context values for the same key results in an error.",
"--context='key'='value'. Keys must be unique, supplying more than one "
"value for the same key results in an error.",
)

runner: BaseRunner = get_component_class("runner", config=self.config)
Expand Down Expand Up @@ -128,18 +127,21 @@ def run(self, args: List[str]) -> int:
runner: BaseRunner = get_component_class("runner", config=self.config)
reporter: BaseReporter = get_component_class("reporter", config=self.config)

# whether to use legacy checkouts (git < 2.17)
use_legacy_checkout: bool = self.config.get_value("git.legacycheckout")

verbose: bool = runner_options.pop("verbose")

env_ids: List[str] = runner_options.pop("environments") or []
run_all: bool = runner_options.pop("run_all")
checkout_mode: bool = runner_options.pop("checkout")
source_ref: Optional[str] = runner_options.pop("source_ref")
source_path = Path(runner_options.pop("benchmarks"))
run_as_module = runner_options.pop("run_as_module")
run_as_module: bool = runner_options.pop("run_as_module")

# runner dispatch arguments
repetitions = runner_options.pop("repetitions")
benchmark_filter = runner_options.pop("benchmark_filter")
repetitions: int = runner_options.pop("repetitions")
benchmark_filter: Optional[str] = runner_options.pop("benchmark_filter")
benchmark_context = runner_options.pop("benchmark_context")
# at this point, runner_options only include the additional runner kwargs

Expand Down Expand Up @@ -198,7 +200,7 @@ def run(self, args: List[str]) -> int:
worktree=worktree,
source_path=source_path,
source_ref=source_ref,
use_legacy_checkout=self.use_legacy_checkout,
use_legacy_checkout=use_legacy_checkout,
) as benchmark_targets:
n = len(benchmark_targets)
if n > 0:
Expand All @@ -208,9 +210,9 @@ def run(self, args: List[str]) -> int:
)
else:
msg = (
f"Benchmark selector {str(source_path)!r} did not "
f"match any directory or Python files for {ref_type} "
f"{ref!r} in environment {environment.name!r}."
f"Benchmark selector {str(source_path)!r} did not match any "
f"directory or Python files for {ref_type} {ref!r} in "
f"environment {environment.name!r}."
)

if runner.fail_fast:
Expand Down Expand Up @@ -247,8 +249,8 @@ def run(self, args: List[str]) -> int:
"benchmark subprocess. Please check that the configured "
"benchmark runner actually writes the results to stdout."
)
else:
reporter.write(ref, benchmark, data)

reporter.write(ref, benchmark, data)

if checkout_mode:
root_ref, root_type = root_checkout
Expand Down
4 changes: 2 additions & 2 deletions pybm/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
disambiguate_info,
resolve_ref,
is_main_worktree,
checkout,
checkout as git_checkout,
resolve_commit,
)
from pybm.util.path import current_folder
Expand Down Expand Up @@ -325,7 +325,7 @@ def switch(self, worktree: Worktree, ref: str, ref_type: Optional[str] = None):
f"Object {ref!r} could not be understood as a valid git reference."
)

checkout(ref=ref, cwd=worktree.root)
git_checkout(ref=ref, cwd=worktree.root)

# null the old reference type if necessary
if ref_type != old_type:
Expand Down
18 changes: 13 additions & 5 deletions pybm/reporters/console.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,26 @@ def process(bm: Dict[str, Any], target_time_unit: str, shalength: int):
bm["name"] = format_benchmark(bm.pop("name"), bm.pop("executable"))
bm["reference"] = format_ref(bm.pop("ref"), bm.pop("commit"), shalength=shalength)

time_unit: str = bm.pop("time_unit")
time_values: Dict[str, Any] = dfilter_regex(".*time", bm)
time_unit: Optional[str] = bm.pop("time_unit", None)

rescale_fn = partial(rescale, current_unit=time_unit, target_unit=target_time_unit)
if time_unit is not None:
time_values: Dict[str, Any] = dfilter_regex(".*time", bm)

bm.update(dvmap(rescale_fn, time_values))
rescale_fn = partial(
rescale, current_unit=time_unit, target_unit=target_time_unit
)

bm.update(dvmap(rescale_fn, time_values))

return bm


def compare(results: List[Dict[str, Any]]):
"""Compare results between different refs with respect to an anchor ref. Assumes
that the results are sorted in the same order."""
if len(results) == 1:
return results

anchor_result = results[0]

for result in results:
Expand Down Expand Up @@ -152,7 +159,7 @@ def compare(

processed_results = lmap(process_fn, reduced)

# group results by again benchmark name
# group results again by benchmark name
grouped_results = groupby("name", processed_results)

if absolute:
Expand All @@ -164,6 +171,7 @@ def compare(
formatted_results = lmap(transform_fn, compared_results)

log_to_console(formatted_results, padding=self.padding)
# TODO: Print summary about improvements etc.

def transform_result(self, bm: Dict[str, Any], anchor_ref: str) -> Dict[str, str]:
"""Finalize column header names, cast all values to string and
Expand Down
1 change: 0 additions & 1 deletion pybm/reporters/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ def log_to_console(results: List[Dict[str, str]], padding: int = 1):
print(make_separator(column_widths, padding=padding))

print(make_line(res.values(), column_widths, padding=padding))
# TODO: Print summary about improvements etc.


def reduce(results: List[Dict[str, Any]]) -> Dict[str, Any]:
Expand Down
13 changes: 9 additions & 4 deletions pybm/util/print.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,14 @@


def abbrev_home(path: Union[str, Path]) -> str:
homepath = Path(path).resolve().relative_to(Path.home())
return str(Path("~") / homepath)
path = Path(path).resolve()
try:
homepath = path.relative_to(Path.home())
abbrev_path = str(Path("~") / homepath)
except ValueError:
# case where the env path is not a subpath of the home directory
abbrev_path = str(path)
return abbrev_path


def calculate_column_widths(data: Iterable[Iterable[str]]) -> List[int]:
Expand All @@ -17,9 +23,8 @@ def calculate_column_widths(data: Iterable[Iterable[str]]) -> List[int]:
def make_line(values: Iterable[str], column_widths: Iterable[int], padding: int) -> str:
pad_char = " " * padding
sep = "|".join([pad_char] * 2)
offset = pad_char

line = offset + sep.join(f"{n:<{w}}" for n, w in zip(values, column_widths))
line = pad_char + sep.join(f"{n:<{w}}" for n, w in zip(values, column_widths))
return line


Expand Down

0 comments on commit 597ed41

Please sign in to comment.