✨ Add bsdtar vs pna performance benchmark script#2934
Conversation
Add hyperfine-based benchmark comparing bsdtar and pna create/extract performance across store, deflate, xz, and zstd compression. Generates Markdown and JSON reports with archive size comparison.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces a performance benchmarking suite to compare bsdtar and pna processing speeds. It includes a comprehensive Bash script for automated data generation and benchmarking using hyperfine, as well as a detailed README. Feedback focuses on improving the script's portability and robustness, specifically addressing a non-portable dd flag on macOS, ensuring the realpath dependency is verified, and refining how CPU information is captured on Linux systems.
| for i in $(seq 1 "$zero_count"); do | ||
| local subdir="$target_dir/zero/dir$(( (i - 1) / 50 ))" | ||
| mkdir -p "$subdir" | ||
| dd if=/dev/zero bs=1024 count="$zero_size_kb" of="$subdir/zero_${i}.bin" status=none |
There was a problem hiding this comment.
The status=none flag for dd is a GNU extension and is not supported by the default dd on macOS (BSD-based). This will cause the test data generation to fail on macOS with an unknown status none error. Use 2>/dev/null to silence dd portably.
| dd if=/dev/zero bs=1024 count="$zero_size_kb" of="$subdir/zero_${i}.bin" status=none | |
| dd if=/dev/zero bs=1024 count="$zero_size_kb" of="$subdir/zero_${i}.bin" 2>/dev/null |
| check_command hyperfine | ||
| check_command "$BSDTAR_BIN" | ||
| check_command "$PNA_BIN" |
There was a problem hiding this comment.
The script uses realpath on lines 118 and 236, but it is not checked in the check_command list. Since realpath is not always available by default on all systems (e.g., older macOS versions without GNU coreutils), it should be verified or a fallback should be provided.
| check_command hyperfine | |
| check_command "$BSDTAR_BIN" | |
| check_command "$PNA_BIN" | |
| check_command hyperfine | |
| check_command "$BSDTAR_BIN" | |
| check_command "$PNA_BIN" | |
| if ! command -v realpath &>/dev/null; then | |
| echo "Error: 'realpath' not found. Please install coreutils or provide a realpath implementation." >&2; exit 1 | |
| fi |
| mem_bytes=$(sysctl -n hw.memsize 2>/dev/null || echo 0) | ||
| echo "- **Memory**: $(human_size "$mem_bytes")" | ||
| else | ||
| echo "- **CPU**: $(lscpu 2>/dev/null | awk -F: '/Model name/{gsub(/^ +/,"",$2); print $2}' || echo 'unknown')" |
There was a problem hiding this comment.
If lscpu is missing on a Linux system, the pipe to awk will result in an empty string, and the || echo 'unknown' fallback might not trigger because awk itself exits with status 0. It's safer to capture the output and check if it's empty.
| echo "- **CPU**: $(lscpu 2>/dev/null | awk -F: '/Model name/{gsub(/^ +/,"",$2); print $2}' || echo 'unknown')" | |
| local cpu_info | |
| cpu_info=$(lscpu 2>/dev/null | awk -F: '/Model name/{gsub(/^ +/,"",$2); print $2}') | |
| echo "- **CPU**: ${cpu_info:-unknown}" |
Add hyperfine-based benchmark comparing bsdtar and pna create/extract performance across store, deflate, xz, and zstd compression. Generates Markdown and JSON reports with archive size comparison.