Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate CacheFileFormat to System.Text.Json #6081

Open
wants to merge 14 commits into
base: dev
Choose a base branch
from
Open

Migrate CacheFileFormat to System.Text.Json #6081

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
aad148e
Migrate CacheFileFormat to system.text.json
Nigusu-Allehu Oct 3, 2024
e3823cb
Add tests and fix json write
Nigusu-Allehu Oct 4, 2024
179bd18
cleanup
Nigusu-Allehu Oct 4, 2024
75ad105
cleanup
Nigusu-Allehu Oct 5, 2024
7bf9b8e
remove getawaiter
Nigusu-Allehu Oct 5, 2024
a6aed78
Add condition
Nigusu-Allehu Oct 15, 2024
cc13dd6
undo lockfileformat.cs edit
Nigusu-Allehu Oct 15, 2024
6aa1c4e
undo lockfileformat.cs edit
Nigusu-Allehu Oct 15, 2024
655b5f0
simplify converter
Nigusu-Allehu Oct 31, 2024
6a6d8cb
simpify
Nigusu-Allehu Oct 31, 2024
3956ece
cleanup
Nigusu-Allehu Feb 26, 2025
3135134
Test
Nigusu-Allehu Feb 26, 2025
f79c1a7
undo
Nigusu-Allehu Feb 26, 2025
6280326
Preserve old code
Nigusu-Allehu Feb 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions src/NuGet.Core/NuGet.ProjectModel/CacheFile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Text.Json.Serialization;
using NuGet.Common;
using NuGet.Shared;

