From a5ad0069b2a6494f11fd3346b75756984217f79f Mon Sep 17 00:00:00 2001 From: Cheng Wan <54331508+ch-wan@users.noreply.github.com> Date: Mon, 17 Nov 2025 23:18:43 -0800 Subject: [PATCH] fix: change performance log directory to cache path (#13482) Co-authored-by: Mick --- .github/workflows/pr-test.yml | 89 ++++--------------- .../runtime/utils/performance_logger.py | 29 ++++-- .../test/server/diffusion_server.py | 2 +- .../sglang/multimodal_gen/test/test_utils.py | 22 +++-- 4 files changed, 52 insertions(+), 90 deletions(-) diff --git a/.github/workflows/pr-test.yml b/.github/workflows/pr-test.yml index ea37c16e4..ba4625895 100644 --- a/.github/workflows/pr-test.yml +++ b/.github/workflows/pr-test.yml @@ -21,9 +21,6 @@ concurrency: group: pr-test-${{ github.ref }} cancel-in-progress: true -env: - SGLANG_IS_IN_CI: true - jobs: call-gate: uses: ./.github/workflows/pr-gate.yml @@ -169,11 +166,6 @@ jobs: cd sgl-kernel pytest tests/ - - name: Cleanup logs directory - if: always() - run: | - rm -rf python/sglang/logs || true - sgl-kernel-mla-test: needs: [check-changes, sgl-kernel-build-wheels] if: needs.check-changes.outputs.sgl_kernel == 'true' @@ -205,11 +197,6 @@ jobs: cd test/srt python3 test_mla_deepseek_v3.py - - name: Cleanup logs directory - if: always() - run: | - rm -rf python/sglang/logs || true - sgl-kernel-benchmark-test: needs: [check-changes, sgl-kernel-build-wheels] if: needs.check-changes.outputs.sgl_kernel == 'true' @@ -254,35 +241,24 @@ jobs: echo "All benchmark tests completed!" - - name: Cleanup logs directory - if: always() - run: | - rm -rf python/sglang/logs || true - # =============================================== multimodal_gen ==================================================== multimodal-gen-test: - needs: [check-changes] - if: needs.check-changes.outputs.multimodal_gen == 'true' - runs-on: 1-gpu-runner - steps: - - name: Checkout code - uses: actions/checkout@v4 + needs: [check-changes] + if: needs.check-changes.outputs.multimodal_gen == 'true' + runs-on: 1-gpu-runner + steps: + - name: Checkout code + uses: actions/checkout@v4 - - name: Install dependencies - run: | - CUSTOM_BUILD_SGL_KERNEL=${{needs.check-changes.outputs.sgl_kernel}} bash scripts/ci/ci_install_dependency.sh diffusion + - name: Install dependencies + run: | + CUSTOM_BUILD_SGL_KERNEL=${{needs.check-changes.outputs.sgl_kernel}} bash scripts/ci/ci_install_dependency.sh diffusion - - name: Run diffusion server tests - timeout-minutes: 60 - run: | - cd python - pytest -s -v --log-cli-level=INFO sglang/multimodal_gen/test/server/test_server_performance.py - - - name: Cleanup logs directory - if: always() - run: | - # Remove logs directory to prevent NFS lock files - rm -rf python/sglang/logs || true + - name: Run diffusion server tests + timeout-minutes: 60 + run: | + cd python + pytest -s -v --log-cli-level=INFO sglang/multimodal_gen/test/server/test_server_performance.py # Adding a single CUDA13 smoke test to verify that the kernel builds and runs # TODO: Add back this test when it can pass on CI @@ -387,11 +363,6 @@ jobs: cd test/srt python3 run_suite.py --suite per-commit-1-gpu --auto-partition-id ${{ matrix.part }} --auto-partition-size 15 - - name: Cleanup logs directory - if: always() - run: | - rm -rf python/sglang/logs || true - unit-test-backend-2-gpu: needs: [check-changes, unit-test-backend-1-gpu, sgl-kernel-build-wheels] if: always() && !failure() && !cancelled() && @@ -425,11 +396,6 @@ jobs: cd test/srt python3 run_suite.py --suite per-commit-2-gpu --auto-partition-id ${{ matrix.part }} --auto-partition-size 2 - - name: Cleanup logs directory - if: always() - run: | - rm -rf python/sglang/logs || true - unit-test-backend-4-gpu: needs: [check-changes, unit-test-backend-2-gpu, sgl-kernel-build-wheels] if: always() && !failure() && !cancelled() && @@ -591,11 +557,6 @@ jobs: python3 -m unittest test_bench_serving.TestBenchServing.test_lora_online_latency python3 -m unittest test_bench_serving.TestBenchServing.test_lora_online_latency_with_concurrent_adapter_updates - - name: Cleanup logs directory - if: always() - run: | - rm -rf python/sglang/logs || true - performance-test-1-gpu-part-2: needs: [check-changes, sgl-kernel-build-wheels, stage-a-test-1] if: always() && !failure() && !cancelled() && @@ -649,11 +610,6 @@ jobs: cd test/srt python3 -m unittest test_bench_serving.TestBenchServing.test_vlm_online_latency - - name: Cleanup logs directory - if: always() - run: | - rm -rf python/sglang/logs || true - performance-test-1-gpu-part-3: needs: [check-changes, sgl-kernel-build-wheels, stage-a-test-1] if: always() && !failure() && !cancelled() && @@ -765,11 +721,6 @@ jobs: cd test/srt python3 -m unittest test_bench_serving.TestBenchServing.test_pp_long_context_prefill - - name: Cleanup logs directory - if: always() - run: | - rm -rf python/sglang/logs || true - accuracy-test-1-gpu: needs: [check-changes, sgl-kernel-build-wheels, stage-a-test-1] if: always() && !failure() && !cancelled() && @@ -802,11 +753,6 @@ jobs: cd test/srt python3 test_eval_accuracy_large.py - - name: Cleanup logs directory - if: always() - run: | - rm -rf python/sglang/logs || true - accuracy-test-2-gpu: needs: [check-changes, accuracy-test-1-gpu, sgl-kernel-build-wheels] if: always() && !failure() && !cancelled() && @@ -839,11 +785,6 @@ jobs: cd test/srt python3 test_moe_eval_accuracy_large.py - - name: Cleanup logs directory - if: always() - run: | - rm -rf python/sglang/logs || true - unit-test-deepep-4-gpu: needs: [check-changes, unit-test-backend-2-gpu, sgl-kernel-build-wheels] if: always() && !failure() && !cancelled() && @@ -978,7 +919,7 @@ jobs: sgl-kernel-mla-test, sgl-kernel-benchmark-test, - # multimodal-gen-test, + multimodal-gen-test, stage-a-test-1, unit-test-backend-1-gpu, diff --git a/python/sglang/multimodal_gen/runtime/utils/performance_logger.py b/python/sglang/multimodal_gen/runtime/utils/performance_logger.py index 6bf066d57..95d93ca2b 100644 --- a/python/sglang/multimodal_gen/runtime/utils/performance_logger.py +++ b/python/sglang/multimodal_gen/runtime/utils/performance_logger.py @@ -10,13 +10,28 @@ from typing import Any from dateutil.tz import UTC -LOG_DIR = os.environ.get("SGLANG_PERF_LOG_DIR") -if LOG_DIR: - LOG_DIR = os.path.abspath(LOG_DIR) -elif LOG_DIR is None: # Not set - project_root = os.path.abspath(os.path.join(os.path.dirname(__file__), "../../../")) - LOG_DIR = os.path.join(project_root, "logs") -# if LOG_DIR is "", it will remain "", disabling file logging. + +def get_diffusion_perf_log_dir() -> str: + """ + Determines the directory for performance logs, centralizing the logic. + + Resolution order: + 1. SGLANG_PERF_LOG_DIR environment variable, if set and not empty. + 2. Default to ~/.cache/sglang/logs if the environment variable is not set. + 3. Returns an empty string if SGLANG_PERF_LOG_DIR is set to an empty string, + which effectively disables file logging. + """ + log_dir = os.environ.get("SGLANG_PERF_LOG_DIR") + if log_dir: + return os.path.abspath(log_dir) + if log_dir is None: + # Not set, use default + return os.path.join(os.path.expanduser("~/.cache/sglang"), "logs") + # Is set, but is an empty string + return "" + + +LOG_DIR = get_diffusion_perf_log_dir() # Configure a specific logger for performance metrics perf_logger = logging.getLogger("performance") diff --git a/python/sglang/multimodal_gen/test/server/diffusion_server.py b/python/sglang/multimodal_gen/test/server/diffusion_server.py index fabfe0e60..6a5df3eb9 100644 --- a/python/sglang/multimodal_gen/test/server/diffusion_server.py +++ b/python/sglang/multimodal_gen/test/server/diffusion_server.py @@ -105,7 +105,7 @@ class ServerManager: def start(self) -> ServerContext: """Start the diffusion server and wait for readiness.""" - log_dir, perf_log_path = prepare_perf_log(Path(__file__)) + log_dir, perf_log_path = prepare_perf_log() safe_model_name = self.model.replace("/", "_") stdout_path = ( diff --git a/python/sglang/multimodal_gen/test/test_utils.py b/python/sglang/multimodal_gen/test/test_utils.py index a923e5697..a1e315f07 100644 --- a/python/sglang/multimodal_gen/test/test_utils.py +++ b/python/sglang/multimodal_gen/test/test_utils.py @@ -17,6 +17,9 @@ from PIL import Image from sglang.multimodal_gen.configs.sample.base import DataType from sglang.multimodal_gen.runtime.utils.common import get_bool_env_var from sglang.multimodal_gen.runtime.utils.logging_utils import init_logger +from sglang.multimodal_gen.runtime.utils.performance_logger import ( + get_diffusion_perf_log_dir, +) logger = init_logger(__name__) @@ -107,12 +110,15 @@ def check_image_size(ut, image, width, height): ut.assertEqual(image.size, (width, height)) -def get_perf_log_dir(start_file: Path) -> Path: - """Mirror runtime/utils/performance_logger.py behaviour for locating logs.""" - this_file = start_file.resolve() - root_logs = this_file.parents[3] / "logs" - fallback = this_file.parents[2] / "logs" - return root_logs if root_logs.exists() or not fallback.exists() else fallback +def get_perf_log_dir() -> Path: + """Gets the performance log directory from the centralized sglang utility.""" + log_dir_str = get_diffusion_perf_log_dir() + if not log_dir_str: + raise RuntimeError( + "Performance logging is disabled (SGLANG_PERF_LOG_DIR is empty), " + "but a test tried to access the log directory." + ) + return Path(log_dir_str) def _ensure_log_path(log_dir: Path) -> Path: @@ -129,9 +135,9 @@ def clear_perf_log(log_dir: Path) -> Path: return log_path -def prepare_perf_log(start_file: Path) -> tuple[Path, Path]: +def prepare_perf_log() -> tuple[Path, Path]: """Convenience helper to resolve and clear the perf log in one call.""" - log_dir = get_perf_log_dir(start_file) + log_dir = get_perf_log_dir() log_path = clear_perf_log(log_dir) return log_dir, log_path