diff --git a/.github/workflows/build-swebench-images.yml b/.github/workflows/build-swebench-images.yml index 6053db373..c5d0820db 100644 --- a/.github/workflows/build-swebench-images.yml +++ b/.github/workflows/build-swebench-images.yml @@ -22,7 +22,12 @@ on: max-workers: description: 'Number of concurrent builds' required: false - default: '12' + default: '32' + type: string + build-batch-size: + description: 'Number of images to submit per batch' + required: false + default: '50' type: string max-retries: description: 'Retries per image build' @@ -54,12 +59,12 @@ on: env: DATASET: princeton-nlp/SWE-bench_Verified SPLIT: test - MAX_WORKERS: '12' + MAX_WORKERS: '32' MAX_RETRIES: '5' N_LIMIT: '500' INSTANCE_IDS: '' SELECT_FILE: '' - BUILD_BATCH_SIZE: '15' + BUILD_BATCH_SIZE: '50' BUILDKIT_PRUNE_KEEP_GB: '60' BUILDKIT_PRUNE_THRESHOLD_PCT: '60' @@ -78,7 +83,6 @@ jobs: runs-on: labels: blacksmith-32vcpu-ubuntu-2204 - # Allow pushing to GHCR and commenting on issues permissions: contents: read @@ -86,6 +90,11 @@ jobs: issues: write steps: + - name: Record build start time + run: | + echo "BUILD_START=$(date +%s)" >> "$GITHUB_ENV" + echo "Build started at $(date -u)" + - name: Determine checkout ref id: checkout-ref run: | @@ -114,6 +123,7 @@ jobs: if [ -n "${{ inputs.dataset }}" ]; then echo "DATASET=${{ inputs.dataset }}" >> "$GITHUB_ENV"; fi if [ -n "${{ inputs.split }}" ]; then echo "SPLIT=${{ inputs.split }}" >> "$GITHUB_ENV"; fi if [ -n "${{ inputs.max-workers }}" ]; then echo "MAX_WORKERS=${{ inputs.max-workers }}" >> "$GITHUB_ENV"; fi + if [ -n "${{ inputs.build-batch-size }}" ]; then echo "BUILD_BATCH_SIZE=${{ inputs.build-batch-size }}" >> "$GITHUB_ENV"; fi if [ -n "${{ inputs.max-retries }}" ]; then echo "MAX_RETRIES=${{ inputs.max-retries }}" >> "$GITHUB_ENV"; fi # Empty string means "no limit" if [ -n "${{ inputs.n-limit }}" ]; then echo "N_LIMIT=${{ inputs.n-limit }}" >> "$GITHUB_ENV"; else echo "N_LIMIT=" >> "$GITHUB_ENV"; fi @@ -232,6 +242,7 @@ jobs: --image ghcr.io/openhands/eval-agent-server \ --push \ --max-workers '${MAX_WORKERS}' \ + --build-batch-size '${BUILD_BATCH_SIZE}' \ --max-retries '${MAX_RETRIES}'" # Only include --n-limit if provided (non-empty) @@ -249,6 +260,21 @@ jobs: BUILDKIT_PROGRESS: plain BUILDKIT_RESET_ON_FAILURE: 1 + - name: Post-build disk and timing report + if: always() + run: | + set -euo pipefail + BUILD_END=$(date +%s) + ELAPSED=$(( BUILD_END - ${BUILD_START:-$BUILD_END} )) + echo "## Build Timing" >> "$GITHUB_STEP_SUMMARY" + echo "**Elapsed:** $((ELAPSED / 60))m $((ELAPSED % 60))s" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + + echo "## Disk Usage After Build" >> "$GITHUB_STEP_SUMMARY" + df -h / /var/lib/buildkit 2>/dev/null | tee -a "$GITHUB_STEP_SUMMARY" || true + echo "" >> "$GITHUB_STEP_SUMMARY" + docker buildx du --verbose 2>/dev/null | head -40 | tee -a "$GITHUB_STEP_SUMMARY" || true + - name: Archive build logs if: always() run: | diff --git a/.github/workflows/build-swtbench-images.yml b/.github/workflows/build-swtbench-images.yml index fe9d07be6..d92b16e0b 100644 --- a/.github/workflows/build-swtbench-images.yml +++ b/.github/workflows/build-swtbench-images.yml @@ -22,7 +22,12 @@ on: max-workers: description: 'Maximum number of parallel workers' required: false - default: '4' + default: '16' + type: string + agent-build-batch-size: + description: 'Number of agent-server images to submit per batch' + required: false + default: '50' type: string n-limit: description: 'Limit number of images to build (0 for all)' @@ -72,7 +77,6 @@ jobs: runs-on: labels: blacksmith-32vcpu-ubuntu-2204 - permissions: contents: read packages: write @@ -82,12 +86,18 @@ jobs: env: DATASET: eth-sri/SWT-bench_Verified_bm25_27k_zsp SPLIT: test - MAX_WORKERS: '4' + MAX_WORKERS: '16' + BUILD_BATCH_SIZE: '50' N_LIMIT: '0' INSTANCE_IDS: '' SELECT_FILE: '' steps: + - name: Record build start time + run: | + echo "BUILD_START=$(date +%s)" >> "$GITHUB_ENV" + echo "Build started at $(date -u)" + - name: Determine checkout ref id: checkout-ref run: | @@ -134,6 +144,35 @@ jobs: run: | make build + - name: "Preflight: prune cache and verify BuildKit disk" + run: | + set -euo pipefail + KEEP_GB=60 + echo "Pruning BuildKit cache (target max-storage ${KEEP_GB} GiB, no filters)..." + if ! docker buildx prune --all --force --max-storage ${KEEP_GB}g; then + docker buildx prune --all --force --keep-storage ${KEEP_GB}g || true + fi + + if df -B1 /var/lib/buildkit > /tmp/buildkit_df 2>/dev/null; then + LINE=$(tail -n1 /tmp/buildkit_df) + TOTAL=$(echo "$LINE" | awk '{print $2}') + USED=$(echo "$LINE" | awk '{print $3}') + FREE=$(echo "$LINE" | awk '{print $4}') + if [ -n "$TOTAL" ] && [ -n "$FREE" ]; then + PCT=$(( 100 * USED / TOTAL )) + echo "BuildKit disk: used ${USED} / ${TOTAL} bytes (${PCT}%); free ${FREE} bytes" + MIN=$((75 * 1024 * 1024 * 1024)) + if [ "$FREE" -lt "$MIN" ]; then + echo "::error::Not enough free space on /var/lib/buildkit (${FREE} bytes free, need >= ${MIN})" + exit 1 + fi + else + echo "Warning: unable to parse df output for /var/lib/buildkit" + fi + else + echo "Warning: /var/lib/buildkit not found; skipping disk check" + fi + - name: Build and push SWT-Bench images run: | set -euo pipefail @@ -141,7 +180,8 @@ jobs: # Get inputs with defaults DATASET="${{ inputs.dataset || 'eth-sri/SWT-bench_Verified_bm25_27k_zsp' }}" SPLIT="${{ inputs.split || 'test' }}" - MAX_WORKERS="${{ inputs.max-workers || '4' }}" + MAX_WORKERS="${{ inputs.max-workers || '16' }}" + BUILD_BATCH_SIZE="${{ inputs.agent-build-batch-size || '50' }}" N_LIMIT="${{ inputs.n-limit || '0' }}" INSTANCE_IDS="${{ inputs.instance-ids }}" @@ -166,6 +206,7 @@ jobs: --image ghcr.io/openhands/eval-agent-server \ --target ${TARGET} \ --max-workers ${MAX_WORKERS} \ + --build-batch-size ${BUILD_BATCH_SIZE} \ --push" # Add n-limit if specified @@ -181,6 +222,8 @@ jobs: env: DOCKER_BUILDKIT: 1 BUILDKIT_PROGRESS: plain + BUILDKIT_RESET_ON_FAILURE: 1 + OPENHANDS_BUILDKIT_CACHE_MODE: off - name: Build prebaked eval env images if: ${{ inputs.build-eval-env == 'true' }} @@ -240,6 +283,21 @@ jobs: docker ps -a || true docker system df || true + - name: Post-build disk and timing report + if: always() + run: | + set -euo pipefail + BUILD_END=$(date +%s) + ELAPSED=$(( BUILD_END - ${BUILD_START:-$BUILD_END} )) + echo "## Build Timing" >> "$GITHUB_STEP_SUMMARY" + echo "**Elapsed:** $((ELAPSED / 60))m $((ELAPSED % 60))s" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + + echo "## Disk Usage After Build" >> "$GITHUB_STEP_SUMMARY" + df -h / /var/lib/buildkit 2>/dev/null | tee -a "$GITHUB_STEP_SUMMARY" || true + echo "" >> "$GITHUB_STEP_SUMMARY" + docker buildx du --verbose 2>/dev/null | head -40 | tee -a "$GITHUB_STEP_SUMMARY" || true + - name: Archive build logs if: always() run: | diff --git a/benchmarks/commit0/build_images.py b/benchmarks/commit0/build_images.py index 3f24567ec..fce111401 100644 --- a/benchmarks/commit0/build_images.py +++ b/benchmarks/commit0/build_images.py @@ -124,6 +124,7 @@ def main(argv: list[str]) -> int: image=args.image, push=args.push, max_workers=args.max_workers, + build_batch_size=args.build_batch_size, dry_run=args.dry_run, max_retries=args.max_retries, base_image_to_custom_tag_fn=extract_custom_tag, diff --git a/benchmarks/gaia/build_images.py b/benchmarks/gaia/build_images.py index 45aee7550..73ca42ad1 100644 --- a/benchmarks/gaia/build_images.py +++ b/benchmarks/gaia/build_images.py @@ -94,6 +94,7 @@ def tag_fn(_base: str) -> str: image=args.image, push=args.push, max_workers=1, # Only building one image + build_batch_size=args.build_batch_size, dry_run=args.dry_run, max_retries=args.max_retries, base_image_to_custom_tag_fn=tag_fn, diff --git a/benchmarks/multiswebench/build_images.py b/benchmarks/multiswebench/build_images.py index 6c3ef3b9f..c967e8506 100644 --- a/benchmarks/multiswebench/build_images.py +++ b/benchmarks/multiswebench/build_images.py @@ -118,6 +118,7 @@ def main(): ), base_image_to_custom_tag_fn=extract_custom_tag, max_workers=args.num_workers, + build_batch_size=args.build_batch_size, dry_run=False, ) diff --git a/benchmarks/swebench/build_images.py b/benchmarks/swebench/build_images.py index 5ace5419b..1e7cb8ff8 100644 --- a/benchmarks/swebench/build_images.py +++ b/benchmarks/swebench/build_images.py @@ -9,6 +9,9 @@ --image ghcr.io/openhands/eval-agent-server --target source-minimal """ +# Use stdlib logging instead of openhands.sdk.get_logger to avoid initializing +# Rich console state before ProcessPoolExecutor forks (causes deadlocks). +import logging import sys from pathlib import Path @@ -23,10 +26,9 @@ ) from benchmarks.utils.dataset import get_dataset from benchmarks.utils.image_utils import remote_image_exists -from openhands.sdk import get_logger -logger = get_logger(__name__) +logger = logging.getLogger(__name__) WRAPPER_DOCKERFILE = Path(__file__).with_name("Dockerfile.swebench-deps") @@ -177,6 +179,7 @@ def main(argv: list[str]) -> int: image=args.image, push=args.push, max_workers=args.max_workers, + build_batch_size=args.build_batch_size, dry_run=args.dry_run, max_retries=args.max_retries, base_image_to_custom_tag_fn=extract_custom_tag, diff --git a/benchmarks/swebenchmultimodal/build_images.py b/benchmarks/swebenchmultimodal/build_images.py index 987cf7bda..32628bd78 100644 --- a/benchmarks/swebenchmultimodal/build_images.py +++ b/benchmarks/swebenchmultimodal/build_images.py @@ -83,6 +83,7 @@ def main(argv: list[str]) -> int: image=args.image, push=args.push, max_workers=args.max_workers, + build_batch_size=args.build_batch_size, dry_run=args.dry_run, max_retries=args.max_retries, base_image_to_custom_tag_fn=extract_custom_tag, diff --git a/benchmarks/swegym/build_images.py b/benchmarks/swegym/build_images.py index 6c116a020..009bb0dab 100644 --- a/benchmarks/swegym/build_images.py +++ b/benchmarks/swegym/build_images.py @@ -87,6 +87,7 @@ def main(argv: list[str]) -> int: image=args.image, push=args.push, max_workers=args.max_workers, + build_batch_size=args.build_batch_size, dry_run=args.dry_run, max_retries=args.max_retries, base_image_to_custom_tag_fn=extract_custom_tag, diff --git a/benchmarks/swesmith/build_images.py b/benchmarks/swesmith/build_images.py index 51fda90ae..7a1ee7469 100644 --- a/benchmarks/swesmith/build_images.py +++ b/benchmarks/swesmith/build_images.py @@ -84,6 +84,7 @@ def main(argv: list[str]) -> int: image=args.image, push=args.push, max_workers=args.max_workers, + build_batch_size=args.build_batch_size, dry_run=args.dry_run, max_retries=args.max_retries, base_image_to_custom_tag_fn=extract_custom_tag, diff --git a/benchmarks/swtbench/build_eval_env_images.py b/benchmarks/swtbench/build_eval_env_images.py index 2f0ea9862..c64df2883 100644 --- a/benchmarks/swtbench/build_eval_env_images.py +++ b/benchmarks/swtbench/build_eval_env_images.py @@ -1,7 +1,10 @@ from __future__ import annotations +# Use stdlib logging instead of openhands.sdk.get_logger to avoid initializing +# Rich console state before ProcessPoolExecutor forks (causes deadlocks). import argparse import json +import logging import os import sys from pathlib import Path @@ -13,10 +16,9 @@ from benchmarks.swtbench.image_utils import ensure_swt_bench_repo from benchmarks.utils.dataset import get_dataset from benchmarks.utils.image_utils import remote_image_exists -from openhands.sdk import get_logger -logger = get_logger(__name__) +logger = logging.getLogger(__name__) def select_instance_ids( diff --git a/benchmarks/swtbench/build_images.py b/benchmarks/swtbench/build_images.py index 3fcd2d8de..621a3385b 100644 --- a/benchmarks/swtbench/build_images.py +++ b/benchmarks/swtbench/build_images.py @@ -44,6 +44,7 @@ def main(argv: list[str]) -> int: image=args.image, push=args.push, max_workers=args.max_workers, + build_batch_size=args.build_batch_size, dry_run=args.dry_run, max_retries=args.max_retries, base_image_to_custom_tag_fn=extract_custom_tag, diff --git a/benchmarks/swtbench/image_utils.py b/benchmarks/swtbench/image_utils.py index 0aa3ec6f5..387a94085 100644 --- a/benchmarks/swtbench/image_utils.py +++ b/benchmarks/swtbench/image_utils.py @@ -1,5 +1,7 @@ from __future__ import annotations +# Use stdlib logging instead of openhands.sdk.get_logger to avoid initializing +# Rich console state before ProcessPoolExecutor forks (causes deadlocks). import json import logging import os @@ -9,10 +11,9 @@ from typing import Iterable from benchmarks.swtbench.config import EVAL_DEFAULTS -from openhands.sdk import get_logger -logger = get_logger(__name__) +logger = logging.getLogger(__name__) def ensure_swt_bench_repo(cache_dir: Path | None = None) -> Path: diff --git a/benchmarks/utils/build_utils.py b/benchmarks/utils/build_utils.py index b4e56b9ad..d21c6ed89 100644 --- a/benchmarks/utils/build_utils.py +++ b/benchmarks/utils/build_utils.py @@ -3,12 +3,17 @@ Shared utilities for batch building agent-server images. """ +# Use stdlib logging instead of openhands.sdk.get_logger to avoid initializing +# Rich console state before ProcessPoolExecutor forks (causes deadlocks). import argparse import contextlib import io +import logging import os +import shutil import subprocess import sys +import tempfile import time import tomllib from concurrent.futures import ProcessPoolExecutor, as_completed @@ -29,10 +34,9 @@ ) from benchmarks.utils.constants import EVAL_AGENT_SERVER_IMAGE from benchmarks.utils.image_utils import local_image_exists, remote_image_exists -from openhands.sdk import get_logger -logger = get_logger(__name__) +logger = logging.getLogger(__name__) class BuildOutput(BaseModel): @@ -198,6 +202,57 @@ def _get_sdk_submodule_info() -> tuple[str, str, str]: return git_ref, git_sha, sdk_version +def _sdk_root() -> Path: + benchmarks_root = Path(__file__).resolve().parent.parent.parent + return benchmarks_root / "vendor" / "software-agent-sdk" + + +def _pre_build_sdist() -> Path: + """ + Build the SDK sdist once and reuse it across all image builds in a batch. + + The caller must clean up the parent directory of the returned tarball. + """ + sdk_path = _sdk_root() + sdist_dir = Path(tempfile.mkdtemp(prefix="shared-sdist-")).resolve() + + logger.info("Pre-building SDK sdist from %s", sdk_path) + start = time.time() + proc = subprocess.run( + ["uv", "build", "--sdist", "--out-dir", str(sdist_dir)], + cwd=str(sdk_path), + capture_output=True, + text=True, + ) + if proc.returncode != 0: + shutil.rmtree(sdist_dir, ignore_errors=True) + raise RuntimeError(f"Failed to build SDK sdist: {proc.stderr}") + + sdists = sorted(sdist_dir.glob("*.tar.gz")) + if len(sdists) != 1: + shutil.rmtree(sdist_dir, ignore_errors=True) + raise RuntimeError(f"Expected 1 SDK sdist, got {len(sdists)}") + + logger.info("Pre-built SDK sdist in %.1fs: %s", time.time() - start, sdists[0]) + return sdists[0] + + +@contextlib.contextmanager +def _prepare_cached_sdist(): + cached_sdist_path: Path | None = None + try: + try: + cached_sdist_path = _pre_build_sdist() + except Exception as e: + logger.warning( + "Failed to pre-build SDK sdist; each image will build its own: %s", e + ) + yield cached_sdist_path + finally: + if cached_sdist_path: + shutil.rmtree(cached_sdist_path.parent, ignore_errors=True) + + @contextlib.contextmanager def capture_output(base_name: str, out_dir: Path): """ @@ -269,6 +324,15 @@ def get_build_parser() -> argparse.ArgumentParser: parser.add_argument( "--max-workers", type=int, default=1, help="Concurrent builds (be cautious)" ) + parser.add_argument( + "--build-batch-size", + type=int, + default=None, + help=( + "Number of images to submit per batch. Defaults to BUILD_BATCH_SIZE " + "when unset." + ), + ) parser.add_argument( "--dry-run", action="store_true", help="List base images only, don’t build" ) @@ -281,10 +345,14 @@ def build_image( custom_tag: str, target: TargetType = "source-minimal", push: bool = False, + cached_sdist: Path | None = None, ) -> BuildOutput: # Importing here because openhands.agent_server.docker.build runs git checks # which fails when installed as a package outside the git repo - from openhands.agent_server.docker.build import BuildOptions, build + from openhands.agent_server.docker import build as sdk_build_module + + BuildOptions = sdk_build_module.BuildOptions + build = sdk_build_module.build # Get SDK info from submodule to ensure tags use the correct SDK SHA git_ref, git_sha, sdk_version = _get_sdk_submodule_info() @@ -301,13 +369,16 @@ def build_image( git_ref=git_ref, git_sha=git_sha, sdk_version=sdk_version, + prebuilt_sdist=cached_sdist, ) for t in opts.all_tags: # Check if image exists or not if remote_image_exists(t): logger.info("Image %s already exists. Skipping build.", t) return BuildOutput(base_image=base_image, tags=[t], error=None) + tags = build(opts) + return BuildOutput(base_image=base_image, tags=tags, error=None) @@ -358,6 +429,7 @@ def _build_with_logging( push: bool = False, max_retries: int = 3, post_build_fn: Callable[[BuildOutput, bool], BuildOutput] | None = None, + cached_sdist: Path | None = None, ) -> BuildOutput: """ Module-level function for building a single image with output capture. @@ -379,7 +451,14 @@ def _build_with_logging( ) time.sleep(2 + attempt * 2) try: - result = build_image(base_image, target_image, custom_tag, target, push) + result = build_image( + base_image, + target_image, + custom_tag, + target, + push, + cached_sdist, + ) except Exception as e: result = BuildOutput( base_image=base_image, @@ -417,6 +496,51 @@ def _build_with_logging( raise RuntimeError("Unreachable code reached in _build_with_logging") +def _iter_batch_results( + batch: list[str], + log_dir: Path, + image: str, + target: TargetType, + push: bool, + base_image_to_custom_tag_fn: Callable[[str], str] | None, + max_workers: int, + max_retries: int, + post_build_fn: Callable[[BuildOutput, bool], BuildOutput] | None, + cached_sdist: Path | None, +): + with ProcessPoolExecutor(max_workers=max_workers) as ex: + futures = {} + for base in batch: + resolved_tag = ( + base_image_to_custom_tag_fn(base) if base_image_to_custom_tag_fn else "" + ) + fut = ex.submit( + _build_with_logging, + log_dir=log_dir, + base_image=base, + target_image=image, + custom_tag=resolved_tag, + target=target, + push=push, + max_retries=max_retries, + post_build_fn=post_build_fn, + cached_sdist=cached_sdist, + ) + futures[fut] = base + + for fut in as_completed(futures): + base = futures[fut] + try: + yield fut.result() + except Exception as e: + logger.error("Build failed for %s: %r", base, e) + yield BuildOutput( + base_image=base, + tags=[], + error=repr(e), + ) + + def _update_pbar( pbar: tqdm, successes: int, @@ -453,6 +577,7 @@ def build_all_images( push: bool = False, base_image_to_custom_tag_fn: Callable[[str], str] | None = None, max_workers: int = 1, + build_batch_size: int | None = None, dry_run: bool = False, max_retries: int = 3, post_build_fn: Callable[[BuildOutput, bool], BuildOutput] | None = None, @@ -470,6 +595,8 @@ def build_all_images( base_image_to_custom_tag_fn: Function to extract a custom tag from a base image. Evaluated before scheduling builds so it can safely be a closure. max_workers: Number of concurrent builds. + build_batch_size: Number of images to submit per batch. If None, use the + BUILD_BATCH_SIZE environment variable. dry_run: If True, only list base images without building. max_retries: Number of times to retry each failed build (default: 3). post_build_fn: Optional callback called after each successful build. @@ -494,7 +621,11 @@ def build_all_images( # Batch/prune settings (tunable via env to control disk usage on sticky runners) # Default to smaller batches and more aggressive pruning on shared runners. - batch_size = int(os.getenv("BUILD_BATCH_SIZE", "15")) + batch_size = ( + build_batch_size + if build_batch_size is not None + else int(os.getenv("BUILD_BATCH_SIZE", "15")) + ) prune_keep_storage_gb = int(os.getenv("BUILDKIT_PRUNE_KEEP_GB", "60")) prune_threshold_pct = float(os.getenv("BUILDKIT_PRUNE_THRESHOLD_PCT", "60")) # Prune aggressively by default; filters like "unused-for=12h" prevented GC from @@ -512,6 +643,7 @@ def _chunks(seq: list[str], size: int): total_batches = len(batches) with ( + _prepare_cached_sdist() as cached_sdist, manifest_file.open("w") as writer, tqdm( total=len(base_images), desc="Building agent-server images", leave=True @@ -524,72 +656,56 @@ def _chunks(seq: list[str], size: int): continue logger.info( - "Starting batch %d/%d (%d images)", batch_idx, total_batches, len(batch) + "Starting batch %d/%d (%d images)", + batch_idx, + total_batches, + len(batch), + ) + in_progress: set[str] = set(batch) + _update_pbar( + pbar, + successes, + failures, + len(in_progress), + next(iter(in_progress), None), + f"Batch {batch_idx}/{total_batches} running", ) - in_progress: set[str] = set() - - with ProcessPoolExecutor(max_workers=max_workers) as ex: - futures = {} - for base in batch: - in_progress.add(base) - resolved_tag = ( - base_image_to_custom_tag_fn(base) - if base_image_to_custom_tag_fn - else "" - ) - fut = ex.submit( - _build_with_logging, - log_dir=build_log_dir, - base_image=base, - target_image=image, - custom_tag=resolved_tag, - target=target, - push=push, - max_retries=max_retries, - post_build_fn=post_build_fn, - ) - futures[fut] = base + for result in _iter_batch_results( + batch=batch, + log_dir=build_log_dir, + image=image, + target=target, + push=push, + base_image_to_custom_tag_fn=base_image_to_custom_tag_fn, + max_workers=max_workers, + max_retries=max_retries, + post_build_fn=post_build_fn, + cached_sdist=cached_sdist, + ): + status = None + writer.write(result.model_dump_json() + "\n") + writer.flush() + + with mu: + if result.error or not result.tags: + failures += 1 + status = "❌ Failed" + else: + successes += 1 + status = "✅ Done" + + in_progress.discard(result.base_image) + pbar.update(1) _update_pbar( pbar, successes, failures, len(in_progress), next(iter(in_progress), None), - f"Batch {batch_idx}/{total_batches} running", + status, ) - for fut in as_completed(futures): - base = futures[fut] - status = None - try: - result: BuildOutput = fut.result() - except Exception as e: - logger.error("Build failed for %s: %r", base, e) - result = BuildOutput(base_image=base, tags=[], error=repr(e)) - - writer.write(result.model_dump_json() + "\n") - writer.flush() - - with mu: - if result.error or not result.tags: - failures += 1 - status = "❌ Failed" - else: - successes += 1 - status = "✅ Done" - - in_progress.discard(base) - pbar.update(1) - _update_pbar( - pbar, - successes, - failures, - len(in_progress), - next(iter(in_progress), None), - status, - ) - used, total = buildkit_disk_usage() if total > 0: logger.info( @@ -622,10 +738,10 @@ def _chunks(seq: list[str], size: int): total_batches, prune_threshold_pct, ) - logger.info( - "Done. Built=%d Failed=%d Manifest=%s", - successes, - failures, - str(manifest_file), - ) - return 1 if failures else 0 + logger.info( + "Done. Built=%d Failed=%d Manifest=%s", + successes, + failures, + str(manifest_file), + ) + return 1 if failures else 0 diff --git a/benchmarks/utils/buildx_utils.py b/benchmarks/utils/buildx_utils.py index 648bb7d25..0a2aa2037 100644 --- a/benchmarks/utils/buildx_utils.py +++ b/benchmarks/utils/buildx_utils.py @@ -3,7 +3,10 @@ Buildx/BuildKit utilities for image build resets and pruning. """ +# Use stdlib logging instead of openhands.sdk.get_logger to avoid initializing +# Rich console state before ProcessPoolExecutor forks (causes deadlocks). import json +import logging import os import re import shutil @@ -11,10 +14,8 @@ import time from pathlib import Path -from openhands.sdk import get_logger - -logger = get_logger(__name__) +logger = logging.getLogger(__name__) def _read_reset_state(path: Path) -> dict[str, float]: diff --git a/benchmarks/utils/image_utils.py b/benchmarks/utils/image_utils.py index 467074cb9..45c6f770f 100644 --- a/benchmarks/utils/image_utils.py +++ b/benchmarks/utils/image_utils.py @@ -1,7 +1,10 @@ #!/usr/bin/env python3 from __future__ import annotations +# Use stdlib logging instead of openhands.sdk.get_logger to avoid initializing +# Rich console state before ProcessPoolExecutor forks (causes deadlocks). import base64 +import logging import os import subprocess import sys @@ -14,10 +17,8 @@ import requests -from openhands.sdk import get_logger - -logger = get_logger(__name__) +logger = logging.getLogger(__name__) ACCEPT = ",".join( diff --git a/tests/test_image_utils.py b/tests/test_image_utils.py index c46830cb6..82ade18a6 100644 --- a/tests/test_image_utils.py +++ b/tests/test_image_utils.py @@ -4,8 +4,10 @@ which centralize Docker image detection and build logic across all benchmarks. """ +import contextlib import os import subprocess +from pathlib import Path from unittest.mock import MagicMock, patch import pytest @@ -255,3 +257,97 @@ def test_passes_target_to_build_image(self, mock_build, _mock_exists): _, kwargs = mock_build.call_args assert kwargs["target"] == "binary" assert kwargs["push"] is False + + +class TestCachedSdistReuse: + def test_build_image_passes_cached_sdist_to_sdk_build_module( + self, + tmp_path: Path, + ): + from benchmarks.utils.build_utils import build_image + from openhands.agent_server.docker import build as sdk_build_module + + cached_sdist = tmp_path / "openhands-sdk.tar.gz" + cached_sdist.write_text("cached", encoding="utf-8") + captured = {} + + def fake_build(opts): + captured["prebuilt_sdist"] = opts.prebuilt_sdist + return ["integration:test"] + + with ( + patch( + "benchmarks.utils.build_utils.remote_image_exists", return_value=False + ), + patch( + "benchmarks.utils.build_utils._get_sdk_submodule_info", + return_value=("main", "abcdef0", "1.0.0"), + ), + patch.object(sdk_build_module, "build", side_effect=fake_build), + ): + result = build_image( + base_image="base:latest", + target_image="ghcr.io/openhands/eval-agent-server", + custom_tag="mytag", + cached_sdist=cached_sdist, + ) + + assert result.error is None + assert result.tags == ["integration:test"] + assert captured["prebuilt_sdist"] == cached_sdist + + +class TestBuildBatchSizeConfig: + def test_build_parser_accepts_build_batch_size(self): + from benchmarks.utils.build_utils import get_build_parser + + args = get_build_parser().parse_args(["--build-batch-size", "50"]) + + assert args.build_batch_size == 50 + + @patch.dict(os.environ, {"BUILD_BATCH_SIZE": "99"}) + def test_build_all_images_prefers_explicit_batch_size_over_env( + self, + tmp_path: Path, + ): + from benchmarks.utils import build_utils + + seen_batches: list[list[str]] = [] + + @contextlib.contextmanager + def fake_prepare_cached_sdist(): + yield None + + def fake_iter_batch_results(**kwargs): + batch = kwargs["batch"] + seen_batches.append(list(batch)) + for base in batch: + yield BuildOutput( + base_image=base, + tags=[f"tag:{base}"], + error=None, + ) + + with ( + patch.object( + build_utils, + "_prepare_cached_sdist", + side_effect=fake_prepare_cached_sdist, + ), + patch.object( + build_utils, + "_iter_batch_results", + side_effect=fake_iter_batch_results, + ), + patch.object(build_utils, "buildkit_disk_usage", return_value=(0, 0)), + patch.object(build_utils, "maybe_prune_buildkit_cache", return_value=False), + ): + exit_code = build_utils.build_all_images( + base_images=["base-1", "base-2", "base-3"], + target="source-minimal", + build_dir=tmp_path, + build_batch_size=2, + ) + + assert exit_code == 0 + assert seen_batches == [["base-1", "base-2"], ["base-3"]] diff --git a/vendor/software-agent-sdk b/vendor/software-agent-sdk index bde715c12..8e8223b24 160000 --- a/vendor/software-agent-sdk +++ b/vendor/software-agent-sdk @@ -1 +1 @@ -Subproject commit bde715c12bce8fb112980529d5ad162f6b81a7f1 +Subproject commit 8e8223b24dfd041875fe845b13326b0a1238b793