Expand All @@ -12,34 +13,42 @@ public class CacheFile : IEquatable<CacheFile>
{
internal const int CurrentVersion = 2;

public string DgSpecHash { get; }

[JsonPropertyName("version")]
public int Version { get; set; }

[JsonPropertyName("dgSpecHash")]
public string DgSpecHash { get; }

[JsonPropertyName("success")]
public bool Success { get; set; }

/// <summary>
/// Gets or sets the full path to the project file.
/// </summary>
[JsonPropertyName("projectFilePath")]
public string ProjectFilePath { get; set; }

/// <summary>
/// Gets or sets a list of package paths that must exist in order for the project to be considered up-to-date.
/// </summary>
[JsonPropertyName("expectedPackageFiles")]
public IList<string> ExpectedPackageFilePaths { get; set; }

/// <summary>
/// Gets or sets a value indicating if one or more of the expected files are missing.
/// </summary>
[JsonIgnore]
[Obsolete("File existence checks are a function of time not the cache file content.")]
public bool HasAnyMissingPackageFiles
{
get => throw new NotImplementedException("This API is no longer support");
set => throw new NotImplementedException("This API is no longer support");
}

/// <summary>
/// Gets or sets the full path to the project file.
/// </summary>
public string ProjectFilePath { get; set; }

[JsonPropertyName("logs")]
public IList<IAssetsLogMessage> LogMessages { get; set; }

[JsonIgnore]
public bool IsValid { get { return Version == CurrentVersion && Success && DgSpecHash != null; } }

public CacheFile(string dgSpecHash)
Expand Down
136 changes: 32 additions & 104 deletions src/NuGet.Core/NuGet.ProjectModel/CacheFileFormat.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,52 +2,61 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using System.Text.Encodings.Web;
using System.Text.Json;
using System.Text.Json.Serialization;
using NuGet.Common;

namespace NuGet.ProjectModel
{
public static class CacheFileFormat
{
private const string VersionProperty = "version";
private const string DGSpecHashProperty = "dgSpecHash";
private const string SuccessProperty = "success";
private const string ExpectedPackageFilesProperty = "expectedPackageFiles";
private const string ProjectFilePathProperty = "projectFilePath";

public static CacheFile Read(Stream stream, ILogger log, string path)
private static JsonSerializerOptions SerializerOptions = new JsonSerializerOptions
{
if (stream == null) throw new ArgumentNullException(nameof(stream));
if (log == null) throw new ArgumentNullException(nameof(log));
if (path == null) throw new ArgumentNullException(nameof(path));
WriteIndented = true,
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
Converters = { new AssetsLogMessageConverter() }
};

/// <summary>
/// Since Log messages property in CacheFile is an interface type, we have the following custom converter to deserialize the IAssetsLogMessage objects.
/// </summary>
private class AssetsLogMessageConverter : JsonConverter<IAssetsLogMessage>
{
public override IAssetsLogMessage Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
return JsonSerializer.Deserialize<AssetsLogMessage>(ref reader, options);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding https://github.com/NuGet/NuGet.Client/pull/6081/files#r1825078556, we lose about 30% perf needing to go though an additional converter:

code 
using System.Text;
using System.Text.Json;
using System.Text.Json.Serialization;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

[MemoryDiagnoser]
public class Program
{
    private static void Main(string[] args)
    {
        BenchmarkRunner.Run<Program>();
    }

    private static JsonSerializerOptions options1;
    private static JsonSerializerOptions options2;

    [GlobalSetup]
    public void Setup()
    {
        options1 = new JsonSerializerOptions();
        options1.Converters.Add(new ThingConverter1());

        options2 = new JsonSerializerOptions();
        options2.Converters.Add(new ThingConverter2());
    }

    private byte[] json = Encoding.UTF8.GetBytes("{\"Value\":\"something\"}");

    [Benchmark]
    public IThing? JsonSerializerDeserialize()
    {
        return JsonSerializer.Deserialize<IThing>(json, options1);
    }

    [Benchmark]
    public IThing? JsonConverterRead()
    {
        return JsonSerializer.Deserialize<IThing>(json, options2);
    }

    [Benchmark(Baseline = true)]
    public Thing? Baseline()
    {
        return JsonSerializer.Deserialize<Thing>(json);
    }

    public interface IThing
    {
        public string Value { get; }
    }

    public class Thing : IThing
    {
        public required string Value { get; init; }
    }

    private class ThingConverter1 : JsonConverter<IThing>
    {
        public override IThing? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
        {
            return JsonSerializer.Deserialize<Thing>(ref reader, options);
        }

        public override void Write(Utf8JsonWriter writer, IThing value, JsonSerializerOptions options)
        {
            throw new NotImplementedException();
        }
    }

    private class ThingConverter2 : JsonConverter<IThing>
    {
        public override IThing? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
        {
            JsonConverter<Thing> converter = (JsonConverter<Thing>)options.GetConverter(typeof(Thing));
            return converter.Read(ref reader, typeof(Thing), options);
        }

        public override void Write(Utf8JsonWriter writer, IThing value, JsonSerializerOptions options)
        {
            throw new NotImplementedException();
        }
    }
}
Method Mean Error StdDev Ratio RatioSD Gen0 Allocated Alloc Ratio
JsonSerializerDeserialize 189.38 ns 1.185 ns 1.050 ns 2.00 0.02 0.0076 128 B 1.00
JsonConverterRead 123.56 ns 0.691 ns 0.647 ns 1.31 0.01 0.0076 128 B 1.00
Baseline 94.51 ns 0.602 ns 0.563 ns 1.00 0.01 0.0076 128 B 1.00

We lose even more perf calling JsonSerializer.Deserialize from the converter, rather than my suggestion above, getting the converter directly from options, and calling the converter.

It depends on how much we want to microoptimize restore. 30 nanoseconds per project, on my machine, but nanoseconds are super small. Is it worth the public API breaking changes for it? Probably not. But I still think that IAssetsLogMessage doesn't have a reason to exist.

But I think the additional 60 nanoseconds lost by using JsonSerializer.Deserialize is not worth the 1 line of code it saves over getting the converter directly.

}

using (var textReader = new StreamReader(stream))
public override void Write(Utf8JsonWriter writer, IAssetsLogMessage value, JsonSerializerOptions options)
{
return Read(textReader, log, path);
JsonSerializer.Serialize(writer, (AssetsLogMessage)value, options);
}
}

private static CacheFile Read(TextReader reader, ILogger log, string path)
public static CacheFile Read(Stream stream, ILogger log, string path)
{
if (stream == null) throw new ArgumentNullException(nameof(stream));
if (log == null) throw new ArgumentNullException(nameof(log));
if (path == null) throw new ArgumentNullException(nameof(path));

try
{
var json = JsonUtility.LoadJson(reader);
var cacheFile = ReadCacheFile(json);
var cacheFile = JsonSerializer.Deserialize<CacheFile>(utf8Json: stream, SerializerOptions);
return cacheFile;
}
catch (Exception ex)
catch (Exception ex) when (ex is ArgumentNullException || ex is JsonException || ex is NotSupportedException)
{
log.LogWarning(string.Format(CultureInfo.CurrentCulture,
Strings.Log_ProblemReadingCacheFile,
path, ex.Message));

// Parsing error, the cache file is invalid.
return new CacheFile(null);
}

return new CacheFile(null);
}

public static void Write(string filePath, CacheFile lockFile)
Expand Down Expand Up @@ -76,88 +85,7 @@ public static void Write(Stream stream, CacheFile cacheFile)

private static void Write(TextWriter textWriter, CacheFile cacheFile)
{
using (var jsonWriter = new JsonTextWriter(textWriter))
{
jsonWriter.Formatting = Formatting.Indented;
var json = GetCacheFile(cacheFile);
json.WriteTo(jsonWriter);
}
}

private static CacheFile ReadCacheFile(JObject cursor)
{
var version = ReadInt(cursor[VersionProperty]);
var hash = ReadString(cursor[DGSpecHashProperty]);
var success = ReadBool(cursor[SuccessProperty]);
var cacheFile = new CacheFile(hash);
cacheFile.Version = version;
cacheFile.Success = success;

if (version >= 2)
{
cacheFile.ProjectFilePath = ReadString(cursor[ProjectFilePathProperty]);
cacheFile.ExpectedPackageFilePaths = new List<string>();
foreach (JToken expectedFile in cursor[ExpectedPackageFilesProperty])
{
string path = ReadString(expectedFile);

if (!string.IsNullOrWhiteSpace(path))
{
cacheFile.ExpectedPackageFilePaths.Add(path);
}
}

cacheFile.LogMessages = LockFileFormat.ReadLogMessageArray(cursor[LockFileFormat.LogsProperty] as JArray, cacheFile.ProjectFilePath);
}

return cacheFile;
}

private static JObject GetCacheFile(CacheFile cacheFile)
{
var json = new JObject();
json[VersionProperty] = WriteInt(cacheFile.Version);
json[DGSpecHashProperty] = WriteString(cacheFile.DgSpecHash);
json[SuccessProperty] = WriteBool(cacheFile.Success);

if (cacheFile.Version >= 2)
{
json[ProjectFilePathProperty] = cacheFile.ProjectFilePath;
json[ExpectedPackageFilesProperty] = new JArray(cacheFile.ExpectedPackageFilePaths);
json[LockFileFormat.LogsProperty] = cacheFile.LogMessages == null ? new JArray() : LockFileFormat.WriteLogMessages(cacheFile.LogMessages, cacheFile.ProjectFilePath);
}

return json;
}

private static string ReadString(JToken json)
{
return json.Value<string>();
}

private static JToken WriteString(string item)
{
return item != null ? new JValue(item) : JValue.CreateNull();
}

private static int ReadInt(JToken json)
{
return json.Value<int>();
}

private static JToken WriteInt(int item)
{
return new JValue(item);
}

private static bool ReadBool(JToken json)
{
return json.Value<bool>();
}

private static JToken WriteBool(bool item)
{
return new JValue(item);
textWriter.Write(JsonSerializer.Serialize(cacheFile, SerializerOptions));
}
}
}
1 change: 0 additions & 1 deletion src/NuGet.Core/NuGet.ProjectModel/GlobalSuppressions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using System.Diagnostics.CodeAnalysis;

