Skip to content

fix: validate memory measurement is numeric before display#86

Merged
BryanFRD merged 1 commit intomainfrom
fix/memory-display
Apr 11, 2026
Merged

fix: validate memory measurement is numeric before display#86
BryanFRD merged 1 commit intomainfrom
fix/memory-display

Conversation

@BryanFRD
Copy link
Copy Markdown
Contributor

Summary

  • Fix measure_memory to check /usr/bin/time exists before calling it
  • Validate extracted RSS value is numeric before formatting
  • In markdown output, only accept .mem values matching ^[0-9]+\.?[0-9]*$
  • Prevents 46.6N/A MB display when memory measurement partially fails in Docker

Test plan

  • Run benchmarks in Docker environment
  • Shellcheck clean

Copilot AI review requested due to automatic review settings April 10, 2026 19:58
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens peak memory (RSS) reporting in the benchmark runner to avoid rendering malformed memory strings when measurement fails.

Changes:

  • Add a Linux + /usr/bin/time availability guard before attempting RSS measurement.
  • Validate that the extracted RSS value is numeric before converting/formatting it.
  • In markdown output, only accept .mem values that match a numeric pattern before displaying them.

Comment on lines +127 to +130
local raw
raw=$(/usr/bin/time -v "$@" 2>&1 >/dev/null | grep "Maximum resident" | awk '{print $6}')
if [[ -n "$raw" && "$raw" =~ ^[0-9]+$ ]]; then
awk "BEGIN {printf \"%.1f\", $raw/1024}"
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

With set -euo pipefail, the raw=$(...) pipeline can terminate the entire script if any stage returns non-zero (e.g., /usr/bin/time -v not supported, or the expected "Maximum resident" line isn’t present and grep exits 1). Since this function is intended to fall back to N/A, make the extraction pipeline non-fatal (e.g., append || true inside the command substitution, or avoid grep by using a single awk that yields empty output and exits 0 when no match is found).

Copilot uses AI. Check for mistakes.
@BryanFRD BryanFRD merged commit 6b177f2 into main Apr 11, 2026
11 checks passed
@BryanFRD BryanFRD deleted the fix/memory-display branch April 11, 2026 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants