From bea92ffe97419cf331c199e9d319b597bf5f9c0f Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Sun, 2 Mar 2025 20:52:40 -0500 Subject: [PATCH 1/2] Address a few small nits noticed while browsing --- .../CommonMethodNameTransforms.cs | 11 ++- .../HeaderDelimitedMessageHandler.cs | 13 +-- src/StreamJsonRpc/Hex.cs | 2 +- src/StreamJsonRpc/JsonRpc.cs | 4 + .../LengthHeaderMessageHandler.cs | 3 +- src/StreamJsonRpc/MessagePackFormatter.cs | 8 +- src/StreamJsonRpc/Protocol/TraceParent.cs | 5 +- src/StreamJsonRpc/ProxyGeneration.cs | 4 +- .../StandardCancellationStrategy.cs | 4 + src/StreamJsonRpc/Utilities.cs | 92 +------------------ 10 files changed, 31 insertions(+), 115 deletions(-) diff --git a/src/StreamJsonRpc/CommonMethodNameTransforms.cs b/src/StreamJsonRpc/CommonMethodNameTransforms.cs index 3fe5f9f14..56b14c159 100644 --- a/src/StreamJsonRpc/CommonMethodNameTransforms.cs +++ b/src/StreamJsonRpc/CommonMethodNameTransforms.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.Text.Json; + namespace StreamJsonRpc; /// @@ -23,7 +25,7 @@ public static Func CamelCase throw new ArgumentNullException(); } - return Utilities.ToCamelCase(name); + return JsonNamingPolicy.CamelCase.ConvertName(name); }; } } @@ -45,7 +47,10 @@ public static Func Prepend(string prefix) { return name => name; } - - return name => prefix + name; + else + { + string localPrefix = prefix; + return name => localPrefix + name; + } } } diff --git a/src/StreamJsonRpc/HeaderDelimitedMessageHandler.cs b/src/StreamJsonRpc/HeaderDelimitedMessageHandler.cs index 31ca9e051..93319e4a8 100644 --- a/src/StreamJsonRpc/HeaderDelimitedMessageHandler.cs +++ b/src/StreamJsonRpc/HeaderDelimitedMessageHandler.cs @@ -314,7 +314,7 @@ unsafe int WriteHeaderText(string value, Span memory) var mediaType = MediaTypeHeaderValue.Parse(contentTypeAsText); if (mediaType.CharSet is not null) { - // The common language server protocol accpets 'utf8' as a valid charset due to an early bug. To maintain backwards compatibility, 'utf8' will be + // The common language server protocol accepts 'utf8' as a valid charset due to an early bug. To maintain backwards compatibility, 'utf8' will be // accepted here so StreamJsonRpc can be used to support remote language servers following common language protocol. if (mediaType.CharSet.Equals("utf8", StringComparison.OrdinalIgnoreCase)) { @@ -338,17 +338,6 @@ unsafe int WriteHeaderText(string value, Span memory) } } - private static bool IsLastFourBytesCrlfCrlf(byte[] buffer, int lastIndex) - { - const byte cr = (byte)'\r'; - const byte lf = (byte)'\n'; - return lastIndex >= 4 - && buffer[lastIndex - 4] == cr - && buffer[lastIndex - 3] == lf - && buffer[lastIndex - 2] == cr - && buffer[lastIndex - 1] == lf; - } - private static int GetContentLength(ReadOnlySequence contentLengthValue) { // Ensure the length is reasonable so we don't blow the stack if we execute the stackalloc path. diff --git a/src/StreamJsonRpc/Hex.cs b/src/StreamJsonRpc/Hex.cs index 71f494e74..0e0ac0c0c 100644 --- a/src/StreamJsonRpc/Hex.cs +++ b/src/StreamJsonRpc/Hex.cs @@ -7,7 +7,7 @@ namespace StreamJsonRpc; internal static class Hex { - private static readonly byte[] HexBytes = new byte[] { (byte)'0', (byte)'1', (byte)'2', (byte)'3', (byte)'4', (byte)'5', (byte)'6', (byte)'7', (byte)'8', (byte)'9', (byte)'a', (byte)'b', (byte)'c', (byte)'d', (byte)'e', (byte)'f' }; + private static readonly byte[] HexBytes = "0123456789abcdef"u8.ToArray(); private static readonly byte[] ReverseHexDigits = BuildReverseHexDigits(); internal static void Encode(ReadOnlySpan src, ref Span dest) diff --git a/src/StreamJsonRpc/JsonRpc.cs b/src/StreamJsonRpc/JsonRpc.cs index e8ab2712a..db9210d97 100644 --- a/src/StreamJsonRpc/JsonRpc.cs +++ b/src/StreamJsonRpc/JsonRpc.cs @@ -2601,10 +2601,14 @@ private async Task HandleRpcAsync(JsonRpcMessage rpc) lock (this.dispatcherMapLock) { +#if NET + this.resultDispatcherMap.Remove(resultOrError.RequestId, out data); +#else if (this.resultDispatcherMap.TryGetValue(resultOrError.RequestId, out data)) { this.resultDispatcherMap.Remove(resultOrError.RequestId); } +#endif } if (this.TraceSource.Switch.ShouldTrace(TraceEventType.Information)) diff --git a/src/StreamJsonRpc/LengthHeaderMessageHandler.cs b/src/StreamJsonRpc/LengthHeaderMessageHandler.cs index 281f0bbbe..adc0ba81f 100644 --- a/src/StreamJsonRpc/LengthHeaderMessageHandler.cs +++ b/src/StreamJsonRpc/LengthHeaderMessageHandler.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Buffers; +using System.Buffers.Binary; using System.IO.Pipelines; using Nerdbank.Streams; using StreamJsonRpc.Protocol; @@ -109,7 +110,7 @@ protected override void Write(JsonRpcMessage content, CancellationToken cancella } // Now go back and fill in the header with the actual content length. - Utilities.Write(this.Writer.GetSpan(sizeof(int)), checked((int)contentSequence.Length)); + BinaryPrimitives.WriteInt32BigEndian(this.Writer.GetSpan(sizeof(int)), checked((int)contentSequence.Length)); this.Writer.Advance(sizeof(int)); contentSequence.CopyTo(this.Writer); diff --git a/src/StreamJsonRpc/MessagePackFormatter.cs b/src/StreamJsonRpc/MessagePackFormatter.cs index ec18355b6..b4c6726e9 100644 --- a/src/StreamJsonRpc/MessagePackFormatter.cs +++ b/src/StreamJsonRpc/MessagePackFormatter.cs @@ -402,13 +402,17 @@ private static void WriteProtocolVersionPropertyAndValue(ref MessagePackWriter w } } - private static void ReadUnknownProperty(ref MessagePackReader reader, ref Dictionary>? topLevelProperties, ReadOnlySpan stringKey) + private static unsafe void ReadUnknownProperty(ref MessagePackReader reader, ref Dictionary>? topLevelProperties, ReadOnlySpan stringKey) { topLevelProperties ??= new Dictionary>(StringComparer.Ordinal); #if NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER string name = Encoding.UTF8.GetString(stringKey); #else - string name = Encoding.UTF8.GetString(stringKey.ToArray()); + string name; + fixed (byte* stringKeyPtr = stringKey) + { + name = Encoding.UTF8.GetString(stringKeyPtr, stringKey.Length); + } #endif topLevelProperties.Add(name, GetSliceForNextToken(ref reader)); } diff --git a/src/StreamJsonRpc/Protocol/TraceParent.cs b/src/StreamJsonRpc/Protocol/TraceParent.cs index b4fe393e2..3d44d15dd 100644 --- a/src/StreamJsonRpc/Protocol/TraceParent.cs +++ b/src/StreamJsonRpc/Protocol/TraceParent.cs @@ -142,10 +142,7 @@ public override string ToString() Debug.Assert(traceParentRemaining.Length == 0, "Characters were not initialized."); - fixed (char* pValue = traceparent) - { - return new string(pValue, 0, traceparent.Length); - } + return traceparent.ToString(); static void AddHyphen(ref Span value) { diff --git a/src/StreamJsonRpc/ProxyGeneration.cs b/src/StreamJsonRpc/ProxyGeneration.cs index 5e49ff42b..c1b5598a4 100644 --- a/src/StreamJsonRpc/ProxyGeneration.cs +++ b/src/StreamJsonRpc/ProxyGeneration.cs @@ -294,8 +294,8 @@ internal static TypeInfo Get(Type contractInterface, ReadOnlySpan addition string rpcMethodName = methodNameMap.GetRpcMethodName(method); if (rpcInterfaceCode.HasValue) { - methodName = rpcInterfaceCode + "." + method.Name; - rpcMethodName = rpcInterfaceCode + "." + rpcMethodName; + methodName = $"{rpcInterfaceCode.GetValueOrDefault()}.{method.Name}"; + rpcMethodName = $"{rpcInterfaceCode.GetValueOrDefault()}.{rpcMethodName}"; } ParameterInfo[] methodParameters = method.GetParameters(); diff --git a/src/StreamJsonRpc/StandardCancellationStrategy.cs b/src/StreamJsonRpc/StandardCancellationStrategy.cs index 859ce423b..10046c03f 100644 --- a/src/StreamJsonRpc/StandardCancellationStrategy.cs +++ b/src/StreamJsonRpc/StandardCancellationStrategy.cs @@ -115,10 +115,14 @@ protected void CancelInboundRequest(RequestId id) CancellationTokenSource? cts; lock (this.inboundCancellationSources) { +#if NET + this.inboundCancellationSources.Remove(id, out cts); +#else if (this.inboundCancellationSources.TryGetValue(id, out cts)) { this.inboundCancellationSources.Remove(id); } +#endif } if (cts is object) diff --git a/src/StreamJsonRpc/Utilities.cs b/src/StreamJsonRpc/Utilities.cs index 6b8c425e3..1afc648d3 100644 --- a/src/StreamJsonRpc/Utilities.cs +++ b/src/StreamJsonRpc/Utilities.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Buffers; +using System.Buffers.Binary; using System.Diagnostics.CodeAnalysis; namespace StreamJsonRpc; @@ -18,39 +19,7 @@ internal static int ReadInt32BE(ReadOnlySequence sequence) sequence = sequence.Slice(0, 4); Span stackSpan = stackalloc byte[4]; sequence.Slice(0, 4).CopyTo(stackSpan); - return ReadIntBE(stackSpan); - } - - /// - /// Reads an value to a buffer using big endian. - /// - /// The buffer to read from. Must be at most 4 bytes long. - /// The read value. - internal static int ReadIntBE(ReadOnlySpan buffer) - { - Requires.Argument(buffer.Length <= 4, nameof(buffer), "Int32 length exceeded."); - - int local = 0; - for (int offset = 0; offset < buffer.Length; offset++) - { - local <<= 8; - local |= buffer[offset]; - } - - return local; - } - - /// - /// Writes an value to a buffer using big endian. - /// - /// The buffer to write to. Must be at least 4 bytes long. - /// The value to write. - internal static void Write(Span buffer, int value) - { - buffer[0] = (byte)(value >> 24); - buffer[1] = (byte)(value >> 16); - buffer[2] = (byte)(value >> 8); - buffer[3] = (byte)value; + return BinaryPrimitives.ReadInt32BigEndian(stackSpan); } /// @@ -68,61 +37,4 @@ internal static void CopyTo(this in ReadOnlySequence sequence, IBufferWrit writer.Write(segment.Span); } } - - /// - /// Converts a PascalCase identifier to camelCase. - /// - /// The identifier to convert to camcelCase. - /// The camelCase identifier. Null if is null. - /// - /// Originally taken from Newtonsoft.Json. - /// - [return: NotNullIfNotNull("identifier")] - internal static string? ToCamelCase(string? identifier) - { - if (identifier is null || identifier.Length == 0 || !char.IsUpper(identifier[0])) - { - return identifier; - } - - char[] chars = ArrayPool.Shared.Rent(identifier.Length); - identifier.CopyTo(0, chars, 0, identifier.Length); - try - { - for (int i = 0; i < identifier.Length; i++) - { - if (i == 1 && !char.IsUpper(chars[i])) - { - break; - } - - bool hasNext = i + 1 < identifier.Length; - if (i > 0 && hasNext && !char.IsUpper(chars[i + 1])) - { - // if the next character is a space, which is not considered uppercase - // (otherwise we wouldn't be here...) - // we want to ensure that the following: - // 'FOO bar' is rewritten as 'foo bar', and not as 'foO bar' - // The code was written in such a way that the first word in uppercase - // ends when if finds an uppercase letter followed by a lowercase letter. - // now a ' ' (space, (char)32) is considered not upper - // but in that case we still want our current character to become lowercase - if (char.IsSeparator(chars[i + 1])) - { - chars[i] = char.ToLowerInvariant(chars[i]); - } - - break; - } - - chars[i] = char.ToLowerInvariant(chars[i]); - } - - return new string(chars, 0, identifier.Length); - } - finally - { - ArrayPool.Shared.Return(chars); - } - } } From 2ef2bd7a4cf91047d449119e7b5f78e9d331dbf8 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Mon, 3 Mar 2025 16:37:21 -0700 Subject: [PATCH 2/2] Revert the camel case function --- .../CommonMethodNameTransforms.cs | 4 +- src/StreamJsonRpc/Utilities.cs | 57 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/StreamJsonRpc/CommonMethodNameTransforms.cs b/src/StreamJsonRpc/CommonMethodNameTransforms.cs index 56b14c159..4794c86f9 100644 --- a/src/StreamJsonRpc/CommonMethodNameTransforms.cs +++ b/src/StreamJsonRpc/CommonMethodNameTransforms.cs @@ -25,7 +25,7 @@ public static Func CamelCase throw new ArgumentNullException(); } - return JsonNamingPolicy.CamelCase.ConvertName(name); + return Utilities.ToCamelCase(name); }; } } @@ -49,6 +49,8 @@ public static Func Prepend(string prefix) } else { + // Using a local variable for the closure avoids C# from allocating the closure + // earlier in the method, which would impact even the fast path. string localPrefix = prefix; return name => localPrefix + name; } diff --git a/src/StreamJsonRpc/Utilities.cs b/src/StreamJsonRpc/Utilities.cs index 1afc648d3..efab39384 100644 --- a/src/StreamJsonRpc/Utilities.cs +++ b/src/StreamJsonRpc/Utilities.cs @@ -37,4 +37,61 @@ internal static void CopyTo(this in ReadOnlySequence sequence, IBufferWrit writer.Write(segment.Span); } } + + /// + /// Converts a PascalCase identifier to camelCase. + /// + /// The identifier to convert to camcelCase. + /// The camelCase identifier. Null if is null. + /// + /// Originally taken from Newtonsoft.Json. + /// + [return: NotNullIfNotNull("identifier")] + internal static string? ToCamelCase(string? identifier) + { + if (identifier is null || identifier.Length == 0 || !char.IsUpper(identifier[0])) + { + return identifier; + } + + char[] chars = ArrayPool.Shared.Rent(identifier.Length); + identifier.CopyTo(0, chars, 0, identifier.Length); + try + { + for (int i = 0; i < identifier.Length; i++) + { + if (i == 1 && !char.IsUpper(chars[i])) + { + break; + } + + bool hasNext = i + 1 < identifier.Length; + if (i > 0 && hasNext && !char.IsUpper(chars[i + 1])) + { + // if the next character is a space, which is not considered uppercase + // (otherwise we wouldn't be here...) + // we want to ensure that the following: + // 'FOO bar' is rewritten as 'foo bar', and not as 'foO bar' + // The code was written in such a way that the first word in uppercase + // ends when if finds an uppercase letter followed by a lowercase letter. + // now a ' ' (space, (char)32) is considered not upper + // but in that case we still want our current character to become lowercase + if (char.IsSeparator(chars[i + 1])) + { + chars[i] = char.ToLowerInvariant(chars[i]); + } + + break; + } + + chars[i] = char.ToLowerInvariant(chars[i]); + } + + return new string(chars, 0, identifier.Length); + } + finally + { + ArrayPool.Shared.Return(chars); + } + } }