From d56b5e6d79a7455832688ce5714a12442ac1f252 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 22 Jun 2020 09:53:26 -0500 Subject: [PATCH 1/5] Faster benchmark diff, use pandas to print a comparison table for human consumption --- dev/archery/archery/benchmark/google.py | 5 +--- dev/archery/archery/benchmark/runner.py | 9 ++++-- dev/archery/archery/cli.py | 39 +++++++++++++++++++++---- dev/archery/setup.py | 1 + 4 files changed, 42 insertions(+), 12 deletions(-) diff --git a/dev/archery/archery/benchmark/google.py b/dev/archery/archery/benchmark/google.py index 5b2176c7194..a3cc1a13340 100644 --- a/dev/archery/archery/benchmark/google.py +++ b/dev/archery/archery/benchmark/google.py @@ -30,9 +30,6 @@ def partition(pred, iterable): return list(filter(pred, t1)), list(filterfalse(pred, t2)) -DEFAULT_REPETITIONS = 10 - - class GoogleBenchmarkCommand(Command): """ Run a google benchmark binary. @@ -52,7 +49,7 @@ def list_benchmarks(self): stderr=subprocess.PIPE) return str.splitlines(result.stdout.decode("utf-8")) - def results(self, repetitions=DEFAULT_REPETITIONS): + def results(self, repetitions=1): with NamedTemporaryFile() as out: argv = ["--benchmark_repetitions={}".format(repetitions), "--benchmark_out={}".format(out.name), diff --git a/dev/archery/archery/benchmark/runner.py b/dev/archery/archery/benchmark/runner.py index e36696bb7fb..509324c5248 100644 --- a/dev/archery/archery/benchmark/runner.py +++ b/dev/archery/archery/benchmark/runner.py @@ -34,10 +34,15 @@ def regex_filter(re_expr): return lambda s: re_comp.search(s) +DEFAULT_REPETITIONS = 1 + + class BenchmarkRunner: - def __init__(self, suite_filter=None, benchmark_filter=None): + def __init__(self, suite_filter=None, benchmark_filter=None, + repetitions=DEFAULT_REPETITIONS): self.suite_filter = suite_filter self.benchmark_filter = benchmark_filter + self.repetitions = repetitions @property def suites(self): @@ -158,7 +163,7 @@ def suite(self, name, suite_bin): if not benchmark_names: return None - results = suite_cmd.results() + results = suite_cmd.results(repetitions=self.repetitions) benchmarks = GoogleBenchmark.from_json(results.get("benchmarks")) return BenchmarkSuite(name, benchmarks) diff --git a/dev/archery/archery/cli.py b/dev/archery/archery/cli.py index a8a779951e8..95b57304747 100644 --- a/dev/archery/archery/cli.py +++ b/dev/archery/archery/cli.py @@ -16,6 +16,7 @@ # under the License. from collections import namedtuple +from io import StringIO import click import errno import json @@ -467,6 +468,9 @@ def benchmark_run(ctx, rev_or_path, src, preserve, output, cmake_extras, @click.option("--threshold", type=float, default=DEFAULT_THRESHOLD, show_default=True, help="Regression failure threshold in percentage.") +@click.option("--repetitions", type=int, default=1, show_default=True, + help=("Number of repetitions of each benchmark. Increasing " + "may improve result precision.")) @click.argument("contender", metavar="[", default=ArrowSources.WORKSPACE, required=False) @click.argument("baseline", metavar="[]]", default="origin/master", @@ -474,7 +478,7 @@ def benchmark_run(ctx, rev_or_path, src, preserve, output, cmake_extras, @click.pass_context def benchmark_diff(ctx, src, preserve, output, cmake_extras, suite_filter, benchmark_filter, - threshold, contender, baseline, **kwargs): + repetitions, threshold, contender, baseline, **kwargs): """Compare (diff) benchmark runs. This command acts like git-diff but for benchmark results. @@ -554,17 +558,40 @@ def benchmark_diff(ctx, src, preserve, output, cmake_extras, runner_cont = BenchmarkRunner.from_rev_or_path( src, root, contender, conf, - suite_filter=suite_filter, benchmark_filter=benchmark_filter) + repetitions=repetitions, + suite_filter=suite_filter, + benchmark_filter=benchmark_filter) runner_base = BenchmarkRunner.from_rev_or_path( src, root, baseline, conf, - suite_filter=suite_filter, benchmark_filter=benchmark_filter) + repetitions=repetitions, + suite_filter=suite_filter, + benchmark_filter=benchmark_filter) runner_comp = RunnerComparator(runner_cont, runner_base, threshold) # TODO(kszucs): test that the output is properly formatted jsonlines - for comparator in runner_comp.comparisons: - json.dump(comparator, output, cls=JsonEncoder) - output.write("\n") + comparisons_json = _get_comparisons_as_json(runner_comp.comparisons) + formatted = _format_comparisons_with_pandas(comparisons_json) + output.write(formatted) + + +def _get_comparisons_as_json(comparisons): + buf = StringIO() + for comparator in comparisons: + json.dump(comparator, buf, cls=JsonEncoder) + buf.write("\n") + + return buf.getvalue() + + +def _format_comparisons_with_pandas(comparisons_json): + import pandas as pd + df = pd.read_json(StringIO(comparisons_json), lines=True) + # parse change % so we can sort by it + df['change %'] = df.pop('change').str[:-1].map(float) + df = df[['benchmark', 'baseline', 'contender', 'change %', 'regression']] + df = df.sort_values(by='change %', ascending=False) + return df.to_string() # ---------------------------------------------------------------------- diff --git a/dev/archery/setup.py b/dev/archery/setup.py index 8bcd45d665a..dd1b55542b4 100755 --- a/dev/archery/setup.py +++ b/dev/archery/setup.py @@ -25,6 +25,7 @@ sys.exit('Python < 3.5 is not supported') extras = { + 'benchmark': ['pandas'], 'bot': ['ruamel.yaml', 'pygithub'], 'docker': ['ruamel.yaml', 'python-dotenv'] } From 5ecdd24e27d299f69f66c4f8983b1f48be04c4f7 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 22 Jun 2020 11:35:17 -0500 Subject: [PATCH 2/5] Always enable ARROW_IPC --- cpp/CMakeLists.txt | 3 ++- dev/archery/archery/benchmark/runner.py | 14 ++++++++++++-- dev/archery/archery/lang/cpp.py | 2 +- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 8c05d4b84d0..6000d3161a6 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -300,7 +300,8 @@ if(ARROW_BUILD_BENCHMARKS set(ARROW_JSON ON) endif() -if(ARROW_CUDA OR ARROW_FLIGHT OR ARROW_PARQUET OR ARROW_BUILD_TESTS) +if(ARROW_CUDA OR ARROW_FLIGHT OR ARROW_PARQUET OR ARROW_BUILD_TESTS + OR ARROW_BUILD_BENCHMARKS) set(ARROW_IPC ON) endif() diff --git a/dev/archery/archery/benchmark/runner.py b/dev/archery/archery/benchmark/runner.py index 509324c5248..377e77015ee 100644 --- a/dev/archery/archery/benchmark/runner.py +++ b/dev/archery/archery/benchmark/runner.py @@ -142,8 +142,18 @@ def __init__(self, build, **kwargs): def default_configuration(**kwargs): """ Returns the default benchmark configuration. """ return CppConfiguration( - build_type="release", with_tests=True, with_benchmarks=True, - with_compute=True, with_python=False, **kwargs) + build_type="release", with_tests=False, with_benchmarks=True, + with_compute=True, + with_dataset=True, + with_parquet=True, + with_python=False, + with_brotli=True, + with_bz2=True, + with_lz4=True, + with_snappy=True, + with_zlib=True, + with_zstd=True, + **kwargs) @property def suites_binaries(self): diff --git a/dev/archery/archery/lang/cpp.py b/dev/archery/archery/lang/cpp.py index 4af0f5434cc..89ecd3f6636 100644 --- a/dev/archery/archery/lang/cpp.py +++ b/dev/archery/archery/lang/cpp.py @@ -52,7 +52,7 @@ def __init__(self, with_compute=None, with_csv=None, with_cuda=None, with_dataset=None, with_filesystem=None, with_flight=None, with_gandiva=None, with_hdfs=None, with_hiveserver2=None, - with_ipc=None, with_json=None, with_jni=None, + with_ipc=True, with_json=None, with_jni=None, with_mimalloc=None, with_parquet=None, with_plasma=None, with_python=True, with_r=None, with_s3=None, From 0c145e2dc2f6ca6d4e6398de48f9786eab8d13b5 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 22 Jun 2020 11:36:44 -0500 Subject: [PATCH 3/5] Add linebreak --- dev/archery/archery/cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/dev/archery/archery/cli.py b/dev/archery/archery/cli.py index 95b57304747..23ad5ebbd5c 100644 --- a/dev/archery/archery/cli.py +++ b/dev/archery/archery/cli.py @@ -573,6 +573,7 @@ def benchmark_diff(ctx, src, preserve, output, cmake_extras, comparisons_json = _get_comparisons_as_json(runner_comp.comparisons) formatted = _format_comparisons_with_pandas(comparisons_json) output.write(formatted) + output.write('\n') def _get_comparisons_as_json(comparisons): From ba35ae5b2731aef893aded717cae2f96be3d46fd Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 22 Jun 2020 11:44:31 -0500 Subject: [PATCH 4/5] cmake-format --- cpp/CMakeLists.txt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 6000d3161a6..852bc0e850f 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -300,8 +300,11 @@ if(ARROW_BUILD_BENCHMARKS set(ARROW_JSON ON) endif() -if(ARROW_CUDA OR ARROW_FLIGHT OR ARROW_PARQUET OR ARROW_BUILD_TESTS - OR ARROW_BUILD_BENCHMARKS) +if(ARROW_CUDA + OR ARROW_FLIGHT + OR ARROW_PARQUET + OR ARROW_BUILD_TESTS + OR ARROW_BUILD_BENCHMARKS) set(ARROW_IPC ON) endif() From af0c8bb8ee31c8cc68c90dad7393a2d3388b11e4 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 22 Jun 2020 22:20:00 -0500 Subject: [PATCH 5/5] Show user counters in diff console output --- dev/archery/archery/benchmark/compare.py | 2 ++ dev/archery/archery/benchmark/google.py | 5 ++++- dev/archery/archery/cli.py | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/dev/archery/archery/benchmark/compare.py b/dev/archery/archery/benchmark/compare.py index b059712a254..474f6d40064 100644 --- a/dev/archery/archery/benchmark/compare.py +++ b/dev/archery/archery/benchmark/compare.py @@ -118,6 +118,7 @@ def formatted(self): "contender": fmt(self.contender.value), "unit": self.unit, "less_is_better": self.less_is_better, + "counters": str(self.baseline.counters) } def compare(self, comparator=None): @@ -129,6 +130,7 @@ def compare(self, comparator=None): "contender": self.contender.value, "unit": self.unit, "less_is_better": self.less_is_better, + "counters": self.baseline.counters } def __call__(self, **kwargs): diff --git a/dev/archery/archery/benchmark/google.py b/dev/archery/archery/benchmark/google.py index a3cc1a13340..3fa7a763ce1 100644 --- a/dev/archery/archery/benchmark/google.py +++ b/dev/archery/archery/benchmark/google.py @@ -89,7 +89,7 @@ class GoogleBenchmarkObservation: """ def __init__(self, name, real_time, cpu_time, time_unit, size=None, - bytes_per_second=None, items_per_second=None, **kwargs): + bytes_per_second=None, items_per_second=None, **counters): self._name = name self.real_time = real_time self.cpu_time = cpu_time @@ -97,6 +97,7 @@ def __init__(self, name, real_time, cpu_time, time_unit, size=None, self.size = size self.bytes_per_second = bytes_per_second self.items_per_second = items_per_second + self.counters = counters @property def is_agg(self): @@ -157,6 +158,8 @@ def __init__(self, name, runs): unit = self.runs[0].unit less_is_better = not unit.endswith("per_second") values = [b.value for b in self.runs] + # Slight kludge to extract the UserCounters for each benchmark + self.counters = self.runs[0].counters super().__init__(name, unit, less_is_better, values) def __repr__(self): diff --git a/dev/archery/archery/cli.py b/dev/archery/archery/cli.py index 23ad5ebbd5c..25c817a5f36 100644 --- a/dev/archery/archery/cli.py +++ b/dev/archery/archery/cli.py @@ -590,7 +590,7 @@ def _format_comparisons_with_pandas(comparisons_json): df = pd.read_json(StringIO(comparisons_json), lines=True) # parse change % so we can sort by it df['change %'] = df.pop('change').str[:-1].map(float) - df = df[['benchmark', 'baseline', 'contender', 'change %', 'regression']] + df = df[['benchmark', 'baseline', 'contender', 'change %', 'counters']] df = df.sort_values(by='change %', ascending=False) return df.to_string()