Skip to content

refactor(ui/details): single-allocation hex render for STUN Transaction ID#328

Open
Pablosinyores wants to merge 1 commit into
domcyrus:mainfrom
Pablosinyores:refactor/stun-txn-id-single-alloc-hex
Open

refactor(ui/details): single-allocation hex render for STUN Transaction ID#328
Pablosinyores wants to merge 1 commit into
domcyrus:mainfrom
Pablosinyores:refactor/stun-txn-id-single-alloc-hex

Conversation

@Pablosinyores
Copy link
Copy Markdown

Summary

Detail panel rendered the STUN Transaction-ID display string with

info.transaction_id.iter().map(|b| format!("{:02x}", b)).collect::<String>()

which yields 13 heap allocations per render: one short `String` per byte across the 12-byte RFC 5389 transaction ID, plus the final `String` into which they are collected (`src/ui/tabs/details.rs:867`).

Rewrite to a single `String::with_capacity(bytes.len() * 2)` + `write!` per byte:

  • One allocation, sized exactly (2 hex chars per byte = 24).
  • `write!`-to-`String` is infallible; `Result` discarded via `let _`.
  • Same observable hex output (lowercase, zero-padded, no separator).

Why this shape

Matches the merged single-alloc hex pattern from

575ed75 refactor(dpi/bittorrent): single-allocation hex_encode for info-hash render (#307)

applied to the parallel UI render path. `info.transaction_id` is `[u8; 12]` so the capacity hint is exact.

Test plan

  • `cargo fmt` clean
  • `cargo clippy --all-targets -- -D warnings` clean
  • `cargo test --lib` 365/365 (including `ui::snapshot_tests::*`)

No behavior, layout, or copy change.

…on ID

Detail panel built the STUN Transaction-ID display string with

    info.transaction_id.iter().map(|b| format!("{:02x}", b)).collect::<String>()

That yields 13 heap allocations per render: one short String per byte
across the 12-byte RFC 5389 transaction ID, plus the final String into
which they're collected.

Rewrite to `String::with_capacity(bytes.len() * 2)` + `write!` per byte:
- One allocation, sized exactly (2 hex chars per byte).
- write!-to-String is infallible; result discarded with `let _`.
- Same observable hex output (lowercase, zero-padded, no separators).

Matches the merged single-alloc hex pattern from
575ed75 refactor(dpi/bittorrent): single-allocation hex_encode for
info-hash render (domcyrus#307), applied to the parallel UI render path.

No behavior, layout, or copy change. cargo test --lib 365/365.
@domcyrus
Copy link
Copy Markdown
Owner

Hi @Pablosinyores, thanks for the PR and welcome to RustNet!

The change is correct and clean: the capacity hint is exact, the import is right, and the output is identical to before.

Two pointers from CONTRIBUTING.md on how we weigh changes like this:

  1. Performance & Optimizations. A perf-motivated PR should show the code is on a real hot path, ideally a flamegraph under real traffic (see PROFILING.md). The detail panel render only runs for the single selected connection while a STUN connection is open, so it is a cold path. The guideline notes these micro-optimizations may be closed with thanks, since the gain is not meaningful.

  2. Issue first for refactors. Non-trivial refactors should start with an issue so we can sanity-check fit before you invest time. (Typo, docs, and small bug fixes are exempt.)

None of this is a knock on the code. If you want to dig into allocation work, profiling with PROFILING.md to find a genuinely hot path is the best place to start. Thanks again, and please keep them coming!

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