util: Cap perf workload logs#944
Conversation
Change-Id: Iaa608acb2347577fee8b2a8e530849beae3e189a
📝 WalkthroughWalkthroughAdds a configurable per-workload log truncation limit (default 64 MiB) to distributed_sim.py via a new default helper, CLI option, and threading through run_scheduler/make_job_script; the generated job script pipes output through tail -c and preserves exit status via PIPESTATUS. Mirrors this behavior in parallel_sim.sh with a validated GEM5_PERF_LOG_LIMIT_BYTES variable. ChangesConfigurable Log Truncation
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant main
participant run_scheduler
participant make_job_script
participant GeneratedJobScript
main->>run_scheduler: log_limit_bytes
run_scheduler->>make_job_script: log_limit_bytes
make_job_script->>GeneratedJobScript: embed log_limit_bytes variable
GeneratedJobScript->>GeneratedJobScript: run job, pipe stdout through tail -c
GeneratedJobScript->>GeneratedJobScript: write log.txt, exit via PIPESTATUS[0]
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66796134cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| exit 1 | ||
| fi | ||
|
|
||
| run $checkpoint $work_dir 2>&1 | tail -c "$GEM5_PERF_LOG_LIMIT_BYTES" >$work_dir/$log_file |
There was a problem hiding this comment.
For long-running GEM5 jobs, this pipeline leaves log.txt empty until the workload exits: tail --help defines -c NUM as outputting the last NUM bytes, so when reading from a pipe it must wait for EOF before writing the retained suffix. If a perf run is still being debugged, or the job group is interrupted by Ctrl-C/CI timeout before tail flushes, the old live diagnostics are replaced by an empty/truncated log; the same tail -c pattern is also added to distributed_sim.py. Please use a streaming cap/rotation approach so diagnostics remain visible during and after interrupted runs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR limits the size of per-workload log.txt files produced by perf-style GEM5 runs, preventing multi-GB logs from filling shared storage while keeping the most recent output (where failures/panics typically appear).
Changes:
- Add a
GEM5_PERF_LOG_LIMIT_BYTESdefault (64 MiB) and cap localparallel_sim.shworkload logs to the trailing bytes. - Add
--log-limit-bytes(defaulting fromGEM5_PERF_LOG_LIMIT_BYTES) and apply the same log cap to remote job scripts generated bydistributed_sim.py.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| util/xs_scripts/parallel_sim.sh | Introduces a default log size limit and truncates workload logs to the trailing bytes. |
| util/xs_scripts/distributed_sim.py | Adds a log-limit CLI/env default and applies tail-capping in generated remote job scripts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| exit 1 | ||
| fi | ||
|
|
||
| run $checkpoint $work_dir 2>&1 | tail -c "$GEM5_PERF_LOG_LIMIT_BYTES" >$work_dir/$log_file |
| def parse_positive_int(value: str) -> int: | ||
| number = int(value) | ||
| if number < 1: | ||
| raise argparse.ArgumentTypeError("must be >= 1") | ||
| return number | ||
|
|
||
|
|
||
| def default_log_limit_bytes() -> int: | ||
| value = os.environ.get("GEM5_PERF_LOG_LIMIT_BYTES") | ||
| if not value: | ||
| return DEFAULT_LOG_LIMIT_BYTES | ||
| return parse_positive_int(value) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@util/xs_scripts/parallel_sim.sh`:
- Line 233: The log redirection in parallel_sim.sh is using an unquoted
$work_dir/$log_file target, which can trigger word-splitting or globbing. Update
the run/tail pipeline so the redirection target is properly quoted, keeping the
behavior in the same script section that writes to the log file and preserving
the existing $checkpoint, $work_dir, and $log_file usage.
- Line 233: The pipeline in arg_wrapper currently hides the exit status of
run/check because tail is the last command, so failed jobs can look successful.
Update arg_wrapper to preserve and return run’s status by capturing
${PIPESTATUS[0]} immediately after the pipeline (or by enabling pipefail) before
the function exits, while keeping the existing logging behavior in
parallel_sim.sh.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad5d0381-97ab-48b3-b764-48deb84a64cf
📒 Files selected for processing (2)
util/xs_scripts/distributed_sim.pyutil/xs_scripts/parallel_sim.sh
| exit 1 | ||
| fi | ||
|
|
||
| run $checkpoint $work_dir 2>&1 | tail -c "$GEM5_PERF_LOG_LIMIT_BYTES" >$work_dir/$log_file |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Quote the redirect target to avoid globbing/word-splitting.
Shellcheck flags $work_dir/$log_file as unquoted (SC2086).
🛠️ Proposed fix
- run $checkpoint $work_dir 2>&1 | tail -c "$GEM5_PERF_LOG_LIMIT_BYTES" >$work_dir/$log_file
+ run $checkpoint $work_dir 2>&1 | tail -c "$GEM5_PERF_LOG_LIMIT_BYTES" >"$work_dir/$log_file"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| run $checkpoint $work_dir 2>&1 | tail -c "$GEM5_PERF_LOG_LIMIT_BYTES" >$work_dir/$log_file | |
| run $checkpoint $work_dir 2>&1 | tail -c "$GEM5_PERF_LOG_LIMIT_BYTES" >"$work_dir/$log_file" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 233-233: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 233-233: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@util/xs_scripts/parallel_sim.sh` at line 233, The log redirection in
parallel_sim.sh is using an unquoted $work_dir/$log_file target, which can
trigger word-splitting or globbing. Update the run/tail pipeline so the
redirection target is properly quoted, keeping the behavior in the same script
section that writes to the log file and preserving the existing $checkpoint,
$work_dir, and $log_file usage.
Source: Linters/SAST tools
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for callers relying on arg_wrapper's exit status
rg -n -B3 -A5 'arg_wrapper' util/xs_scripts/parallel_sim.shRepository: OpenXiangShan/GEM5
Length of output: 1008
🏁 Script executed:
#!/bin/bash
sed -n '200,255p' util/xs_scripts/parallel_sim.shRepository: OpenXiangShan/GEM5
Length of output: 1657
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' util/xs_scripts/parallel_sim.sh
printf '\n--- parallel invocation context ---\n'
sed -n '242,255p' util/xs_scripts/parallel_sim.shRepository: OpenXiangShan/GEM5
Length of output: 7602
🌐 Web query:
GNU parallel exit status default behavior job failure wrapper exit code
💡 Result:
By default, GNU Parallel does not halt when a job fails, and its exit status is the number of jobs that failed [1][2]. If all jobs succeed, the default exit status is 0 [3][4]. To change this behavior, you can use the --halt (or --halt-on-error) option, which allows GNU Parallel to terminate based on job success or failure [3][5]. The behavior changes depending on the value provided to --halt [1][2]: - --halt 1 (or --halt soon,fail=1): GNU Parallel will stop spawning new jobs if a job fails, but will allow currently running jobs to complete. The exit status of the final GNU Parallel command will be the exit status of the last failing job [1][2]. - --halt 2 (or --halt now,fail=1): GNU Parallel will kill all running jobs immediately when a job fails and exit. The exit status will be the exit status of the failing job [1][2]. For more complex control, you can use the syntax --halt when,why, where 'when' can be 'now' (kill immediately) or 'soon' (wait for running jobs), and 'why' defines the failure/success condition (e.g., 'fail=X' or 'fail=Y%') [3][5][6]. If you need to track the exit status of every individual job, you can use the --joblog option [5][2]. This creates a log file containing a tab-separated record for each job, including its specific exit status and signal information [5][7].
Citations:
- 1: https://lists.gnu.org/archive/html/bug-parallel/2015-04/msg00004.html
- 2: https://stackoverflow.com/questions/38133201/how-can-i-check-the-exit-code-of-individual-process-running-in-parallel-executed
- 3: https://www.gnu.org/software/parallel/man.html
- 4: https://man.archlinux.org/man/parallel.1
- 5: https://manpages.debian.org/trixie/parallel/parallel.1.en.html
- 6: https://manpages.ubuntu.com/manpages/noble/man1/parallel.1.html
- 7: https://www.gnu.org/software/parallel/parallel_tutorial.pdf
🏁 Script executed:
#!/bin/bash
rg -n 'pipefail|set -o pipefail|set -euo pipefail' util/xs_scripts/common.sh util/xs_scripts/parallel_sim.shRepository: OpenXiangShan/GEM5
Length of output: 156
Preserve run's exit code here. parallel calls arg_wrapper directly, so this pipeline needs to return run/check's status; otherwise failed jobs look successful and won't be reported or retried. Capture ${PIPESTATUS[0]} (or enable pipefail) before the function returns.
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 233-233: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 233-233: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@util/xs_scripts/parallel_sim.sh` at line 233, The pipeline in arg_wrapper
currently hides the exit status of run/check because tail is the last command,
so failed jobs can look successful. Update arg_wrapper to preserve and return
run’s status by capturing ${PIPESTATUS[0]} immediately after the pipeline (or by
enabling pipefail) before the function exits, while keeping the existing logging
behavior in parallel_sim.sh.
🚀 Coremark Smoke Test Results
✅ Difftest smoke test passed! |
Motivation
A noisy GEM5 run can write multi-GB
log.txtfiles under perf archives and exhaust shared NFS before old-run cleanup runs.Approach
GEM5_PERF_LOG_LIMIT_BYTESbytes for localparallel_sim.shworkload logs. The default is 64 MiB.distributed_sim.pyremote job scripts, with a--log-limit-bytesoverride.completed/abortmarker behavior and keep the log tail so final panic or exit status remains visible.Validation
bash -n util/xs_scripts/parallel_sim.shPYTHONPYCACHEPREFIX=/tmp/gem5_review_pycache python3 -m py_compile util/xs_scripts/distributed_sim.pygit diff --checkSummary by CodeRabbit
New Features
Bug Fixes