perf(filter): match SSH connection state via &'static str, drop Debug allocs#337
Open
obchain wants to merge 1 commit into
Open
perf(filter): match SSH connection state via &'static str, drop Debug allocs#337obchain wants to merge 1 commit into
&'static str, drop Debug allocs#337obchain wants to merge 1 commit into
Conversation
…bug` allocs
`matches_application_filter` was deriving the matchable SSH connection-
state string by `Debug`-formatting `SshConnectionState` and lowercasing
the result — two heap allocations per SSH connection per filter
criterion per render frame:
let state_str = format!("{:?}", info.connection_state).to_lowercase();
`SshConnectionState` is a fieldless 4-variant enum, so its canonical
lower-case form is known at compile time. Replace the runtime derivation
with an exhaustive `match` returning `&'static str` — the compiler now
enforces sync with the enum at compile time, so future variants can't
silently drift out of filter coverage.
Beyond the alloc savings this also fixes a small abstraction smell:
`Debug` output is for diagnostics, not a stable user-facing string, and
shouldn't be the source of truth for a filter token.
Cover each variant with a unit test asserting the matching token (and
non-bleed-through across variants) so the canonical strings stay stable.
Closes domcyrus#336
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
matches_application_filterwas deriving the matchable SSH connection-state string byDebug-formattingSshConnectionStateand lowercasing the result — two heap allocations per SSH connection per filter criterion per render frame:SshConnectionStateis a fieldless 4-variant enum, so its canonical lower-case form is a&'static strknown at compile time. Replace the runtime derivation with an exhaustivematch— the compiler now enforces sync with the enum, so future variants can't silently drift out of filter coverage.Beyond the alloc savings this also fixes a small abstraction smell:
Debugoutput is for diagnostics, not a stable user-facing string, and shouldn't be the source of truth for a filter token.Closes #336.
Behavior
No change. The match arms return the same lower-case strings (
banner,keyexchange,authentication,established) thatformat!("{:?}", …).to_lowercase()produced. A new unit test asserts each variant's canonical token matches the right filter and doesn't bleed across variants.Verification
Test plan
SshConnectionStatevariantcargo fmt --checkcargo clippy --all-targets -- -D warningscargo test --lib