Skip to content

perf(cli): shrink help sorting without allocating cached keys - #1396

Merged
jdx merged 2 commits into
mainfrom
codex/help-ordering-size
Sep 7, 2026
Merged

perf(cli): shrink help sorting without allocating cached keys#1396
jdx merged 2 commits into
mainfrom
codex/help-ordering-size

Conversation

@jdx

@jdx jdx commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Reduce help-rendering code size while retaining allocation-free unstable sorting:

  • Compute declaration positions directly from metadata slice addresses, with identity checks preserving the existing fallback for outside references.
  • Keep comparator bodies out of the sort implementation and share command ordering between ordinary and recursive help.
  • Add ordering regression tests and an in-process mise help benchmark (cargo run -p gate --release --bin time-help).

Size

Measured with the oxc migration PR, comparing bpaf on main (0d31f5865) with the migration (5396b74e1) plus this patch:

Binary bpaf usage-rs + this patch Difference
oxlint 12,421,856 B 12,669,440 B +247,584 B (+1.99%)
oxfmt 5,044,368 B 5,241,888 B +197,520 B (+3.92%)

This patch saves 180,592 B (176.4 KiB) combined versus the migration using released usage-rs 6.8.0. Measurements use Rust 1.98.1, macOS arm64, fat LTO, one codegen unit, allocator enabled, and identical stripping/signing; oxfmt disables default features.

Performance and validation

Local mise help timings ranged from −0.2% to +1.3%; the unchanged KDL control varied by 5%, so these samples do not establish an instruction-count improvement. Sorting adds no cached-key allocation. Oxc help and 719 KB of sampled mise help/KDL output are byte-identical to baseline.

  • Clippy, formatting, and Rust 1.91 compatibility checks passed.
  • Full workspace tests: 2,722 passed; the sole failure is a zsh interactive-completion timeout also reproduced on unmodified main.

AI-assisted with Codex (GPT-6).

Summary by CodeRabbit

  • Bug Fixes
    • Help output now consistently preserves declaration order when arguments or flags share the same display order.
    • Subcommands and usage synopsis entries are ordered consistently throughout help pages, including nested commands.
    • Argument and flag listings are sorted more efficiently, improving performance when generating help information.

Entire-Checkpoint: 01M1X151XSYT9H3094FKJSNGCM
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Team

Run ID: 03eae666-ec45-49ec-bbed-9bf842a30789

📥 Commits

Reviewing files that changed from the base of the PR and between 8085571 and 603706d.

📒 Files selected for processing (2)
  • argv/src/help.rs
  • benches/gate/src/bin/time-help.rs

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

Help ordering now uses outlined comparators and declaration-position lookup. Recursive command ordering shares one implementation. Tests cover edge cases and tie-breaking. A new benchmark binary measures help rendering, KDL serialization, and recursive help rendering.

Changes

Help ordering

Layer / File(s) Summary
Declaration-position ordering
argv/src/help.rs
Usage names, arguments, flags, and commands use outlined comparators. Declaration positions support sliced, empty, external, and zero-sized inputs.
Recursive command ordering
argv/src/help.rs
order_commands centralizes command ordering, including recursive help rendering.
Ordering validation and benchmarks
argv/src/help.rs, benches/gate/src/bin/time-help.rs
Tests validate declaration-position matching and declaration-order tie-breaking. The benchmark measures selected help pages, KDL serialization, and recursive help rendering.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 60370

Help-row ordering now computes declaration positions efficiently while preserving explicit display ordering and tie-breaking. The changed ordering paths and regression cases show no remaining merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the performance change in CLI help sorting and the removal of cached-key allocation. It is concise and related to the primary changes.
  • Fix all pre-merge checks with AI

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Entire-Checkpoint: 01M1X2X9FX4CSCJ16YYF663QDC
@jdx jdx changed the title perf(cli): cache help ordering keys to reduce binary size perf(cli): shrink help sorting without allocating cached keys Sep 7, 2026
@jdx
jdx enabled auto-merge (squash) September 7, 2026 04:48
@jdx
jdx merged commit 874a5ff into main Sep 7, 2026
11 checks passed
@jdx
jdx deleted the codex/help-ordering-size branch September 7, 2026 04:56
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Instruction counts

The comparison never ran — an earlier step failed.

603706d5d73b vs `` · measured on the runner, not pushed to the history.

jdx pushed a commit that referenced this pull request Sep 12, 2026
### 🚀 Features

- **(help)** allow remapping semantic colours on Style by
[@lu-zero](https://github.com/lu-zero) in
[#1414](#1414)
- **(parse)** add opt-in default-subcommand flag routing by
[@jdx](https://github.com/jdx) in
[#1413](#1413)
- **(parse)** parse Args without an enclosing CLI by
[@jdx](https://github.com/jdx) in
[#1419](#1419)

### 🐛 Bug Fixes

- **(bash)** preserve colon-prefixed completion words by
[@jdx](https://github.com/jdx) in
[#1405](#1405)

### 📚 Documentation

- write PR titles and descriptions for release notes by
[@jdx](https://github.com/jdx) in
[#1415](#1415)

### ⚡ Performance

- **(cli)** shrink help sorting without allocating cached keys by
[@jdx](https://github.com/jdx) in
[#1396](#1396)
- **(cli)** make advanced help and runtime spec serialization optional
by [@jdx](https://github.com/jdx) in
[#1399](#1399)
- **(cli)** share help sorting and skip unused rendering work by
[@jdx](https://github.com/jdx) in
[#1400](#1400)
- **(cli)** avoid color analysis for plain help by
[@jdx](https://github.com/jdx) in
[#1401](#1401)

### 🔍 Other Changes

- **(ci)** use self-repository workflow references by
[@jdx](https://github.com/jdx) in
[#1409](#1409)

### 📦️ Dependency Updates

- lock file maintenance by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#1398](#1398)
- update jdx/renovate-config digest to 8cabc2e by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#1406](#1406)
- update zizmorcore/zizmor-action action to v0.6.3 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#1403](#1403)
- update dependency go to v1.27.1 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#1407](#1407)
- update actions/deploy-pages action to v5 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#1404](#1404)
- update communique to v1.4.0 by [@jdx](https://github.com/jdx) in
[#1416](#1416)

### New Contributors

- @lu-zero made their first contribution in
[#1414](#1414)
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