imperugo / imperugo/StackExchange.Redis.Extensions
Add ValueTuple overloads to AddAllAsync and deprecate the Tuple ones
Nobody has claimed this yet.
- Dominant language
- C#
- Stars
- 628
- Forks
- 177
- PR merge metrics
- No merged PRs in 30d
Description
Suggested by @LeaFrock.
Removal of the deprecated overloads is tracked separately in #663 (v14.0).
Proposal
Add (string Key, T Value)[] overloads to the three AddAllAsync bulk methods on IRedisDatabase, and mark the existing Tuple<string, T>[] ones [Obsolete]:
// new — the path forward
Task<bool> AddAllAsync<T>((string Key, T Value)[] items, When when = When.Always, CommandFlags flag = CommandFlags.None);
Task<bool> AddAllAsync<T>((string Key, T Value)[] items, DateTimeOffset expiresAt, When when = When.Always, CommandFlags flag = CommandFlags.None);
Task<bool> AddAllAsync<T>((string Key, T Value)[] items, TimeSpan expiresAt, When when = When.Always, CommandFlags flag = CommandFlags.None);
// existing — deprecated, removed in v14 (#663)
[Obsolete("Use the (string Key, T Value)[] overload. This overload will be removed in v14.", DiagnosticId = "SRE0001")]
Task<bool> AddAllAsync<T>(Tuple<string, T>[] items, When when = When.Always, CommandFlags flag = CommandFlags.None);
// ...same for the two expiry overloads
This ships in v13.5 as a fully backward-compatible minor: existing code keeps compiling and binding to the Tuple overloads, with a warning pointing at the replacement.
Rationale
Although both are arrays, the memory layout is fundamentally different. Tuple<string, T>[] is an array of references pointing at separately allocated heap objects: for N items that is N individual allocations (24+ bytes of object header and fields each on 64-bit) plus the reference array, scattered across the heap and each requiring a pointer dereference to read. (string, T)[] is a single contiguous block of values — one allocation total, sequential in memory, far friendlier to both the GC and the CPU cache.
This matters specifically here because AddAllAsync is the bulk-write API: it is the method most likely to be called with large batches, which is exactly where per-element allocation overhead compounds.
Named elements (Key, Value) also read better at the call-site than Item1/Item2.
Implementation notes
The two overload sets do not collide. Tuple<string,T>[] and (string,T)[] are distinct types with no implicit conversion between them, so existing call-sites keep resolving to the deprecated overloads with no ambiguity error.
TreatWarningsAsErrors is true in this repo, so [Obsolete] breaks our own build. There are 23 internal call-sites — 5 in src/, 18 in the test project. The ones in src/ should be migrated to the new overloads. The tests need to keep exercising the deprecated path for as long as we still ship it, so those call-sites need a localised #pragma warning disable CS0618 with a comment explaining that it is intentional until #663.
Mark both the interface and the implementation. An [Obsolete] on IRedisDatabase alone is invisible to anyone typing their variable as the concrete RedisDatabase.
Use DiagnosticId. A dedicated id (e.g. SRE0001) lets consumers suppress this specific deprecation instead of blanket-disabling CS0618 across their project, which would also hide unrelated deprecations from other libraries.
Do not make the new path pay for the old one. The internal helper ValueLengthExtensions.ToRedisEntries<T> currently takes Tuple<string, T>[]. The tempting shortcut is to have the deprecated overloads convert their array to ValueTuple and delegate, but that adds an allocation on exactly the bulk API we are trying to optimise. It is a ten-line internal helper — duplicate it for the two input types so each overload does a single pass.
Scope
IRedisDatabase.cs— 3 new overload declarations, 3 existing ones marked[Obsolete]RedisDatabase.cs— 3 new implementations, 3 existing ones marked[Obsolete]ValueLengthExtensions.cs—ToRedisEntries<T>overload for(string, T)[]- Tests — cover the new overloads; keep the existing cases with scoped
CS0618suppressions - Documentation —
doc/,llms.txt,llms-full.txt, plus a deprecations section in a newdoc/migration-v13-to-v14.mdso 13.5 users already know what is going away
Acceptance criteria
-
ValueTupleoverloads added and covered by tests -
Tupleoverloads marked[Obsolete]withDiagnosticId, on both interface and implementation - Internal call-sites in
src/migrated to the new overloads - Test call-sites for the deprecated path suppressed locally with an explanatory comment
- Build clean with
TreatWarningsAsErrorson all TFMs - Deprecation documented for consumers upgrading to 13.5
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with IRedisDatabase.cs and RedisDatabase.cs to compare the three existing AddAllAsync overloads, then inspect ValueLengthExtensions.cs and the current tests. Add and test the ValueTuple overloads, deprecate both Tuple paths on the interface and implementation, migrate internal call-sites, and update the listed documentation. Verify the build remains clean with TreatWarningsAsErrors across all TFMs.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp, redis
- Domain
- backend, databases, documentation, testing
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100