Repository navigation
Avoid unnecessary allocations in ParseInfo / ParseCategorizedInfo in RedisDatabase #669
Description
Observations
ParseCategorizedInfoallocates aList<InfoDetail>, only to immediately materialize it into anInfoDetail[].ParseInfothen further projects that array into aDictionary<string, string>.EnumerateLinesis currently an extension method, but is only used by this parsing path.
Proposal
- Remove the
EnumerateLinesextension method and move its logic intoInfoDetail. - Add an internal static factory method such as:
internal static List<InfoDetail> Parse(ReadOnlySpan<char> info)
- Allow callers to project directly from the
List<InfoDetail>:- Into an array (
InfoDetail[]) where needed - Or into a
Dictionary<string, string>forParseInfo
- Into an array (
This would:
- Eliminate one intermediate array allocation
- Keep parsing logic co-located with
InfoDetail - Reduce public surface area (even if just
internal) - Make allocation patterns clearer and easier to reason about
If desired, we could also consider a Dictionary-backed fast-path once the list is built, but even the structural cleanup alone seems worthwhile.
I want to handle this after #664 gets merged when the SpanExtensions.cs can be deleted then.
More...
- Would it be better if
InfoDetailbecomes a read-only struct? - Add
Task<InfoDetail[]> GetInfoSectionAsync(string section)as Redis supportsINFO [section], for example,redis.call('INFO', 'memory').
Activity
imperugo commentedon Aug 7, 2026
Good issue — the core of it is straightforward and I'd take it. One part needs splitting off, though.
The struct question is a breaking change
InfoDetailispublic, and it's exposed throughIRedisDatabase:Turning a public class into a struct swaps reference semantics for value semantics on a type consumers already hold: null checks and
InfoDetail x = nullstop compiling, reference equality changes meaning, and it's a binary break on top of the source break. So that one belongs in v14, alongside #663 and theServerIteratorFactoryremoval — not here.Worth weighing on its own merits too, separately from the versioning.
InfoDetailcarries three strings, so as a struct it's 24 bytes copied on every pass rather than 8 for the reference, and it's handed out as an array from an admin-path call that just parsed a multi-kilobyteINFOpayload. The parsing dominates either way. I'm not against it, but I'd want a reason beyond symmetry before spending a breaking change on it.The rest is fine for v13.5
Moving the parse into
InfoDetail, dropping the intermediate array, and shrinking the surface are all internal — nothing observable to consumers. Same forGetInfoSectionAsync(string section): purely additive, and genuinely useful givenINFO memoryalone is a fraction of the full payload we currently parse and discard.Your sequencing is right, and I checked:
SpanExtensions.cscurrently holds bothEnumerateLinesandToFastArray(ReadOnlySpan<T>). #664 moves the latter toGenericsExtensions, which leavesEnumerateLinesas the only occupant — so once this issue relocates it intoInfoDetail, the file goes away entirely. Clean chain.I've put this on v13.5 with the struct question carved out; open a separate issue for that against v14 whenever you want to make the case.
imperugo commentedon Aug 14, 2026
Recording @LeaFrock's case for
InfoDetailas areadonly struct, from the discussion on #670, so it isn't lost when the v14 conversation happens.His two arguments:
The counterweights, for whoever picks this up: it stays a source-and-binary breaking change on a type exposed through
IRedisDatabase.GetInfoCategorizedAsync()— null checks againstInfoDetailstop compiling and reference equality changes meaning. And 24 bytes get copied on every element access, so code doingresponse.Single(x => x.Key == ...)over a largeINFOpayload pays for it.Not a decision for v13.5 either way. Worth its own issue against v14.0 — Breaking Changes when someone wants to make the call.