From 88c32ead70a748a4834065cba9d52c7c3ecadf80 Mon Sep 17 00:00:00 2001 From: Kevin Montrose Date: Thu, 19 Oct 2023 11:27:18 -0400 Subject: [PATCH 1/2] switch RedisKey prefix to be a ReadOnlyMemory and use that to avoid needing to copy memories into arrays when dealing with RedisKeys; expands tests to cover all the new prepend/append possibilities; address an oversite(?) in PhysicalConnection, and adapt to use memory instead of byte[] --- .../KeyspaceIsolation/KeyPrefixed.cs | 4 +- src/StackExchange.Redis/PhysicalConnection.cs | 27 ++- .../PublicAPI/PublicAPI.Unshipped.txt | 3 + src/StackExchange.Redis/RedisKey.cs | 226 ++++++++++++++++-- tests/StackExchange.Redis.Tests/KeyTests.cs | 93 ++++++- 5 files changed, 316 insertions(+), 37 deletions(-) diff --git a/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs b/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs index e34cad895..7adb1cfa2 100644 --- a/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs +++ b/src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs @@ -3,6 +3,7 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Net; +using System.Runtime.InteropServices; using System.Threading.Tasks; namespace StackExchange.Redis.KeyspaceIsolation @@ -843,7 +844,8 @@ protected RedisValue SortGetToInner(RedisValue outer) => protected RedisChannel ToInner(RedisChannel outer) { - var combined = RedisKey.ConcatenateBytes(Prefix, null, (byte[]?)outer); + var combined = RedisKey.ConcatenateBytesArray(Prefix, null, (byte[]?)outer); + return new RedisChannel(combined, outer.IsPattern ? RedisChannel.PatternMode.Pattern : RedisChannel.PatternMode.Literal); } diff --git a/src/StackExchange.Redis/PhysicalConnection.cs b/src/StackExchange.Redis/PhysicalConnection.cs index 22c9d2894..599f8cb6d 100644 --- a/src/StackExchange.Redis/PhysicalConnection.cs +++ b/src/StackExchange.Redis/PhysicalConnection.cs @@ -1148,19 +1148,30 @@ internal static byte ToHexNibble(int value) return value < 10 ? (byte)('0' + value) : (byte)('a' - 10 + value); } - internal static void WriteUnifiedPrefixedString(PipeWriter writer, byte[]? prefix, string? value) + internal static void WriteUnifiedPrefixedString(PipeWriter writer, ReadOnlyMemory prefix, string? value) { - if (value == null) + if (prefix.IsEmpty && value == null) { // special case writer.Write(NullBulkString.Span); } + else if (value == null) + { + int totalLength = prefix.Length; + + var span = writer.GetSpan(3 + Format.MaxInt32TextLen); + span[0] = (byte)'$'; + int bytes = WriteRaw(span, totalLength, offset: 1); + writer.Advance(bytes); + + writer.Write(prefix.Span); + } else { // ${total-len}\r\n 3 + MaxInt32TextLen // {prefix}{value}\r\n int encodedLength = Encoding.UTF8.GetByteCount(value), - prefixLength = prefix?.Length ?? 0, + prefixLength = prefix.Length, totalLength = prefixLength + encodedLength; if (totalLength == 0) @@ -1175,7 +1186,7 @@ internal static void WriteUnifiedPrefixedString(PipeWriter writer, byte[]? prefi int bytes = WriteRaw(span, totalLength, offset: 1); writer.Advance(bytes); - if (prefixLength != 0) writer.Write(prefix); + if (prefixLength != 0) writer.Write(prefix.Span); if (encodedLength != 0) WriteRaw(writer, value, encodedLength); WriteCrlf(writer); } @@ -1251,11 +1262,11 @@ internal static unsafe void WriteRaw(PipeWriter writer, string value, int expect } } - private static void WriteUnifiedPrefixedBlob(PipeWriter writer, byte[]? prefix, byte[]? value) + private static void WriteUnifiedPrefixedBlob(PipeWriter writer, ReadOnlyMemory prefix, byte[]? value) { // ${total-len}\r\n // {prefix}{value}\r\n - if (prefix == null || prefix.Length == 0 || value == null) + if (prefix.Length == 0 || value == null) { // if no prefix, just use the non-prefixed version; // even if prefixed, a null value writes as null, so can use the non-prefixed version WriteUnifiedBlob(writer, value); @@ -1264,10 +1275,10 @@ private static void WriteUnifiedPrefixedBlob(PipeWriter writer, byte[]? prefix, { var span = writer.GetSpan(3 + Format.MaxInt32TextLen); // note even with 2 max-len, we're still in same text range span[0] = (byte)'$'; - int bytes = WriteRaw(span, prefix.LongLength + value.LongLength, offset: 1); + int bytes = WriteRaw(span, prefix.Length + value.LongLength, offset: 1); writer.Advance(bytes); - writer.Write(prefix); + writer.Write(prefix.Span); writer.Write(value); span = writer.GetSpan(2); diff --git a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt index eff457070..0532a648c 100644 --- a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt +++ b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt @@ -8,6 +8,7 @@ StackExchange.Redis.IServer.Protocol.get -> StackExchange.Redis.RedisProtocol StackExchange.Redis.RedisFeatures.ClientId.get -> bool StackExchange.Redis.RedisFeatures.Equals(StackExchange.Redis.RedisFeatures other) -> bool StackExchange.Redis.RedisFeatures.Resp3.get -> bool +StackExchange.Redis.RedisKey.RedisKey(System.ReadOnlyMemory key) -> void StackExchange.Redis.RedisProtocol StackExchange.Redis.RedisProtocol.Resp2 = 20000 -> StackExchange.Redis.RedisProtocol StackExchange.Redis.RedisProtocol.Resp3 = 30000 -> StackExchange.Redis.RedisProtocol @@ -25,6 +26,8 @@ StackExchange.Redis.ResultType.Null = 8 -> StackExchange.Redis.ResultType StackExchange.Redis.ResultType.Push = 37 -> StackExchange.Redis.ResultType StackExchange.Redis.ResultType.Set = 21 -> StackExchange.Redis.ResultType StackExchange.Redis.ResultType.VerbatimString = 12 -> StackExchange.Redis.ResultType +static StackExchange.Redis.RedisKey.implicit operator StackExchange.Redis.RedisKey(System.Memory key) -> StackExchange.Redis.RedisKey +static StackExchange.Redis.RedisKey.implicit operator StackExchange.Redis.RedisKey(System.ReadOnlyMemory key) -> StackExchange.Redis.RedisKey static StackExchange.Redis.RedisResult.Create(StackExchange.Redis.RedisResult![]! values, StackExchange.Redis.ResultType resultType) -> StackExchange.Redis.RedisResult! static StackExchange.Redis.RedisResult.Create(StackExchange.Redis.RedisValue[]! values, StackExchange.Redis.ResultType resultType) -> StackExchange.Redis.RedisResult! virtual StackExchange.Redis.RedisResult.Length.get -> int diff --git a/src/StackExchange.Redis/RedisKey.cs b/src/StackExchange.Redis/RedisKey.cs index 28378d116..b4a8b199c 100644 --- a/src/StackExchange.Redis/RedisKey.cs +++ b/src/StackExchange.Redis/RedisKey.cs @@ -2,6 +2,7 @@ using System.Buffers; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.Runtime.InteropServices; using System.Text; namespace StackExchange.Redis @@ -11,9 +12,9 @@ namespace StackExchange.Redis /// public readonly struct RedisKey : IEquatable { - internal RedisKey(byte[]? keyPrefix, object? keyValue) + internal RedisKey(ReadOnlyMemory keyPrefix, object? keyValue) { - KeyPrefix = keyPrefix?.Length == 0 ? null : keyPrefix; + KeyPrefix = keyPrefix; KeyValue = keyValue; } @@ -22,9 +23,14 @@ internal RedisKey(byte[]? keyPrefix, object? keyValue) /// public RedisKey(string? key) : this(null, key) { } + /// + /// Creates a from a Memory<byte>. + /// + public RedisKey(ReadOnlyMemory key) : this(key, null) { } + internal RedisKey AsPrefix() => new RedisKey((byte[]?)this, null); - internal bool IsNull => KeyPrefix == null && KeyValue == null; + internal bool IsNull => KeyPrefix.IsEmpty && KeyValue == null; internal static RedisKey Null { get; } = new RedisKey(null, null); @@ -32,14 +38,24 @@ internal bool IsEmpty { get { - if (KeyPrefix != null) return false; + if (!KeyPrefix.IsEmpty) return false; if (KeyValue == null) return true; if (KeyValue is string s) return s.Length == 0; return ((byte[])KeyValue).Length == 0; } } - internal byte[]? KeyPrefix { get; } + /// + /// This is used for either prefixes OR memory values. + /// + /// This lets us handle a reasonable common case (concating values together) + /// without allocating, and also lets folks avoid copying into an array + /// if they've already packed things into a memory themselves. + /// + /// The hope is that the case of "have a memory prefix, and then concat another memory" + /// is relatively uncommon. + /// + internal ReadOnlyMemory KeyPrefix { get; } internal object? KeyValue { get; } /// @@ -221,7 +237,19 @@ public override int GetHashCode() internal RedisValue AsRedisValue() { - if (KeyPrefix == null && KeyValue is string keyString) return keyString; + if (KeyPrefix.IsEmpty) + { + if (KeyValue is string keyString) return keyString; + } + else + { + if (KeyValue is null) + { + return KeyPrefix; + } + } + + // gotta actually build an array return (byte[]?)this; } @@ -249,11 +277,28 @@ public static implicit operator RedisKey(byte[]? key) return new RedisKey(null, key); } + /// + /// Create a from a . + /// + /// The Memory<byte> to get a key from. + public static implicit operator RedisKey(Memory key) + => (ReadOnlyMemory)key; + + /// + /// Create a from a . + /// + /// The ReadOnlyMemory<byte> to get a key from. + public static implicit operator RedisKey(ReadOnlyMemory key) + { + if (key.IsEmpty) return default; + return new RedisKey(key, null); + } + /// /// Obtain the as a . /// /// The key to get a byte array for. - public static implicit operator byte[]? (RedisKey key) + public static implicit operator byte[]?(RedisKey key) { if (key.IsNull) return null; if (key.TryGetSimpleBuffer(out var arr)) return arr; @@ -270,9 +315,9 @@ public static implicit operator RedisKey(byte[]? key) /// Obtain the key as a . /// /// The key to get a string for. - public static implicit operator string? (RedisKey key) + public static implicit operator string?(RedisKey key) { - if (key.KeyPrefix is null) + if (key.KeyPrefix.IsEmpty) { return key.KeyValue switch { @@ -314,20 +359,153 @@ public static implicit operator RedisKey(byte[]? key) public static RedisKey operator +(RedisKey x, RedisKey y) => new RedisKey(ConcatenateBytes(x.KeyPrefix, x.KeyValue, y.KeyPrefix), y.KeyValue); - internal static RedisKey WithPrefix(byte[]? prefix, RedisKey value) + internal static RedisKey WithPrefix(RedisKey prefix, RedisKey suffix) + { + // trivial no-alloc cases + if (prefix.IsEmpty) return suffix; + if (suffix.IsEmpty) return prefix; + + var prefixHasPrefix = !prefix.KeyPrefix.IsEmpty; + var prefixHasValue = !(prefix.KeyValue == null || (prefix.KeyValue is byte[] pValBs && pValBs.Length == 0)); + + var suffixHasPrefix = !suffix.KeyPrefix.IsEmpty; + var suffixHasValue = !(suffix.KeyValue == null || (suffix.KeyValue is byte[] sValBs && sValBs.Length == 0)); + + var pieceCount = + (prefixHasPrefix ? 1 : 0) + + (prefixHasValue ? 1 : 0) + + (suffixHasPrefix ? 1 : 0) + + (suffixHasValue ? 1 : 0); + + if (pieceCount == 2) + { + // if there's only one part of each value set, we can try to avoid a copy + ReadOnlyMemory newPrefix; + object? newValue; + + if (prefixHasPrefix) + { + newPrefix = prefix.KeyPrefix; + } + else + { + // implies prefixHasValue == true + + if (prefix.KeyValue is byte[] pValB) + { + // if the prefix's value is backed by an array, we can just slide it over into the memory slot + newPrefix = pValB; + } + else if (prefix.KeyValue is string pValS) + { + // always have to transcode strings, unfortunately + newPrefix = Encoding.UTF8.GetBytes(pValS); + } + else + { + // shouldn't happen + newPrefix = default; + } + } + + if (suffixHasPrefix) + { + // if the suffix's prefix is backed by an array, and covers the entire array, we can just reuse the array + if (MemoryMarshal.TryGetArray(suffix.KeyPrefix, out var arrSeg) && arrSeg.Offset == 0 && arrSeg.Array != null && arrSeg.Count == arrSeg.Array.Length) + { + newValue = arrSeg.Array; + } + else + { + // darn, gotta copy the suffix prefix into an array + newValue = suffix.KeyPrefix.ToArray(); + } + } + else + { + // implies suffixHasValue == true; + newValue = suffix.KeyValue; + } + + return new RedisKey(newPrefix, newValue); + } + + // gotta copy _something_ + if(prefixHasPrefix) + { + // but there are a couple cases we can avoid one copy + if(!prefixHasValue) + { + ReadOnlyMemory newPrefix = prefix.KeyPrefix; + byte[]? newValue = (byte[]?)suffix; + + return new RedisKey(newPrefix, newValue); + } + + if (!suffixHasPrefix) + { + ReadOnlyMemory newPrefix = (byte[]?)prefix; + object? newValue = suffix.KeyValue; + + return new RedisKey(newPrefix, newValue); + } + } + else + { + // implies prefixHasValue == true + if(prefix.KeyValue is byte[] prefixValueBytes) + { + // just shift everything over one, basically + ReadOnlyMemory newPrefix = prefixValueBytes; + byte[]? newValue = (byte[]?)suffix; + + return new RedisKey(newPrefix, newValue); + } + } + + // whelp, just copy everything (at least two allocs here) + return new RedisKey((byte[]?)prefix, (byte[]?)suffix); + } + + internal static ReadOnlyMemory ConcatenateBytes(ReadOnlyMemory a, object? b, ReadOnlyMemory c) { - if (prefix == null || prefix.Length == 0) return value; - if (value.KeyPrefix == null) return new RedisKey(prefix, value.KeyValue); - if (value.KeyValue == null) return new RedisKey(prefix, value.KeyPrefix); - - // two prefixes; darn - byte[] copy = new byte[prefix.Length + value.KeyPrefix.Length]; - Buffer.BlockCopy(prefix, 0, copy, 0, prefix.Length); - Buffer.BlockCopy(value.KeyPrefix, 0, copy, prefix.Length, value.KeyPrefix.Length); - return new RedisKey(copy, value.KeyValue); + if (a.IsEmpty && c.IsEmpty) + { + if (b == null) return null; + if (b is string s) return Encoding.UTF8.GetBytes(s); + return (byte[])b; + } + + int aLen = a.Length, + bLen = b == null ? 0 : (b is string bString + ? Encoding.UTF8.GetByteCount(bString) + : ((byte[])b).Length), + cLen = c.Length; + + var result = new byte[aLen + bLen + cLen]; + if (aLen != 0) + { + a.CopyTo(result); + } + if (bLen != 0) + { + if (b is string s) + { + Encoding.UTF8.GetBytes(s, 0, s.Length, result, aLen); + } + else + { + Buffer.BlockCopy((byte[])b!, 0, result, aLen, bLen); + } + } + if (cLen != 0) + { + c.CopyTo(result.AsMemory().Slice(aLen + bLen, cLen)); + } + return result; } - internal static byte[]? ConcatenateBytes(byte[]? a, object? b, byte[]? c) + internal static byte[]? ConcatenateBytesArray(byte[]? a, object? b, byte[]? c) { if ((a == null || a.Length == 0) && (c == null || c.Length == 0)) { @@ -380,11 +558,11 @@ internal static RedisKey WithPrefix(byte[]? prefix, RedisKey value) internal bool TryGetSimpleBuffer([NotNullWhen(true)] out byte[]? arr) { arr = KeyValue is null ? Array.Empty() : KeyValue as byte[]; - return arr is not null && (KeyPrefix is null || KeyPrefix.Length == 0); + return arr is not null && KeyPrefix.IsEmpty; } internal int TotalLength() => - (KeyPrefix is null ? 0 : KeyPrefix.Length) + KeyValue switch + (KeyPrefix.Length) + KeyValue switch { null => 0, string s => Encoding.UTF8.GetByteCount(s), @@ -394,9 +572,9 @@ internal int TotalLength() => internal int CopyTo(Span destination) { int written = 0; - if (KeyPrefix is not null && KeyPrefix.Length != 0) + if (!KeyPrefix.IsEmpty) { - KeyPrefix.CopyTo(destination); + KeyPrefix.Span.CopyTo(destination); written += KeyPrefix.Length; destination = destination.Slice(KeyPrefix.Length); } diff --git a/tests/StackExchange.Redis.Tests/KeyTests.cs b/tests/StackExchange.Redis.Tests/KeyTests.cs index b0c028d56..4ef7f92f5 100644 --- a/tests/StackExchange.Redis.Tests/KeyTests.cs +++ b/tests/StackExchange.Redis.Tests/KeyTests.cs @@ -89,8 +89,6 @@ public void PrependAppend() RedisKey key1 = "world"; RedisKey key2 = Encoding.UTF8.GetBytes("hello"); var key3 = key1.Prepend(key2); - Assert.True(ReferenceEquals(key1.KeyValue, key3.KeyValue)); - Assert.True(ReferenceEquals(key2.KeyValue, key3.KeyPrefix)); Assert.Equal("helloworld", key3); } @@ -104,10 +102,97 @@ public void PrependAppend() RedisKey key1 = Encoding.UTF8.GetBytes("hello"); RedisKey key2 = "world"; var key3 = key1.Append(key2); - Assert.True(ReferenceEquals(key2.KeyValue, key3.KeyValue)); - Assert.True(ReferenceEquals(key1.KeyValue, key3.KeyPrefix)); Assert.Equal("helloworld", key3); } + + { + RedisKey key1 = Encoding.UTF8.GetBytes("hello"); + var key2 = key1.Append(RedisKey.Null); + Assert.Equal("hello", key2); + } + + { + RedisKey key1 = Encoding.UTF8.GetBytes("hello"); + var key2 = RedisKey.Null.Append(key1); + Assert.Equal("hello", key2); + } + + { + RedisKey key1 = (ReadOnlyMemory)Encoding.UTF8.GetBytes("hello"); + RedisKey key2 = "world"; + var key3 = key1.Append(key2); + Assert.Equal("helloworld", key3); + } + + { + RedisKey key1 = (ReadOnlyMemory)Encoding.UTF8.GetBytes("hello"); + RedisKey key2 = "world"; + var key3 = key2.Append(key1); + Assert.Equal("worldhello", key3); + } + + { + var mem = (ReadOnlyMemory)Encoding.UTF8.GetBytes("abcdhello"); + RedisKey key1 = mem.Slice(4); + RedisKey key2 = "world"; + var key3 = key1.Append(key2); + Assert.Equal("helloworld", key3); + } + + { + var mem1 = (ReadOnlyMemory)Encoding.UTF8.GetBytes("abcdhello"); + RedisKey key1 = mem1.Slice(4); + + var mem2 = (ReadOnlyMemory)Encoding.UTF8.GetBytes("worldefgh"); + RedisKey key2 = mem2.Slice(0, 5); + var key3 = key1.Append(key2); + Assert.Equal("helloworld", key3); + } + + { + var mem1 = (ReadOnlyMemory)Encoding.UTF8.GetBytes("abcdhello"); + RedisKey key1 = mem1.Slice(4); + + var mem2 = (ReadOnlyMemory)Encoding.UTF8.GetBytes("worldefgh"); + RedisKey key2 = mem2.Slice(0, 5); + var key3 = key2.Append("fizz"); + + var key4 = key1.Append(key3); + Assert.Equal("helloworldfizz", key4); + } + + { + var mem1 = (ReadOnlyMemory)Encoding.UTF8.GetBytes("abcdhello"); + RedisKey key1 = mem1.Slice(4); + var key2 = key1.Append("world"); + + RedisKey key3 = "fizz"; + RedisKey key4 = key2.Append(key3); + + Assert.Equal("helloworldfizz", key4); + } + + { + RedisKey key1 = new RedisKey(default, Encoding.UTF8.GetBytes("hello")); + + var mem2 = (ReadOnlyMemory)Encoding.UTF8.GetBytes("worldefgh"); + RedisKey key2 = mem2.Slice(0, 5); + var key3 = key2.Append("fizz"); + + var key4 = key1.Append(key3); + Assert.Equal("helloworldfizz", key4); + } + + { + RedisKey key1 = Encoding.UTF8.GetBytes("hello"); + var key2 = key1.Append("world"); + + RedisKey key3 = Encoding.UTF8.GetBytes("fizz"); + var key4 = key3.Append("buzz"); + + var key5 = key2.Append(key4); + Assert.Equal("helloworldfizz", key5); + } } [Fact] From 6b726d87715159e1686e371897e66eb9e8f761f2 Mon Sep 17 00:00:00 2001 From: Kevin Montrose Date: Thu, 19 Oct 2023 11:41:08 -0400 Subject: [PATCH 2/2] actually commit final version of test --- tests/StackExchange.Redis.Tests/KeyTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/StackExchange.Redis.Tests/KeyTests.cs b/tests/StackExchange.Redis.Tests/KeyTests.cs index 4ef7f92f5..4f8481b0b 100644 --- a/tests/StackExchange.Redis.Tests/KeyTests.cs +++ b/tests/StackExchange.Redis.Tests/KeyTests.cs @@ -191,7 +191,7 @@ public void PrependAppend() var key4 = key3.Append("buzz"); var key5 = key2.Append(key4); - Assert.Equal("helloworldfizz", key5); + Assert.Equal("helloworldfizzbuzz", key5); } }