From: N0D4N <50947930+N0D4N@users.noreply.github.com> Date: Tue, 28 Sep 2021 15:29:58 +0000 (+0300) Subject: Reduce allocations in VersionConverter (#55564) X-Git-Tag: accepted/tizen/unified/riscv/20231226.055536~12940 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=cb78a8999ce56dbac7839866d1167c6bb6841adc;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Reduce allocations in VersionConverter (#55564) * Add more test cases for VersionConverter * Reduce allocations in VersionConverter.Read method * Remove allocations from VersionConverter.Write method * Parse Version from chars, instead of parsing individual components. Assert successful formatting on write. * Allow escaping, disallow leading and trailing whitespaces * Hopefully fix tests for .NET Framework, add few more test cases that should fail * Assume whitespaces can be only on the start, or only at the end, add one more test case that should fail * Remove redundant return statements * Elaborate on comment regarding leading and trailing whitespaces. Copy it to .NetStandard2.0 target. * Add another test case. --- diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx index 6485157..f76179e 100644 --- a/src/libraries/System.Text.Json/src/Resources/Strings.resx +++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx @@ -1,17 +1,17 @@ - @@ -324,6 +324,9 @@ The JSON value is not in a supported Guid format. + + The JSON value is not in a supported Version format. + '{0}' is an invalid start of a property name or value, after a comment. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/VersionConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/VersionConverter.cs index 64f4986..f4bd6df 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/VersionConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/VersionConverter.cs @@ -1,25 +1,115 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Buffers; +using System.Buffers.Text; +using System.Diagnostics; + namespace System.Text.Json.Serialization.Converters { internal sealed class VersionConverter : JsonConverter { +#if BUILDING_INBOX_LIBRARY + private const int MinimumVersionLength = 3; // 0.0 + + private const int MaximumVersionLength = 43; // 2147483647.2147483647.2147483647.2147483647 + + private const int MaximumEscapedVersionLength = JsonConstants.MaxExpansionFactorWhileEscaping * MaximumVersionLength; +#endif + public override Version Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { + if (reader.TokenType != JsonTokenType.String) + { + throw ThrowHelper.GetInvalidOperationException_ExpectedString(reader.TokenType); + } + +#if BUILDING_INBOX_LIBRARY + bool isEscaped = reader._stringHasEscaping; + + int maxLength = isEscaped ? MaximumEscapedVersionLength : MaximumVersionLength; + ReadOnlySpan source = stackalloc byte[0]; + if (reader.HasValueSequence) + { + if (!JsonHelpers.IsInRangeInclusive(reader.ValueSequence.Length, MinimumVersionLength, maxLength)) + { + throw ThrowHelper.GetFormatException(DataType.Version); + } + + Span stackSpan = stackalloc byte[isEscaped ? MaximumEscapedVersionLength : MaximumVersionLength]; + reader.ValueSequence.CopyTo(stackSpan); + source = stackSpan.Slice(0, (int)reader.ValueSequence.Length); + } + else + { + source = reader.ValueSpan; + + if (!JsonHelpers.IsInRangeInclusive(source.Length, MinimumVersionLength, maxLength)) + { + throw ThrowHelper.GetFormatException(DataType.Version); + } + } + + if (isEscaped) + { + int backslash = source.IndexOf(JsonConstants.BackSlash); + Debug.Assert(backslash != -1); + + Span sourceUnescaped = stackalloc byte[MaximumEscapedVersionLength]; + + JsonReaderHelper.Unescape(source, sourceUnescaped, backslash, out int written); + Debug.Assert(written > 0); + + source = sourceUnescaped.Slice(0, written); + Debug.Assert(!source.IsEmpty); + } + + byte firstChar = source[0]; + byte lastChar = source[source.Length - 1]; + if (!JsonHelpers.IsDigit(firstChar) || !JsonHelpers.IsDigit(lastChar)) + { + // Since leading and trailing whitespaces are forbidden throughout System.Text.Json converters + // we need to make sure that our input doesn't have them, + // and if it has - we need to throw, to match behaviour of other converters + // since Version.TryParse allows them and silently parses input to Version + throw ThrowHelper.GetFormatException(DataType.Version); + } + + Span charBuffer = stackalloc char[MaximumVersionLength]; + int writtenChars = JsonReaderHelper.s_utf8Encoding.GetChars(source, charBuffer); + if (Version.TryParse(charBuffer.Slice(0, writtenChars), out Version? result)) + { + return result; + } +#else string? versionString = reader.GetString(); + if (!string.IsNullOrEmpty(versionString) && (!char.IsDigit(versionString[0]) || !char.IsDigit(versionString[versionString.Length - 1]))) + { + // Since leading and trailing whitespaces are forbidden throughout System.Text.Json converters + // we need to make sure that our input doesn't have them, + // and if it has - we need to throw, to match behaviour of other converters + // since Version.TryParse allows them and silently parses input to Version + throw ThrowHelper.GetFormatException(DataType.Version); + } if (Version.TryParse(versionString, out Version? result)) { return result; } - +#endif ThrowHelper.ThrowJsonException(); return null; } public override void Write(Utf8JsonWriter writer, Version value, JsonSerializerOptions options) { +#if BUILDING_INBOX_LIBRARY + Span span = stackalloc char[MaximumVersionLength]; + bool formattedSuccessfully = value.TryFormat(span, out int charsWritten); + Debug.Assert(formattedSuccessfully && charsWritten >= MinimumVersionLength); + writer.WriteStringValue(span.Slice(0, charsWritten)); +#else writer.WriteStringValue(value.ToString()); +#endif } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs index 96296fa..092588c 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs @@ -645,6 +645,9 @@ namespace System.Text.Json case DataType.Guid: message = SR.FormatGuid; break; + case DataType.Version: + message = SR.FormatVersion; + break; default: Debug.Fail($"The DateType enum value: {dateType} is not part of the switch. Add the appropriate case and exception message."); break; @@ -729,5 +732,6 @@ namespace System.Text.Json TimeSpan, Base64String, Guid, + Version, } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/Value.ReadTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/Value.ReadTests.cs index 3bfb3c9..de711c2 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/Value.ReadTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/Value.ReadTests.cs @@ -309,6 +309,8 @@ namespace System.Text.Json.Serialization.Tests Assert.Throws(() => JsonSerializer.Deserialize(unexpectedString)); Assert.Throws(() => JsonSerializer.Deserialize(unexpectedString)); + Assert.Throws(() => JsonSerializer.Deserialize(unexpectedString)); + Assert.Throws(() => JsonSerializer.Deserialize("1")); Assert.Throws(() => JsonSerializer.Deserialize("1")); @@ -318,24 +320,53 @@ namespace System.Text.Json.Serialization.Tests Assert.Throws(() => JsonSerializer.Deserialize(unexpectedString)); } - [Fact] - public static void ReadVersion() + [Theory] + [InlineData("1.2")] + [InlineData("\\u0031\\u002e\\u0032", "1.2")] + [InlineData("1.2.3")] + [InlineData("\\u0031\\u002e\\u0032\\u002e\\u0033", "1.2.3")] + [InlineData("1.2.3.4")] + [InlineData("\\u0031\\u002e\\u0032\\u002e\\u0033\\u002e\\u0034", "1.2.3.4")] + [InlineData("2147483647.2147483647")] + [InlineData("\\u0032\\u0031\\u0034\\u0037\\u0034\\u0038\\u0033\\u0036\\u0034\\u0037\\u002e\\u0032\\u0031\\u0034\\u0037\\u0034\\u0038\\u0033\\u0036\\u0034\\u0037", + "2147483647.2147483647")] + [InlineData("2147483647.2147483647.2147483647")] + [InlineData("\\u0032\\u0031\\u0034\\u0037\\u0034\\u0038\\u0033\\u0036\\u0034\\u0037\\u002e\\u0032\\u0031\\u0034\\u0037\\u0034\\u0038\\u0033\\u0036\\u0034\\u0037\\u002e\\u0032\\u0031\\u0034\\u0037\\u0034\\u0038\\u0033\\u0036\\u0034\\u0037", + "2147483647.2147483647.2147483647")] + [InlineData("2147483647.2147483647.2147483647.2147483647")] + [InlineData("\\u0032\\u0031\\u0034\\u0037\\u0034\\u0038\\u0033\\u0036\\u0034\\u0037\\u002e\\u0032\\u0031\\u0034\\u0037\\u0034\\u0038\\u0033\\u0036\\u0034\\u0037\\u002e\\u0032\\u0031\\u0034\\u0037\\u0034\\u0038\\u0033\\u0036\\u0034\\u0037\\u002e\\u0032\\u0031\\u0034\\u0037\\u0034\\u0038\\u0033\\u0036\\u0034\\u0037", + "2147483647.2147483647.2147483647.2147483647")] + public static void Version_Read_Success(string json, string? actual = null) { - Version version; - - version = JsonSerializer.Deserialize(@"""1.2"""); - Assert.Equal(new Version(1, 2), version); + actual ??= json; + Version value = JsonSerializer.Deserialize($"\"{json}\""); - version = JsonSerializer.Deserialize(@"""1.2.3"""); - Assert.Equal(new Version(1, 2, 3), version); - - version = JsonSerializer.Deserialize(@"""1.2.3.4"""); - Assert.Equal(new Version(1, 2, 3, 4), version); + Assert.Equal(Version.Parse(actual), value); + } - version = JsonSerializer.Deserialize("null"); - Assert.Null(version); + [Theory] + [InlineData("")] + [InlineData(" ")] + [InlineData(" ")] + [InlineData("2147483648.2147483648.2147483648.2147483648")] //int.MaxValue + 1 + [InlineData("2147483647.2147483647.2147483647.21474836477")] // Slightly bigger in size than max length of Version + [InlineData("-2147483648.-2147483648")] + [InlineData("-2147483648.-2147483648.-2147483648")] + [InlineData("-2147483648.-2147483648.-2147483648.-2147483648")] + [InlineData("1.-1")] + [InlineData("1")] + [InlineData(" 1.2.3.4")] //Valid but has leading whitespace + [InlineData("1.2.3.4 ")] //Valid but has trailing whitespace + [InlineData(" 1.2.3.4 ")] //Valid but has trailing and leading whitespaces + [InlineData("{}", false)] + [InlineData("[]", false)] + [InlineData("true", false)] + public static void Version_Read_Failure(string json, bool addQuotes = true) + { + if (addQuotes) + json = $"\"{json}\""; - Assert.Throws(() => JsonSerializer.Deserialize(@"""""")); + Assert.Throws(() => JsonSerializer.Deserialize(json)); } [Fact] diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/Value.WriteTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/Value.WriteTests.cs index f3e6bd8..4f1121a 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/Value.WriteTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/Value.WriteTests.cs @@ -111,6 +111,21 @@ namespace System.Text.Json.Serialization.Tests Version version = new Version(1, 2, 3, 4); Assert.Equal(@"""1.2.3.4""", JsonSerializer.Serialize(version)); } + + { + Version version = new Version(int.MaxValue, int.MaxValue); + Assert.Equal(@"""2147483647.2147483647""", JsonSerializer.Serialize(version)); + } + + { + Version version = new Version(int.MaxValue, int.MaxValue, int.MaxValue); + Assert.Equal(@"""2147483647.2147483647.2147483647""", JsonSerializer.Serialize(version)); + } + + { + Version version = new Version(int.MaxValue, int.MaxValue, int.MaxValue, int.MaxValue); + Assert.Equal(@"""2147483647.2147483647.2147483647.2147483647""", JsonSerializer.Serialize(version)); + } } [Theory]