Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 53 additions & 24 deletions tools/perf_smoke_test/seed_baselines.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,11 @@ def _prepare_seed_source(workdir: Path, seed_src_dir: Path, sha: str) -> None:
subprocess.run(["chmod", "-R", "a+rwX", str(seed_src_dir)], check=False)


def _create_seed_source_dir() -> Path:
"""Create a unique disposable source-clone directory."""
return Path(tempfile.mkdtemp(prefix="perf-seed-src-"))


def _prepare_jit_cache(cache_dir: Path) -> Path:
"""Create a writable JIT cache directory for one task/backend bucket."""
for subdir in (cache_dir / "warp", cache_dir / "nv"):
Expand All @@ -359,7 +364,15 @@ def _prepare_jit_cache(cache_dir: Path) -> Path:
return cache_dir


def _jit_cache_path(cache_root: Path, target_branch: str, commit: str, task_id: str, backend_key: str) -> Path:
def _prepare_kit_cache(cache_dir: Path) -> Path:
"""Create a writable Kit cache directory for one task/backend bucket."""
cache_dir.mkdir(parents=True, exist_ok=True)
# Kit writes shader and generated-node caches from the in-container user.
_run(["chmod", "-R", "0777", str(cache_dir)], check=False)
return cache_dir


def _cache_bucket_path(cache_root: Path, target_branch: str, commit: str, task_id: str, backend_key: str) -> Path:
"""Return the cache shared by repeated samples of one task/backend bucket."""
return (
cache_root
Expand All @@ -370,14 +383,14 @@ def _jit_cache_path(cache_root: Path, target_branch: str, commit: str, task_id:
)


def _cleanup_jit_cache(cache_dir: Path) -> None:
"""Remove the disposable JIT cache for one seeder invocation."""
if not cache_dir.exists():
def _cleanup_run_dir(run_dir: Path) -> None:
"""Remove a disposable directory for one seeder invocation."""
if not run_dir.exists():
return
subprocess.run(["chmod", "-R", "a+rwX", str(cache_dir)], check=False, capture_output=True, text=True)
shutil.rmtree(cache_dir, ignore_errors=True)
if cache_dir.exists():
print(f"::warning::[seed] Could not fully remove JIT cache {cache_dir}")
subprocess.run(["chmod", "-R", "a+rwX", str(run_dir)], check=False, capture_output=True, text=True)
shutil.rmtree(run_dir, ignore_errors=True)
if run_dir.exists():
print(f"::warning::[seed] Could not fully remove run directory {run_dir}")


def _docker_run_benchmark(
Expand All @@ -395,9 +408,10 @@ def _docker_run_benchmark(
seed_token = f"--seed {task.seed}" if task.seed is not None else ""
inner = (
"set -e\n"
# Warp creates nested cache directories at runtime. A permissive umask
# keeps them writable when successive containers use different host uids.
# Warp and Kit create nested cache directories at runtime. A permissive
# umask keeps them writable for successive containers.
"umask 000\n"
"mkdir -p /tmp/bench_out\n"
"cd /workspace/isaaclab\n"
"rm -f _isaac_sim\n"
"ln -s /isaac-sim _isaac_sim\n"
Expand All @@ -414,7 +428,6 @@ def _docker_run_benchmark(
cmd = [
"docker",
"run",
"--rm",
"--name",
container_name,
"--init",
Expand Down Expand Up @@ -449,8 +462,6 @@ def _docker_run_benchmark(
"-e",
"CUDA_CACHE_PATH=/tmp/jit-cache/nv",
"-v",
f"{artifact_dir}:/tmp/bench_out",
"-v",
f"{jit_cache}:/tmp/jit-cache",
"-v",
f"{kit_cache}:/isaac-sim/kit/cache",
Expand All @@ -462,7 +473,20 @@ def _docker_run_benchmark(
cmd += [image, "-c", inner]

result = subprocess.run(cmd, text=True)
return result.returncode
permissions_returncode = 0
try:
copy_result = subprocess.run(
["docker", "cp", f"{container_name}:/tmp/bench_out/.", str(artifact_dir)],
text=True,
)
if copy_result.returncode == 0:
permissions_returncode = subprocess.run(
["chmod", "-R", "a+rwX", str(artifact_dir)],
text=True,
).returncode
finally:
subprocess.run(["docker", "rm", "-f", container_name], capture_output=True, text=True)
return result.returncode or copy_result.returncode or permissions_returncode
Comment on lines +476 to +489

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 copy_result referenced outside the try block it is assigned in

copy_result is assigned only inside the try block (line 478) but read on the return at line 489, which is outside the try/finally. At runtime this is safe: any OS-level exception from subprocess.run(["docker", "cp", ...]) propagates through finally and exits the function without reaching the return. However, static-analysis tools (mypy, ruff) flag this as a potentially-unbound reference, and a future maintainer who wraps the outer body in a try/except could trigger a genuine NameError. Initializing copy_result = subprocess.CompletedProcess([], returncode=1) (or similar sentinel) before the try block would make the intent explicit and satisfy static analysis without changing runtime behaviour.



def _build_bench_result(task: TaskConfig, artifact_dir: Path, exit_code: int, wall_time_s: int) -> None:
Expand Down Expand Up @@ -621,16 +645,18 @@ def main() -> int:
artifacts_root = (workdir / args.artifacts_root).resolve()
artifacts_root.mkdir(parents=True, exist_ok=True)

seed_src_dir = (
Path(args.seed_src_dir).resolve() if args.seed_src_dir else Path(tempfile.gettempdir()) / "perf-seed-src"
)
if args.seed_src_dir:
seed_src_dir = Path(args.seed_src_dir).resolve()
else:
seed_src_dir = _create_seed_source_dir()
atexit.register(_cleanup_run_dir, seed_src_dir)
run_id = os.environ.get("GITHUB_RUN_ID", f"local-{os.getpid()}")
run_attempt = os.environ.get("GITHUB_RUN_ATTEMPT", "0")
jit_cache_root = workdir / "jit-cache" / "seed" / f"run-{run_id}-attempt-{run_attempt}"
atexit.register(_cleanup_jit_cache, jit_cache_root)
kit_cache = workdir / "kit-cache"
kit_cache.mkdir(parents=True, exist_ok=True)
_run(["chmod", "-R", "0777", str(kit_cache)], check=False)
cache_run = f"run-{run_id}-attempt-{run_attempt}"
jit_cache_root = workdir / "jit-cache" / "seed" / cache_run
kit_cache_root = workdir / "kit-cache" / "seed" / cache_run
atexit.register(_cleanup_run_dir, jit_cache_root)
atexit.register(_cleanup_run_dir, kit_cache_root)

plan = _build_seed_plan(args)
plan = _filter_plan_by_ancestry(
Expand Down Expand Up @@ -675,7 +701,10 @@ def main() -> int:
continue
for task in tasks:
task_jit_cache = _prepare_jit_cache(
_jit_cache_path(jit_cache_root, target_branch, commit, task.task_id, task.backend_key)
_cache_bucket_path(jit_cache_root, target_branch, commit, task.task_id, task.backend_key)
)
task_kit_cache = _prepare_kit_cache(
_cache_bucket_path(kit_cache_root, target_branch, commit, task.task_id, task.backend_key)
)
for sample_idx in range(args.samples_per_commit):
total += 1
Expand All @@ -699,7 +728,7 @@ def main() -> int:
task=task,
artifact_dir=artifact_dir,
jit_cache=task_jit_cache,
kit_cache=kit_cache,
kit_cache=task_kit_cache,
seed_src_dir=seed_src_dir if source_mount else None,
container_name=container,
)
Expand Down
93 changes: 87 additions & 6 deletions tools/perf_smoke_test/test/test_seed_ancestry.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import subprocess
import sys
from pathlib import Path
from types import SimpleNamespace

import pytest

Expand Down Expand Up @@ -145,16 +146,29 @@ def test_prepare_jit_cache_opens_task_cache(tmp_path: Path) -> None:
assert cache_dir.stat().st_mode & 0o777 == 0o777


def test_seed_source_directories_are_unique_and_disposable() -> None:
"""Independent seeder invocations do not reuse source-clone residue."""
first = seed_baselines._create_seed_source_dir()
second = seed_baselines._create_seed_source_dir()
try:
assert first != second
assert first.is_dir()
assert second.is_dir()
finally:
seed_baselines._cleanup_run_dir(first)
seed_baselines._cleanup_run_dir(second)


def test_jit_cache_reuses_samples_and_isolates_backends(tmp_path: Path) -> None:
"""Repeated samples share compiled kernels, but different backends do not."""
cache_root = tmp_path / "jit-cache" / "seed" / "run-1-attempt-1"
newton_cache = seed_baselines._jit_cache_path(
newton_cache = seed_baselines._cache_bucket_path(
cache_root, "perf-smoke/develop-staging", "abcdef123456", "Isaac-Cartpole-Direct", "newton"
)
repeated_newton_cache = seed_baselines._jit_cache_path(
repeated_newton_cache = seed_baselines._cache_bucket_path(
cache_root, "perf-smoke/develop-staging", "abcdef123456", "Isaac-Cartpole-Direct", "newton"
)
physx_cache = seed_baselines._jit_cache_path(
physx_cache = seed_baselines._cache_bucket_path(
cache_root, "perf-smoke/develop-staging", "abcdef123456", "Isaac-Cartpole-Direct", "physx"
)

Expand All @@ -169,14 +183,81 @@ def test_jit_cache_reuses_samples_and_isolates_backends(tmp_path: Path) -> None:
assert not physx_cache.exists()


def test_cleanup_jit_cache_removes_run_tree(tmp_path: Path) -> None:
"""Seeder exit cleanup removes compiled kernels from its run directory."""
def test_kit_cache_reuses_samples_and_isolates_backends(tmp_path: Path) -> None:
"""Repeated samples share Kit data, but different backends do not."""
cache_root = tmp_path / "kit-cache" / "seed" / "run-1-attempt-1"
newton_cache = seed_baselines._cache_bucket_path(
cache_root, "perf-smoke/develop-staging", "abcdef123456", "Isaac-Camera-Direct", "newton"
)
repeated_newton_cache = seed_baselines._cache_bucket_path(
cache_root, "perf-smoke/develop-staging", "abcdef123456", "Isaac-Camera-Direct", "newton"
)
physx_cache = seed_baselines._cache_bucket_path(
cache_root, "perf-smoke/develop-staging", "abcdef123456", "Isaac-Camera-Direct", "physx"
)

_ = seed_baselines._prepare_kit_cache(newton_cache)
shader_cache = newton_cache / "shader-cache.bin"
shader_cache.write_bytes(b"compiled")
_ = seed_baselines._prepare_kit_cache(repeated_newton_cache)

assert repeated_newton_cache == newton_cache
assert shader_cache.read_bytes() == b"compiled"
assert newton_cache.stat().st_mode & 0o777 == 0o777
assert physx_cache != newton_cache
assert not physx_cache.exists()


def test_cleanup_run_dir_removes_run_tree(tmp_path: Path) -> None:
"""Seeder exit cleanup removes generated files from its run directory."""
cache_dir = tmp_path / "jit-cache" / "seed" / "run-1-attempt-1"
compiled_dir = cache_dir / "warp" / "version" / "module"
compiled_dir.mkdir(parents=True)
(compiled_dir / "kernel.so").write_bytes(b"compiled")
compiled_dir.chmod(0o500)

seed_baselines._cleanup_jit_cache(cache_dir)
seed_baselines._cleanup_run_dir(cache_dir)

assert not cache_dir.exists()


def test_docker_benchmark_copies_internal_output_after_exit(tmp_path: Path, monkeypatch) -> None:
"""Benchmark output is copied from the container instead of bind-mounted."""
calls: list[list[str]] = []

def _run(cmd: list[str], **_kwargs) -> SimpleNamespace:
calls.append(cmd)
return SimpleNamespace(returncode=0)

task = SimpleNamespace(
task_id="Isaac-Cartpole-Direct",
num_envs=16,
num_frames=10,
warmup_frames=2,
seed=42,
)
artifact_dir = tmp_path / "artifacts"
monkeypatch.setattr(seed_baselines, "hydra_args_for_task", lambda _task: [])
monkeypatch.setattr(seed_baselines.subprocess, "run", _run)

exit_code = seed_baselines._docker_run_benchmark(
image="isaac-lab:test",
task=task,
artifact_dir=artifact_dir,
jit_cache=tmp_path / "jit-cache",
kit_cache=tmp_path / "kit-cache",
seed_src_dir=tmp_path / "source",
container_name="perf-seed-test",
)

docker_run = calls[0]
assert exit_code == 0
assert "--rm" not in docker_run
assert f"{artifact_dir}:/tmp/bench_out" not in docker_run
assert f"{tmp_path / 'jit-cache'}:/tmp/jit-cache" in docker_run
assert f"{tmp_path / 'kit-cache'}:/isaac-sim/kit/cache" in docker_run
assert "umask 000" in docker_run[-1]
assert "mkdir -p /tmp/bench_out" in docker_run[-1]
assert calls[1] == ["docker", "cp", "perf-seed-test:/tmp/bench_out/.", str(artifact_dir)]
assert calls[2] == ["chmod", "-R", "a+rwX", str(artifact_dir)]
assert calls[3] == ["docker", "rm", "-f", "perf-seed-test"]
Loading