From 226155f9173a6fd2f298989a387dac4a5a5d24f1 Mon Sep 17 00:00:00 2001 From: filzrev <103790468+filzrev@users.noreply.github.com> Date: Tue, 2 Apr 2024 07:04:27 +0900 Subject: [PATCH 1/3] perf: change to use System.Text.Json serializer for XrefMap --- src/Docfx.Build/XRefMaps/XRefMapDownloader.cs | 37 ++++--- .../XRefMaps/XRefMapRedirection.cs | 2 +- .../ObjectToInferredTypesConverter.cs | 77 +++++++++++++ .../System.Text.Json/SystemTextJsonUtility.cs | 101 ++++++++++++++++++ .../XRefMapSerializationTest.cs | 48 +++++++-- test/docfx.Tests/Api.verified.cs | 19 ++-- 6 files changed, 250 insertions(+), 34 deletions(-) create mode 100644 src/Docfx.Common/Json/System.Text.Json/ObjectToInferredTypesConverter.cs create mode 100644 src/Docfx.Common/Json/System.Text.Json/SystemTextJsonUtility.cs diff --git a/src/Docfx.Build/XRefMaps/XRefMapDownloader.cs b/src/Docfx.Build/XRefMaps/XRefMapDownloader.cs index de8c576951b..10e7423c28b 100644 --- a/src/Docfx.Build/XRefMaps/XRefMapDownloader.cs +++ b/src/Docfx.Build/XRefMaps/XRefMapDownloader.cs @@ -10,7 +10,7 @@ namespace Docfx.Build.Engine; -public class XRefMapDownloader +public sealed class XRefMapDownloader { private readonly SemaphoreSlim _semaphore; private readonly IReadOnlyList _localFileFolders; @@ -40,22 +40,22 @@ public XRefMapDownloader(string baseFolder = null, IReadOnlyList fallbac /// The uri of xref map file. /// An instance of . /// This method is thread safe. - public async Task DownloadAsync(Uri uri) + public async Task DownloadAsync(Uri uri, CancellationToken token = default) { ArgumentNullException.ThrowIfNull(uri); - await _semaphore.WaitAsync(); + await _semaphore.WaitAsync(token); return await Task.Run(async () => { try { if (uri.IsAbsoluteUri) { - return await DownloadBySchemeAsync(uri); + return await DownloadBySchemeAsync(uri, token); } else { - return ReadLocalFileWithFallback(uri); + return await ReadLocalFileWithFallback(uri, token); } } finally @@ -65,14 +65,14 @@ public async Task DownloadAsync(Uri uri) }); } - private IXRefContainer ReadLocalFileWithFallback(Uri uri) + private ValueTask ReadLocalFileWithFallback(Uri uri, CancellationToken token = default) { foreach (var localFileFolder in _localFileFolders) { var localFilePath = Path.Combine(localFileFolder, uri.OriginalString); if (File.Exists(localFilePath)) { - return ReadLocalFile(localFilePath); + return ReadLocalFileAsync(localFilePath, token); } } throw new FileNotFoundException($"Cannot find xref map file {uri.OriginalString} in path: {string.Join(",", _localFileFolders)}", uri.OriginalString); @@ -81,17 +81,17 @@ private IXRefContainer ReadLocalFileWithFallback(Uri uri) /// /// Support scheme: http, https, file. /// - protected virtual async Task DownloadBySchemeAsync(Uri uri) + private async ValueTask DownloadBySchemeAsync(Uri uri, CancellationToken token = default) { IXRefContainer result; if (uri.IsFile) { - result = DownloadFromLocal(uri); + result = await DownloadFromLocalAsync(uri, token); } else if (uri.Scheme == Uri.UriSchemeHttp || uri.Scheme == Uri.UriSchemeHttps) { - result = await DownloadFromWebAsync(uri); + result = await DownloadFromWebAsync(uri, token); } else { @@ -104,13 +104,13 @@ protected virtual async Task DownloadBySchemeAsync(Uri uri) return result; } - protected static IXRefContainer DownloadFromLocal(Uri uri) + private static ValueTask DownloadFromLocalAsync(Uri uri, CancellationToken token = default) { var filePath = uri.LocalPath; - return ReadLocalFile(filePath); + return ReadLocalFileAsync(filePath, token); } - private static IXRefContainer ReadLocalFile(string filePath) + private static async ValueTask ReadLocalFileAsync(string filePath, CancellationToken token = default) { Logger.LogVerbose($"Reading from file: {filePath}"); @@ -121,8 +121,8 @@ private static IXRefContainer ReadLocalFile(string filePath) case ".json": { - using var stream = File.OpenText(filePath); - return JsonUtility.Deserialize(stream); + using var stream = File.OpenRead(filePath); + return await SystemTextJsonUtility.DeserializeAsync(stream, token); } case ".yml": @@ -134,7 +134,7 @@ private static IXRefContainer ReadLocalFile(string filePath) } } - protected static async Task DownloadFromWebAsync(Uri uri) + private static async Task DownloadFromWebAsync(Uri uri, CancellationToken token = default) { Logger.LogVerbose($"Reading from web: {uri.OriginalString}"); @@ -147,14 +147,13 @@ protected static async Task DownloadFromWebAsync(Uri uri) Timeout = TimeSpan.FromMinutes(30), // Default: 100 seconds }; - using var stream = await httpClient.GetStreamAsync(uri); + using var stream = await httpClient.GetStreamAsync(uri, token); switch (Path.GetExtension(uri.AbsolutePath).ToLowerInvariant()) { case ".json": { - using var sr = new StreamReader(stream, bufferSize: 81920); // Default :1024 byte - var xrefMap = JsonUtility.Deserialize(sr); + var xrefMap = await SystemTextJsonUtility.DeserializeAsync(stream, token); xrefMap.BaseUrl = ResolveBaseUrl(xrefMap, uri); return xrefMap; } diff --git a/src/Docfx.Build/XRefMaps/XRefMapRedirection.cs b/src/Docfx.Build/XRefMaps/XRefMapRedirection.cs index 1035a7f0650..a053fe1862c 100644 --- a/src/Docfx.Build/XRefMaps/XRefMapRedirection.cs +++ b/src/Docfx.Build/XRefMaps/XRefMapRedirection.cs @@ -15,7 +15,7 @@ public class XRefMapRedirection public string UidPrefix { get; set; } [YamlMember(Alias = "href")] - [JsonProperty("Href")] + [JsonProperty("href")] [JsonPropertyName("href")] public string Href { get; set; } } diff --git a/src/Docfx.Common/Json/System.Text.Json/ObjectToInferredTypesConverter.cs b/src/Docfx.Common/Json/System.Text.Json/ObjectToInferredTypesConverter.cs new file mode 100644 index 00000000000..ac3664e6d3c --- /dev/null +++ b/src/Docfx.Common/Json/System.Text.Json/ObjectToInferredTypesConverter.cs @@ -0,0 +1,77 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Text.Json; +using System.Text.Json.Serialization; + +#nullable enable + +namespace Docfx.Common; + +/// +/// Custom JsonConverters for . +/// +/// +/// +internal class ObjectToInferredTypesConverter : JsonConverter +{ + /// + public override object? Read( + ref Utf8JsonReader reader, + Type typeToConvert, + JsonSerializerOptions options) + { + switch (reader.TokenType) + { + case JsonTokenType.True: + return true; + case JsonTokenType.False: + return false; + case JsonTokenType.Number when reader.TryGetInt32(out int intValue): + return intValue; + case JsonTokenType.Number when reader.TryGetInt64(out long longValue): + return longValue; + case JsonTokenType.Number: + return reader.GetDouble(); + case JsonTokenType.String when reader.TryGetDateTime(out DateTime datetime): + return datetime; + case JsonTokenType.String: + return reader.GetString(); + case JsonTokenType.Null: + return null; + case JsonTokenType.StartArray: + { + var list = new List(); + while (reader.Read() && reader.TokenType != JsonTokenType.EndArray) + { + object? element = Read(ref reader, typeof(object), options); + list.Add(element); + } + return list; + } + case JsonTokenType.StartObject: + { + try + { + using var doc = JsonDocument.ParseValue(ref reader); + return JsonSerializer.Deserialize>(doc, options); + } + catch (Exception) + { + goto default; + } + } + default: + { + using var doc = JsonDocument.ParseValue(ref reader); + return doc.RootElement.Clone(); + } + } + } + + /// + public override void Write(Utf8JsonWriter writer, object objectToWrite, JsonSerializerOptions options) + { + JsonSerializer.Serialize(writer, objectToWrite, objectToWrite.GetType(), options); + } +} diff --git a/src/Docfx.Common/Json/System.Text.Json/SystemTextJsonUtility.cs b/src/Docfx.Common/Json/System.Text.Json/SystemTextJsonUtility.cs new file mode 100644 index 00000000000..c0aa3527de8 --- /dev/null +++ b/src/Docfx.Common/Json/System.Text.Json/SystemTextJsonUtility.cs @@ -0,0 +1,101 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Text.Json; +using System.Text.Json.Serialization; + +#nullable enable + +namespace Docfx.Common; + +/// +/// Utility class for JSON serialization/deserialization. +/// +public static class SystemTextJsonUtility +{ + /// + /// Default JsonSerializerOptions options. + /// + public static readonly JsonSerializerOptions DefaultSerializerOptions; + + /// + /// Default JsonSerializerOptions options with indent setting. + /// + public static readonly JsonSerializerOptions IndentedSerializerOptions; + + static SystemTextJsonUtility() + { + DefaultSerializerOptions = new JsonSerializerOptions() + { + // DefaultBufferSize = 1024 * 16, // TODO: Set appropriate buffer size based on benchmark.(Default: 16KB) + AllowTrailingCommas = true, + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull | JsonIgnoreCondition.WhenWritingDefault, + PropertyNameCaseInsensitive = true, + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + // DictionaryKeyPolicy = JsonNamingPolicy.CamelCase, // This setting is not compatible to `Newtonsoft.Json` serialize result. + NumberHandling = JsonNumberHandling.AllowReadingFromString, + Converters = + { + new JsonStringEnumConverter(), + new ObjectToInferredTypesConverter(), // Required for `Dictionary` type deserialization. + }, + WriteIndented = false, + }; + + IndentedSerializerOptions = new JsonSerializerOptions(DefaultSerializerOptions) + { + WriteIndented = true, + }; + } + + /// + /// Converts the value of a type specified by a generic type parameter into a JSON string. + /// + public static string Serialize(T model, bool indented = false) + { + var options = indented + ? IndentedSerializerOptions + : DefaultSerializerOptions; + + return JsonSerializer.Serialize(model, options); + } + + /// + /// Converts the value of a type specified by a generic type parameter into a JSON string. + /// + public static string Serialize(Stream stream, bool indented = false) + { + var options = indented + ? IndentedSerializerOptions + : DefaultSerializerOptions; + + return JsonSerializer.Serialize(stream, options); + } + + /// + /// Reads the UTF-8 encoded text representing a single JSON value into a TValue. + /// The Stream will be read to completion. + /// + public static T? Deserialize(string json) + { + return JsonSerializer.Deserialize(json, DefaultSerializerOptions); + } + + /// + /// Reads the UTF-8 encoded text representing a single JSON value into a TValue. + /// The Stream will be read to completion. + /// + public static T? Deserialize(Stream stream) + { + return JsonSerializer.Deserialize(stream, DefaultSerializerOptions); + } + + /// + /// Asynchronously reads the UTF-8 encoded text representing a single JSON value + // into an instance of a type specified by a generic type parameter. The stream + // will be read to completion. + public static async ValueTask DeserializeAsync(Stream stream, CancellationToken token = default) + { + return await JsonSerializer.DeserializeAsync(stream, DefaultSerializerOptions, cancellationToken: token); + } +} diff --git a/test/Docfx.Build.Tests/XRefMapSerializationTest.cs b/test/Docfx.Build.Tests/XRefMapSerializationTest.cs index 2300f4696d6..b36348bfb36 100644 --- a/test/Docfx.Build.Tests/XRefMapSerializationTest.cs +++ b/test/Docfx.Build.Tests/XRefMapSerializationTest.cs @@ -10,7 +10,7 @@ namespace Docfx.Build.Engine.Tests; public class XRefMapSerializationTest -{ +{ [Fact] public void XRefMapSerializationRoundTripTest() { @@ -43,17 +43,49 @@ public void XRefMapSerializationRoundTripTest() }, Others = new Dictionary { - ["Other1"] = "Dummy", + ["StringValue"] = "Dummy", + ["BooleanValue"] = true, + ["IntValue"] = int.MaxValue, + ["LongValue"] = long.MaxValue, + ["DoubleValue"] = 1.234d, + + //// YamlDotNet don't deserialize dictionary's null value. + // ["NullValue"] = null, + + //// Following types has no deserialize compatibility (NewtonsoftJson deserialize value to JArray/Jvalue) + // ["ArrayValue"] = new object[] { 1, 2, 3 }, + // ["ObjectValue"] = new Dictionary{["Prop1"="Dummy"]} } }; - // Arrange - var jsonResult = RoundtripByNewtonsoftJson(model); - var yamlResult = RoundtripWithYamlDotNet(model); + // Validate serialized JSON text. + { + // Arrange + var systemTextJson = SystemTextJsonUtility.Serialize(model); + var newtonsoftJson = JsonUtility.Serialize(model); - // Assert - jsonResult.Should().BeEquivalentTo(model); - yamlResult.Should().BeEquivalentTo(model); + // Assert + systemTextJson.Should().Be(newtonsoftJson); + } + + // Validate roundtrip result. + { + // Arrange + var systemTextJsonResult = RoundtripBySystemTextJson(model); + var newtonsoftJsonResult = RoundtripByNewtonsoftJson(model); + var yamlResult = RoundtripWithYamlDotNet(model); + + // Assert + systemTextJsonResult.Should().BeEquivalentTo(model); + newtonsoftJsonResult.Should().BeEquivalentTo(model); + yamlResult.Should().BeEquivalentTo(model); + } + } + + private static T RoundtripBySystemTextJson(T model) + { + var json = SystemTextJsonUtility.Serialize(model); + return SystemTextJsonUtility.Deserialize(json); } private static T RoundtripByNewtonsoftJson(T model) diff --git a/test/docfx.Tests/Api.verified.cs b/test/docfx.Tests/Api.verified.cs index 1b018280980..4eb91e431cb 100644 --- a/test/docfx.Tests/Api.verified.cs +++ b/test/docfx.Tests/Api.verified.cs @@ -469,13 +469,10 @@ public Docfx.Build.Engine.IXRefContainerReader GetReader() { } public void Sort() { } public void UpdateHref(System.Uri baseUri) { } } - public class XRefMapDownloader + public sealed class XRefMapDownloader { public XRefMapDownloader(string baseFolder = null, System.Collections.Generic.IReadOnlyList fallbackFolders = null, int maxParallelism = 16) { } - public System.Threading.Tasks.Task DownloadAsync(System.Uri uri) { } - protected virtual System.Threading.Tasks.Task DownloadBySchemeAsync(System.Uri uri) { } - protected static Docfx.Build.Engine.IXRefContainer DownloadFromLocal(System.Uri uri) { } - protected static System.Threading.Tasks.Task DownloadFromWebAsync(System.Uri uri) { } + public System.Threading.Tasks.Task DownloadAsync(System.Uri uri, System.Threading.CancellationToken token = default) { } } public sealed class XRefMapReader : Docfx.Build.Engine.XRefRedirectionReader { @@ -485,7 +482,7 @@ protected override Docfx.Build.Engine.IXRefContainer GetMap(string name) { } public class XRefMapRedirection { public XRefMapRedirection() { } - [Newtonsoft.Json.JsonProperty("Href")] + [Newtonsoft.Json.JsonProperty("href")] [System.Text.Json.Serialization.JsonPropertyName("href")] [YamlDotNet.Serialization.YamlMember(Alias="href")] public string Href { get; set; } @@ -2056,6 +2053,16 @@ public static class Build public const string EmptyInputFiles = "EmptyInputFiles"; } } + public static class SystemTextJsonUtility + { + public static readonly System.Text.Json.JsonSerializerOptions DefaultSerializerOptions; + public static readonly System.Text.Json.JsonSerializerOptions IndentedSerializerOptions; + public static T? Deserialize(System.IO.Stream stream) { } + public static T? Deserialize(string json) { } + public static System.Threading.Tasks.ValueTask DeserializeAsync(System.IO.Stream stream, System.Threading.CancellationToken token = default) { } + public static string Serialize(System.IO.Stream stream, bool indented = false) { } + public static string Serialize(T model, bool indented = false) { } + } public static class TreeIterator { public static void Preorder(T current, T parent, System.Func> childrenGetter, System.Func action) { } From bb8cfd1d4e311bd94f444843aa3e2e4cc2bd6cdd Mon Sep 17 00:00:00 2001 From: filzrev <103790468+filzrev@users.noreply.github.com> Date: Fri, 12 Apr 2024 14:05:03 +0900 Subject: [PATCH 2/3] chore: change SystemTextJsonUtility visibility from public to internal --- src/Docfx.Common/Docfx.Common.csproj | 8 +++++++- .../Json/System.Text.Json/SystemTextJsonUtility.cs | 2 +- test/docfx.Tests/Api.verified.cs | 10 ---------- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/Docfx.Common/Docfx.Common.csproj b/src/Docfx.Common/Docfx.Common.csproj index 46d7bccbca5..d83fc78f609 100644 --- a/src/Docfx.Common/Docfx.Common.csproj +++ b/src/Docfx.Common/Docfx.Common.csproj @@ -1,4 +1,4 @@ - + @@ -7,4 +7,10 @@ + + + + + + diff --git a/src/Docfx.Common/Json/System.Text.Json/SystemTextJsonUtility.cs b/src/Docfx.Common/Json/System.Text.Json/SystemTextJsonUtility.cs index c0aa3527de8..1011f2e46f9 100644 --- a/src/Docfx.Common/Json/System.Text.Json/SystemTextJsonUtility.cs +++ b/src/Docfx.Common/Json/System.Text.Json/SystemTextJsonUtility.cs @@ -11,7 +11,7 @@ namespace Docfx.Common; /// /// Utility class for JSON serialization/deserialization. /// -public static class SystemTextJsonUtility +internal static class SystemTextJsonUtility { /// /// Default JsonSerializerOptions options. diff --git a/test/docfx.Tests/Api.verified.cs b/test/docfx.Tests/Api.verified.cs index 4eb91e431cb..978bdf5a41f 100644 --- a/test/docfx.Tests/Api.verified.cs +++ b/test/docfx.Tests/Api.verified.cs @@ -2053,16 +2053,6 @@ public static class Build public const string EmptyInputFiles = "EmptyInputFiles"; } } - public static class SystemTextJsonUtility - { - public static readonly System.Text.Json.JsonSerializerOptions DefaultSerializerOptions; - public static readonly System.Text.Json.JsonSerializerOptions IndentedSerializerOptions; - public static T? Deserialize(System.IO.Stream stream) { } - public static T? Deserialize(string json) { } - public static System.Threading.Tasks.ValueTask DeserializeAsync(System.IO.Stream stream, System.Threading.CancellationToken token = default) { } - public static string Serialize(System.IO.Stream stream, bool indented = false) { } - public static string Serialize(T model, bool indented = false) { } - } public static class TreeIterator { public static void Preorder(T current, T parent, System.Func> childrenGetter, System.Func action) { } From b86e143697bce8939adfed06f676945a815b64c2 Mon Sep 17 00:00:00 2001 From: filzrev <103790468+filzrev@users.noreply.github.com> Date: Fri, 12 Apr 2024 14:14:22 +0900 Subject: [PATCH 3/3] chore: remove WhenWritingDefault attribute --- src/Docfx.Common/Json/System.Text.Json/SystemTextJsonUtility.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Docfx.Common/Json/System.Text.Json/SystemTextJsonUtility.cs b/src/Docfx.Common/Json/System.Text.Json/SystemTextJsonUtility.cs index 1011f2e46f9..e952067a491 100644 --- a/src/Docfx.Common/Json/System.Text.Json/SystemTextJsonUtility.cs +++ b/src/Docfx.Common/Json/System.Text.Json/SystemTextJsonUtility.cs @@ -29,7 +29,7 @@ static SystemTextJsonUtility() { // DefaultBufferSize = 1024 * 16, // TODO: Set appropriate buffer size based on benchmark.(Default: 16KB) AllowTrailingCommas = true, - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull | JsonIgnoreCondition.WhenWritingDefault, + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, PropertyNameCaseInsensitive = true, PropertyNamingPolicy = JsonNamingPolicy.CamelCase, // DictionaryKeyPolicy = JsonNamingPolicy.CamelCase, // This setting is not compatible to `Newtonsoft.Json` serialize result.