Skip to content

perf(logs): skip unnecessary filtering of ASCII logs - #499

Merged
dormancygrace merged 2 commits into
mainfrom
codex/c906-log-sanitize
Oct 26, 2026
Merged

dormancygrace merged 2 commits into
mainfrom
codex/c906-log-sanitize

Conversation

@dormancygrace

@dormancygrace dormancygrace commented Oct 26, 2026 •

Copy link
Copy Markdown
Owner

Log snapshots always ran an ANSI regexp and classified every rune, even for ordinary ASCII text. Skip the regexp when no ESC is present and bypass Unicode classification for printable ASCII, LF and TAB, retaining secret redaction and fallback behavior.

The committed three-process, 200 ms sweep on the 1 GHz C906 measured a median 478.435 ms -> 333.671 ms (-30.26%) for the existing ~1 MiB ASCII DHCP-log fixture. Allocated bytes fell by about 2 MiB per operation. Complete samples and reproduction commands are in tests/manual/c906-log-sanitize/. These are sanitizer microbenchmarks, not boot-time measurements; fallback-case differences are not claimed as separate gains.

Validation: host logs/logger race tests and vet pass, including valid UTF-8 C1 controls and secrets revealed by filtering. The implementation previously passed the device logs suite and production RISC-V cross-build; this follow-up changes tests and documentation only. Existing cases cover every input byte, invalid UTF-8, ANSI/OSC and Unicode controls. RVV work is separate.

@github-actions

github-actions Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Build outputs

Compared with the build of main at c93fb1a.

1 changed, 0 added, 0 removed of 570 outputs.

directory files
changed server server/NanoKVM-Server
Hashes: outputs-sha256 artifact of this run.

@dormancygrace dormancygrace left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

The change is correct. It keeps every guarantee of the original filter:

  • ESC skip: the regexp can only match text that starts with ESC, so skipping it when no ESC is present is exact.
  • Rune-filter bypass: only printable ASCII, LF and TAB skip the rune filter. C0 controls, DEL, CR, NUL, ESC and all non-ASCII (C1 controls included) still reach it.
  • Redaction order: redaction still runs after filtering.

I also compared the original and the new sanitiser on 2M random inputs built from ESC and partial escape sequences, OSC, DEL, NUL, CR, C1 bytes, invalid UTF-8 and ZWSP. There were no differences. go test -tags teststub ./logs passes.

Only two small comments follow.

Comment thread tests/manual/c906-log-sanitize/README.md
Comment thread server/logs/sanitize_fast_test.go

@dormancygrace dormancygrace left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Reviewed the sanitization fast paths and secret-keyword filtering. The runtime behavior is preserved: ASCII text avoids redundant regexp/UTF-8 passes while escape removal, Unicode controls and redaction remain covered. Added C1-control cases, including a control embedded in a sensitive keyword, and aligned performance claims with the committed measurement artifact. Race tests and vet pass for logs/logger; the integrated merge passes the same race tests. All 15 checks on e2a16c1 pass, including firmware. Remaining findings were documentation/test coverage only; both discussions are resolved.

@dormancygrace
dormancygrace merged commit 27c3cc1 into main Oct 26, 2026
15 checks passed
@dormancygrace
dormancygrace deleted the codex/c906-log-sanitize branch October 26, 2026 21:44
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.

1 participant