From 723cd7d0de581a5252e2ba1f0c40bc960deda51d Mon Sep 17 00:00:00 2001 From: Alex McAuliffe Date: Wed, 31 May 2023 15:18:18 +0100 Subject: [PATCH 1/3] Add ISpanParsable on to GrainId --- src/Orleans.Core.Abstractions/IDs/GrainId.cs | 69 ++++++++++++++++---- 1 file changed, 58 insertions(+), 11 deletions(-) diff --git a/src/Orleans.Core.Abstractions/IDs/GrainId.cs b/src/Orleans.Core.Abstractions/IDs/GrainId.cs index 37b85af27e..a098322b83 100644 --- a/src/Orleans.Core.Abstractions/IDs/GrainId.cs +++ b/src/Orleans.Core.Abstractions/IDs/GrainId.cs @@ -12,7 +12,7 @@ namespace Orleans.Runtime /// [Serializable, GenerateSerializer, Immutable] [JsonConverter(typeof(GrainIdJsonConverter))] - public readonly struct GrainId : IEquatable, IComparable, ISerializable, ISpanFormattable + public readonly struct GrainId : IEquatable, IComparable, ISerializable, ISpanFormattable, ISpanParsable { [Id(0)] private readonly GrainType _type; @@ -64,36 +64,71 @@ private GrainId(SerializationInfo info, StreamingContext context) public static GrainId Create(GrainType type, IdSpan key) => new GrainId(type, key); /// - /// Creates a new instance. + /// Parses a from the span. /// - public static GrainId Parse(string value) + public static GrainId Parse(ReadOnlySpan value, IFormatProvider? provider = null) { - if (!TryParse(value, out var result)) + if (!TryParse(value, provider, out var result)) { ThrowInvalidGrainId(value); - static void ThrowInvalidGrainId(string value) => throw new ArgumentException($"Unable to parse \"{value}\" as a grain id"); + static void ThrowInvalidGrainId(ReadOnlySpan value) => throw new ArgumentException($"Unable to parse \"{value}\" as a grain id"); } return result; } /// - /// Creates a new instance. + /// Tries to parse a from the span. /// - public static bool TryParse(string? value, out GrainId grainId) + /// if a valid was parsed. otherwise + public static bool TryParse(ReadOnlySpan value, IFormatProvider? provider, out GrainId result) { int i; - if (value is null || (i = value.IndexOf('/')) < 0) + if ((i = value.IndexOf('/')) < 0) { - grainId = default; + result = default; return false; } - grainId = new(new GrainType(Encoding.UTF8.GetBytes(value, 0, i)), new IdSpan(Encoding.UTF8.GetBytes(value, i + 1, value.Length - i - 1))); + var typeSpan = value[0..i]; + var type = new byte[Encoding.UTF8.GetByteCount(typeSpan)]; + Encoding.UTF8.GetBytes(typeSpan, type); + + var idSpan = value[(i + 1)..]; + var id = new byte[Encoding.UTF8.GetByteCount(idSpan)]; + Encoding.UTF8.GetBytes(idSpan, id); + + result = new(new GrainType(type), new IdSpan(id)); return true; } + /// + /// Parses a from the string. + /// + public static GrainId Parse(string value) + => Parse(value.AsSpan(), null); + + /// + /// Parses a from the string. + /// + public static GrainId Parse(string value, IFormatProvider? provider = null) + => Parse(value.AsSpan(), provider); + + /// + /// Tries to parse a from the string. + /// + /// if a valid was parsed. otherwise + public static bool TryParse(string? value, out GrainId result) + => TryParse(value.AsSpan(), null, out result); + + /// + /// Tries to parse a from the string. + /// + /// if a valid was parsed. otherwise + public static bool TryParse(string? value, IFormatProvider? provider, out GrainId result) + => TryParse(value.AsSpan(), provider, out result); + /// /// if this instance is the default value, if it is not. /// @@ -167,7 +202,19 @@ bool ISpanFormattable.TryFormat(Span destination, out int charsWritten, Re public sealed class GrainIdJsonConverter : JsonConverter { /// - public override GrainId Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => GrainId.Parse(reader.GetString()!); + public override GrainId Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + var valueLength = reader.HasValueSequence + ? checked((int)reader.ValueSequence.Length) + : reader.ValueSpan.Length; + + Span buf = stackalloc char[valueLength]; + + var written = reader.CopyString(buf); + buf = buf[..written]; + + return GrainId.Parse(buf); + } /// public override void Write(Utf8JsonWriter writer, GrainId value, JsonSerializerOptions options) From 79adcab4d3db80a63abf22a402952b939fbbc013 Mon Sep 17 00:00:00 2001 From: Alex McAuliffe Date: Wed, 2 Aug 2023 14:37:30 +0100 Subject: [PATCH 2/3] Add GrainId tests for ISpanParsable and JsonConverter JsonConverter now uses span parsing path so tests added to verify this works. --- test/NonSilo.Tests/General/Identifiertests.cs | 82 +++++++++++-------- 1 file changed, 47 insertions(+), 35 deletions(-) diff --git a/test/NonSilo.Tests/General/Identifiertests.cs b/test/NonSilo.Tests/General/Identifiertests.cs index 9f94af2c89..9cb9b9db33 100644 --- a/test/NonSilo.Tests/General/Identifiertests.cs +++ b/test/NonSilo.Tests/General/Identifiertests.cs @@ -1,5 +1,6 @@ using System; using System.Net; +using System.Text.Json; using Microsoft.Extensions.DependencyInjection; using Orleans; using Orleans.GrainReferences; @@ -97,45 +98,56 @@ public void GrainIdShouldEncodeAndDecodePrimaryKeyGuidCorrectly() } } - [Fact, TestCategory("SlowBVT"), TestCategory("Identifiers")] - public void GrainId_ToFromPrintableString() + [Theory, TestCategory("SlowBVT"), TestCategory("Identifiers")] + [MemberData(nameof(TestGrainIds))] + public void GrainId_ToFromPrintableString(GrainId grainId) { - Guid guid = Guid.NewGuid(); - GrainId grainId = GrainId.Create(GrainType.Create("test"), GrainIdKeyExtensions.CreateGuidKey(guid)); - GrainId roundTripped = RoundTripGrainIdToParsable(grainId); - Assert.Equal(grainId, roundTripped); // GrainId.ToPrintableString -- Guid key - - string extKey = "Guid-ExtKey-1"; - guid = Guid.NewGuid(); - grainId = GrainId.Create(GrainType.Create("test"), GrainIdKeyExtensions.CreateGuidKey(guid, extKey)); - roundTripped = RoundTripGrainIdToParsable(grainId); - Assert.Equal(grainId, roundTripped); // GrainId.ToPrintableString -- Guid key + Extended Key - - grainId = GrainId.Create(GrainType.Create("test"), GrainIdKeyExtensions.CreateGuidKey(guid, (string)null)); - roundTripped = RoundTripGrainIdToParsable(grainId); - Assert.Equal(grainId, roundTripped); // GrainId.ToPrintableString -- Guid key + null Extended Key - - long key = random.Next(); - grainId = GrainId.Create(GrainType.Create("test"), GrainIdKeyExtensions.CreateIntegerKey(key)); - roundTripped = RoundTripGrainIdToParsable(grainId); - Assert.Equal(grainId, roundTripped); // GrainId.ToPrintableString -- Int64 key - - extKey = "Long-ExtKey-2"; - key = random.Next(); - grainId = GrainId.Create(GrainType.Create("test"), GrainIdKeyExtensions.CreateIntegerKey(key, extKey)); - roundTripped = RoundTripGrainIdToParsable(grainId); - Assert.Equal(grainId, roundTripped); // GrainId.ToPrintableString -- Int64 key + Extended Key - - key = UniqueKey.NewKey(key).PrimaryKeyToLong(); - grainId = GrainId.Create(GrainType.Create("test"), GrainIdKeyExtensions.CreateIntegerKey(key, extKey)); - roundTripped = RoundTripGrainIdToParsable(grainId); - Assert.Equal(grainId, roundTripped); // GrainId.ToPrintableString -- Int64 key + null Extended Key + string str = grainId.ToString(); + var roundTripped = GrainId.Parse(str); + + Assert.Equal(grainId, roundTripped); + } + + [Theory, TestCategory("SlowBVT"), TestCategory("Identifiers")] + [MemberData(nameof(TestGrainIds))] + public void GrainId_TryParseFromPrintableString(GrainId grainId) + { + string str = grainId.ToString(); + var success = GrainId.TryParse(str, out var roundTripped); + + Assert.True(success); + Assert.Equal(grainId, roundTripped); } - private GrainId RoundTripGrainIdToParsable(GrainId input) + [Theory, TestCategory("SlowBVT"), TestCategory("Identifiers")] + [MemberData(nameof(TestGrainIds))] + public void GrainId_RoundTripJsonConverter(GrainId grainId) { - string str = input.ToString(); - return GrainId.Parse(str); + var serialized = JsonSerializer.Serialize(grainId); + var deserialized = JsonSerializer.Deserialize(serialized); + + Assert.Equal(grainId, deserialized); + } + + public static TheoryData TestGrainIds + { + get + { + var td = new TheoryData(); + var grainType = GrainType.Create("test"); + var guid = Guid.NewGuid(); + var integer = Random.Shared.NextInt64(); + + td.Add(GrainId.Create(grainType, GrainIdKeyExtensions.CreateGuidKey(guid))); + td.Add(GrainId.Create(grainType, GrainIdKeyExtensions.CreateGuidKey(guid, "Guid-ExtKey-1"))); + td.Add(GrainId.Create(grainType, GrainIdKeyExtensions.CreateGuidKey(guid, (string)null))); + td.Add(GrainId.Create(grainType, GrainIdKeyExtensions.CreateIntegerKey(integer))); + td.Add(GrainId.Create(grainType, GrainIdKeyExtensions.CreateIntegerKey(integer, "Long-ExtKey-2"))); + td.Add(GrainId.Create(grainType, GrainIdKeyExtensions.CreateIntegerKey(integer, (string)null))); + td.Add(GrainId.Create(grainType, GrainIdKeyExtensions.CreateIntegerKey(UniqueKey.NewKey(integer).PrimaryKeyToLong(), "Long-ExtKey-2"))); + + return td; + } } [Fact, TestCategory("BVT"), TestCategory("Identifiers")] From 922acede30cd731547c4cd947014e3c99500af82 Mon Sep 17 00:00:00 2001 From: Alex McAuliffe Date: Wed, 2 Aug 2023 15:18:40 +0100 Subject: [PATCH 3/3] Use constant in stack allocation Avoids possible stack overflow and speed in the hot path --- src/Orleans.Core.Abstractions/IDs/GrainId.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Orleans.Core.Abstractions/IDs/GrainId.cs b/src/Orleans.Core.Abstractions/IDs/GrainId.cs index a098322b83..b7bb724693 100644 --- a/src/Orleans.Core.Abstractions/IDs/GrainId.cs +++ b/src/Orleans.Core.Abstractions/IDs/GrainId.cs @@ -208,7 +208,9 @@ public override GrainId Read(ref Utf8JsonReader reader, Type typeToConvert, Json ? checked((int)reader.ValueSequence.Length) : reader.ValueSpan.Length; - Span buf = stackalloc char[valueLength]; + Span buf = valueLength <= 128 + ? (stackalloc char[128])[..valueLength] + : new char[valueLength]; var written = reader.CopyString(buf); buf = buf[..written];