[assembly: SuppressMessage("Build", "CA1062:In externally visible method 'IAssetsLogMessage AssetsLogMessage.Create(IRestoreLogMessage logMessage)', validate parameter 'logMessage' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectModel.AssetsLogMessage.Create(NuGet.Common.IRestoreLogMessage)~NuGet.ProjectModel.IAssetsLogMessage")]
[assembly: SuppressMessage("Build", "CA1031:Modify 'Read' to catch a more specific allowed exception type, or rethrow the exception.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectModel.CacheFileFormat.Read(System.IO.TextReader,NuGet.Common.ILogger,System.String)~NuGet.ProjectModel.CacheFile")]
[assembly: SuppressMessage("Build", "CA1062:In externally visible method 'void DependencyGraphSpec.AddProject(PackageSpec projectSpec)', validate parameter 'projectSpec' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectModel.DependencyGraphSpec.AddProject(NuGet.ProjectModel.PackageSpec)")]
[assembly: SuppressMessage("Build", "CA1062:In externally visible method 'DependencyGraphSpec DependencyGraphSpec.WithReplacedSpec(PackageSpec project)', validate parameter 'project' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectModel.DependencyGraphSpec.WithReplacedSpec(NuGet.ProjectModel.PackageSpec)~NuGet.ProjectModel.DependencyGraphSpec")]
[assembly: SuppressMessage("Build", "CA1062:In externally visible method 'FileFormatException FileFormatException.Create(Exception exception, JToken value, string path)', validate parameter 'exception' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.ProjectModel.FileFormatException.Create(System.Exception,Newtonsoft.Json.Linq.JToken,System.String)~NuGet.ProjectModel.FileFormatException")]
Expand Down
58 changes: 57 additions & 1 deletion src/NuGet.Core/NuGet.ProjectModel/LockFile/AssetsLogMessage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,54 @@

