andygrove commented on code in PR #5374:
URL: https://github.com/apache/datafusion-comet/pull/5374#discussion_r3805357870


##########
benchmarks/micro/run.py:
##########
@@ -0,0 +1,1066 @@
+#!/usr/bin/env python3
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Runner for the Comet micro benchmark suites on a single machine (typically 
EC2).
+
+The script is self-contained and can be downloaded on its own, before the
+repository is cloned:
+
+    curl -sSLO 
https://raw.githubusercontent.com/apache/datafusion-comet/main/benchmarks/micro/run.py
+    python3 run.py all
+
+Subcommands:
+
+    setup     install prerequisites, clone/update the repository, build a 
release
+    run       run the benchmark suites, one JVM per suite
+    collect   copy the generated results into benchmarks/results/micro
+    publish   commit the collected results and optionally open a pull request
+    all       setup + run + collect
+
+Examples:
+
+    python3 run.py all --ref main
+    python3 run.py run --only Cast
+    python3 run.py run --suites my-suites.txt --heap 12g
+    python3 run.py collect --publish
+    python3 run.py publish --push --open-pr
+"""
+
+import argparse
+import json
+import os
+import platform
+import re
+import shutil
+import subprocess
+import sys
+import tempfile
+import time
+import urllib.error
+import urllib.request
+import zipfile
+from datetime import datetime, timezone
+from pathlib import Path
+
+BENCH_PACKAGE = "org.apache.spark.sql.benchmark"
+DEFAULT_REPO = "https://github.com/apache/datafusion-comet.git";
+DEFAULT_COMET_HOME = Path.home() / "datafusion-comet"
+DEFAULT_RUNS_ROOT = Path.home() / "comet-bench-runs"
+DEFAULT_HEAP = "8g"
+DEFAULT_JDK = "17"
+# Spark 4.x, the default build profile, does not compile on anything older
+MINIMUM_JDK = 17
+DEFAULT_TIMEOUT_MINUTES = 60
+# Only used when the distribution has no protoc package. prost-build needs a
+# protoc on the PATH to compile the Comet protobuf definitions.
+PROTOC_VERSION = "25.5"
+RESULTS_SUBDIR = Path("benchmarks") / "results" / "micro"
+IMDS_BASE = "http://169.254.169.254/latest";
+
+# Suites that generate their own data and write a results file. Keep this list
+# sorted; it is the default set that `run` executes.
+DEFAULT_SUITES = [
+    "CometAggregateExpressionBenchmark",
+    "CometArithmeticBenchmark",
+    "CometArrayExpressionBenchmark",
+    "CometBroadcastHashJoinBenchmark",
+    "CometBroadcastNestedLoopJoinBenchmark",
+    "CometCastBooleanBenchmark",
+    "CometCastNumericToNumericBenchmark",
+    "CometCastNumericToStringBenchmark",
+    "CometCastNumericToTemporalBenchmark",
+    "CometCastStringToNumericBenchmark",
+    "CometCastStringToTemporalBenchmark",
+    "CometCastTemporalToNumericBenchmark",
+    "CometCastTemporalToStringBenchmark",
+    "CometCastTemporalToTemporalBenchmark",
+    "CometColumnarToRowBenchmark",
+    "CometComparisonExpressionBenchmark",
+    "CometConditionalExpressionBenchmark",
+    "CometCsvExpressionBenchmark",
+    "CometDatetimeExpressionBenchmark",
+    "CometExecBenchmark",
+    "CometGetJsonObjectBenchmark",
+    "CometHashExpressionBenchmark",
+    "CometHashJoinBenchmark",
+    "CometIcebergReadBenchmark",
+    "CometJsonExpressionBenchmark",
+    "CometLengthOfJsonArrayBenchmark",
+    "CometOperatorSerdeBenchmark",
+    "CometPartitionColumnBenchmark",
+    "CometPredicateExpressionBenchmark",
+    "CometReadBenchmark",
+    "CometRegExpBenchmark",
+    "CometRegExpExtractBenchmark",
+    "CometShuffleBenchmark",
+    "CometSortMergeJoinBenchmark",
+    "CometStringExpressionBenchmark",
+]
+
+# Suites that legitimately take much longer than the default timeout allows.
+# Minutes, applied in place of --timeout for that suite only.
+SUITE_TIMEOUT_OVERRIDES = {
+    "CometShuffleBenchmark": 180,
+}
+
+# Suites deliberately left out of the default set, with the reason. They can
+# still be run by naming them in a --suites file.
+EXCLUDED_SUITES = {
+    "CometTPCHQueryBenchmark": "needs TPC-H data, pass --data-location",
+    "CometTPCDSQueryBenchmark": "needs TPC-DS data, pass --data-location",
+    "CometTPCDSMicroBenchmark": "needs TPC-DS data, pass --data-location",
+    "CometC2RIsolatedBench": "prints to stdout only, writes no results file",
+}
+
+
+# ---------------------------------------------------------------------------
+# small helpers
+# ---------------------------------------------------------------------------
+
+
+def log(message):
+    stamp = datetime.now().strftime("%H:%M:%S")
+    print(f"[{stamp}] {message}", flush=True)
+
+
+def fail(message):
+    print(f"error: {message}", file=sys.stderr)
+    sys.exit(1)
+
+
+def utc_now():
+    return datetime.now(timezone.utc)
+
+
+def format_duration(seconds):
+    minutes, seconds = divmod(int(seconds), 60)
+    hours, minutes = divmod(minutes, 60)
+    if hours:
+        return f"{hours}h{minutes:02d}m{seconds:02d}s"
+    if minutes:
+        return f"{minutes}m{seconds:02d}s"
+    return f"{seconds}s"
+
+
+def run_command(cmd, cwd=None, env=None, log_path=None, timeout=None, 
check=True, dry_run=False):
+    """Run a command, optionally sending its output to a log file."""
+    printable = " ".join(str(part) for part in cmd)
+    if dry_run:
+        log(f"[dry-run] {printable}")
+        return 0
+    log(printable)
+    if log_path is None:
+        completed = subprocess.run(cmd, cwd=cwd, env=env, timeout=timeout)
+    else:
+        with open(log_path, "w") as handle:
+            handle.write(f"$ {printable}\n\n")
+            handle.flush()
+            completed = subprocess.run(
+                cmd, cwd=cwd, env=env, stdout=handle, 
stderr=subprocess.STDOUT, timeout=timeout
+            )
+    if check and completed.returncode != 0:
+        fail(f"command failed with exit code {completed.returncode}: 
{printable}")
+    return completed.returncode
+
+
+def capture(cmd, cwd=None, env=None):
+    """Run a command and return stripped stdout, or None if it fails."""
+    try:
+        completed = subprocess.run(
+            cmd, cwd=cwd, env=env, capture_output=True, text=True, timeout=300
+        )
+    except (OSError, subprocess.SubprocessError):
+        return None
+    if completed.returncode != 0:
+        return None
+    return completed.stdout.strip()
+
+
+def sudo_prefix():
+    if os.geteuid() == 0:
+        return []
+    if shutil.which("sudo") is None:
+        fail("this step needs root privileges but sudo was not found")
+    return ["sudo"]
+
+
+def tail_file(path, lines=20):
+    try:
+        content = Path(path).read_text(errors="replace").splitlines()
+    except OSError:
+        return ""
+    return "\n".join(content[-lines:])
+
+
+# The last lines of a failed run are Maven's epilogue, which says nothing about
+# the cause. Look for the exception that started it instead.
+EXCEPTION_PATTERN = re.compile(r"^\s*[\w.$]+(Exception|Error)(:|\b)")
+MAVEN_FAILURE_PATTERN = re.compile(r"^\[ERROR\] Failed to execute goal")
+
+
+def failure_excerpt(path, context=12, fallback_lines=20):
+    """The first exception in a log, falling back to Maven's message or the 
tail."""
+    try:
+        content = Path(path).read_text(errors="replace").splitlines()
+    except OSError:
+        return ""
+    for pattern in (EXCEPTION_PATTERN, MAVEN_FAILURE_PATTERN):
+        for index, line in enumerate(content):
+            if pattern.search(line):
+                return "\n".join(content[index : index + context])
+    return "\n".join(content[-fallback_lines:])
+
+
+# ---------------------------------------------------------------------------
+# environment
+# ---------------------------------------------------------------------------
+
+
+def find_checkout_from_script():
+    """Return the Comet checkout this script lives in, if any."""
+    for parent in Path(__file__).resolve().parents:
+        if (parent / "Makefile").is_file() and (parent / "native" / 
"Cargo.toml").is_file():
+            return parent
+    return None
+
+
+def resolve_comet_home(args):
+    if args.comet_home:
+        return Path(args.comet_home).expanduser().resolve()
+    if os.environ.get("COMET_HOME"):
+        return Path(os.environ["COMET_HOME"]).expanduser().resolve()
+    checkout = find_checkout_from_script()
+    if checkout is not None:
+        return checkout
+    return DEFAULT_COMET_HOME
+
+
+def require_comet_home(args):
+    comet_home = resolve_comet_home(args)
+    if not (comet_home / "Makefile").is_file():
+        fail(f"{comet_home} is not a Comet checkout, run `setup` first or pass 
--comet-home")
+    return comet_home
+
+
+def detect_java_home():
+    if os.environ.get("JAVA_HOME"):
+        return os.environ["JAVA_HOME"]
+    candidates = []
+    for pattern in ("java-*-amazon-corretto*", "java-*-openjdk*", "jdk-*"):
+        candidates.extend(Path("/usr/lib/jvm").glob(pattern))
+
+    def version_of(path):
+        match = re.search(r"(\d+)", path.name)
+        return int(match.group(1)) if match else 0
+
+    # JDK 17 first, since the pom only auto-activates a profile for 11 or 17,
+    # then newest, so that an older JDK left on the machine is not picked up
+    ordered = sorted(candidates, key=lambda path: (version_of(path) == 17, 
version_of(path)), reverse=True)
+    for candidate in ordered:
+        if (candidate / "bin" / "javac").is_file():
+            return str(candidate)
+    java = shutil.which("java")
+    if java is not None:
+        resolved = Path(java).resolve()
+        return str(resolved.parent.parent)
+    return None
+
+
+def base_env(comet_home):
+    """Environment shared by the build and the benchmark runs."""
+    env = dict(os.environ)
+    cargo_bin = Path.home() / ".cargo" / "bin"
+    if cargo_bin.is_dir() and str(cargo_bin) not in env.get("PATH", ""):
+        env["PATH"] = f"{cargo_bin}{os.pathsep}{env.get('PATH', '')}"
+    java_home = detect_java_home()
+    if java_home:
+        env["JAVA_HOME"] = java_home
+        java_bin = Path(java_home) / "bin"
+        if str(java_bin) not in env.get("PATH", ""):
+            env["PATH"] = f"{java_bin}{os.pathsep}{env.get('PATH', '')}"
+    env["COMET_CONF_DIR"] = str(comet_home / "conf")
+    return env
+
+
+def profile_args(args, jdk_major=None):

Review Comment:
   Both fixed. `profile_args()` and `maven_extra_java_args()` are gone entirely 
(the pom fix and the Makefile target replaced them), and `suite_command(suite, 
mvn_args, profile)` now takes what it uses. `require_supported_jdk()` no longer 
returns the version, since nothing needed it once the `-Pjdk17` handling went 
away.



##########
benchmarks/micro/run.py:
##########


Review Comment:
   Added `dev/ci/check-benchmark-runner.py`, run from the `preflight` lint job 
next to `check-suites.py`. It imports the runner (which is the syntax check), 
asserts that discovery finds a plausible set and that `EXCLUDED_SUITES` and 
`SUITE_TIMEOUT_OVERRIDES` still name real benchmarks, checks the 
`--only`/`--skip` filtering, and feeds a synthetic failing log through 
`failure_excerpt` to cover the exception-first and Maven-fallback paths. Plain 
asserts and stdlib only, matching `check-suites.py`, since the lint job has no 
pytest.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to