using System;
using System.Collections.Generic;
using System.Text.Json.Serialization;
using NuGet.Common;
using NuGet.Shared;

namespace NuGet.ProjectModel
{
public class AssetsLogMessage : IAssetsLogMessage, IEquatable<IAssetsLogMessage>
{

[JsonConverter(typeof(JsonStringEnumConverter))]
public LogLevel Level { get; }

[JsonConverter(typeof(JsonStringEnumConverter))]
public NuGetLogCode Code { get; }
public string Message { get; }
public string ProjectPath { get; set; }

[JsonIgnore]
public WarningLevel WarningLevel { get; set; } = WarningLevel.Severe; //setting default to Severe as 0 implies show no warnings

[JsonInclude]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]
[JsonPropertyName("warningLevel")]
internal WarningLevel? WarningLevelJson
{
get => Level == LogLevel.Warning ? WarningLevel : null;
set
{
if (value.HasValue)
{
WarningLevel = value.Value;
}
}
}

public string FilePath { get; set; }
public string LibraryId { get; set; }
public IReadOnlyList<string> TargetGraphs { get; set; }

[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]
public int StartLineNumber { get; set; }

[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]
public int StartColumnNumber { get; set; }

[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]
public int EndLineNumber { get; set; }

[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]
public int EndColumnNumber { get; set; }

public static IAssetsLogMessage Create(IRestoreLogMessage logMessage)
Expand All @@ -40,6 +69,33 @@ public static IAssetsLogMessage Create(IRestoreLogMessage logMessage)
};
}

[JsonConstructor]
private AssetsLogMessage(
LogLevel level,
NuGetLogCode code,
string message,
string projectPath,
string filePath,
string libraryId,
IReadOnlyList<string> targetGraphs,
int startLineNumber,
int startColumnNumber,
int endLineNumber,
int endColumnNumber)
{
Level = level;
Code = code;
Message = message;
ProjectPath = projectPath;
FilePath = filePath;
LibraryId = libraryId;
TargetGraphs = targetGraphs ?? new List<string>();
StartLineNumber = startLineNumber;
StartColumnNumber = startColumnNumber;
EndLineNumber = endLineNumber;
EndColumnNumber = endColumnNumber;
}

public AssetsLogMessage(LogLevel logLevel, NuGetLogCode errorCode,
string errorString, string targetGraph)
{
Expand Down
4 changes: 4 additions & 0 deletions src/NuGet.Core/NuGet.ProjectModel/NuGet.ProjectModel.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,8 @@
<ItemGroup>
<InternalsVisibleTo Include="NuGet.ProjectModel.Test" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="System.Text.Json" Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'" />
</ItemGroup>
</Project>
Loading
Loading