Skip to content
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
a57aabf
Copied initial files over and addressred some PR comments
jgonz120 Dec 20, 2023
05ed3db
Test updates
jgonz120 Dec 21, 2023
c285f0f
optimized string split
jgonz120 Dec 21, 2023
c06e3f5
cleanup
jgonz120 Dec 21, 2023
ef90a51
switch to stream
jgonz120 Dec 21, 2023
4c31cbf
fix typo
jgonz120 Dec 21, 2023
b787826
fix typo
jgonz120 Dec 21, 2023
c627b25
create static json reader state
jgonz120 Dec 21, 2023
ef35381
typo
jgonz120 Dec 21, 2023
084986d
typo
jgonz120 Dec 21, 2023
5850754
added lazy string split
jgonz120 Dec 22, 2023
014739f
using
jgonz120 Dec 22, 2023
d28a482
Update unit tests
jgonz120 Dec 22, 2023
c9cb53f
unit tests
jgonz120 Dec 22, 2023
d083e23
fix references
jgonz120 Dec 22, 2023
0b22126
Fix typo
jgonz120 Dec 22, 2023
e81974f
Added test for invalid logs
jgonz120 Jan 4, 2024
d2cc5d6
removed extra name assignment from package spec reader
jgonz120 Jan 4, 2024
2de68c4
use array empty
jgonz120 Jan 4, 2024
79e2b71
move public method up
jgonz120 Jan 4, 2024
ccd6fe1
remove uneeded string list
jgonz120 Jan 4, 2024
8d719c6
use false string
jgonz120 Jan 4, 2024
212b7bd
add is final block to test
jgonz120 Jan 4, 2024
b5a82d7
rename test
jgonz120 Jan 4, 2024
82363ee
style
jgonz120 Jan 4, 2024
0cc4e51
style
jgonz120 Jan 4, 2024
c525dc8
add tests for validating empty streams on creationg of utf8jsonstream…
jgonz120 Jan 4, 2024
4e5112a
Added test and implemented string split in two
jgonz120 Jan 5, 2024
2398980
fix validation for lazy string split
jgonz120 Jan 8, 2024
1693913
reduce methods in LikeFileFormat
jgonz120 Jan 8, 2024
e0ef66d
set the list values with the results directly
jgonz120 Jan 8, 2024
b4d0169
store environment variable to avoid calling GetEnvironmentVariable se…
jgonz120 Jan 8, 2024
f060670
switch to splitintwo
jgonz120 Jan 9, 2024
4602b44
Update conditional for framework
jgonz120 Jan 9, 2024
a240062
Caching the parsed NugetVersion and VersionRange objects.
jgonz120 Jan 11, 2024
0883567
Fixes from PR
jgonz120 Jan 11, 2024
c4adaf7
Avoid creating empty lists
jgonz120 Jan 11, 2024
bda30bb
Revert "Caching the parsed NugetVersion and VersionRange objects."
jgonz120 Jan 17, 2024
fb7ebd5
Fix netwonsoft json parsing
jgonz120 Jan 17, 2024
32aba9a
Add missing reference
jgonz120 Jan 17, 2024
8853734
added some examples of the json to be parsed
jgonz120 Jan 17, 2024
1b29455
Fix references in comments
jgonz120 Jan 17, 2024
1877448
Fixes from PR
jgonz120 Jan 22, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -112,19 +112,7 @@ public partial class JsonPackageSpecReader
internal static PackageSpec GetPackageSpecUtf8JsonStreamReader(Stream stream, string name, string packageSpecPath, string snapshotValue)
{
var reader = new Utf8JsonStreamReader(stream);
PackageSpec packageSpec;
packageSpec = GetPackageSpec(ref reader, name, packageSpecPath, snapshotValue);

if (!string.IsNullOrEmpty(name))
{
packageSpec.Name = name;
if (!string.IsNullOrEmpty(packageSpecPath))
{
packageSpec.FilePath = Path.GetFullPath(packageSpecPath);

}
}
return packageSpec;
return GetPackageSpec(ref reader, name, packageSpecPath, snapshotValue);
}

internal static PackageSpec GetPackageSpec(ref Utf8JsonStreamReader jsonReader, string name, string packageSpecPath, string snapshotValue)
Expand Down Expand Up @@ -328,7 +316,7 @@ private static LibraryDependency ReadLibraryDependency(ref Utf8JsonStreamReader
{
try
{
dependencyVersionRange = VersionRange.Parse(dependencyVersionValue);
dependencyVersionRange = JsonUtility.ParseVersionRange(dependencyVersionValue);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -498,7 +486,7 @@ private static void ReadDependencies(
var versionPropValue = jsonReader.GetString();
try
{
versionOverride = VersionRange.Parse(versionPropValue);
versionOverride = JsonUtility.ParseVersionRange(versionPropValue);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -527,7 +515,7 @@ private static void ReadDependencies(
{
try
{
dependencyVersionRange = VersionRange.Parse(dependencyVersionValue);
dependencyVersionRange = JsonUtility.ParseVersionRange(dependencyVersionValue);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -637,7 +625,7 @@ private static void ReadCentralPackageVersions(
throw FileFormatException.Create("The version cannot be null or empty.", filePath);
}

centralPackageVersions[propertyName] = new CentralPackageVersion(propertyName, VersionRange.Parse(version));
centralPackageVersions[propertyName] = new CentralPackageVersion(propertyName, JsonUtility.ParseVersionRange(version));
}
}
}
Expand Down Expand Up @@ -733,13 +721,18 @@ private static void ReadDownloadDependencies(
packageSpecPath);
}

string[] versions = versionValue.Split(VersionSeparators, StringSplitOptions.RemoveEmptyEntries);
var versions = new LazyStringSplit(versionValue, VersionSeparator);

foreach (string singleVersionValue in versions)
{
if (string.IsNullOrEmpty(singleVersionValue))
{
continue;
}

try
{
VersionRange version = VersionRange.Parse(singleVersionValue);
VersionRange version = JsonUtility.ParseVersionRange(singleVersionValue);

downloadDependencies.Add(new DownloadDependency(name, version));
}
Expand Down Expand Up @@ -1496,7 +1489,7 @@ private static RuntimeDependencySet ReadRuntimeDependencySet(ref Utf8JsonStreamR
var propertyName = jsonReader.GetString();
dependencies ??= [];

var dependency = new RuntimePackageDependency(propertyName, VersionRange.Parse(jsonReader.ReadNextTokenAsString()));
var dependency = new RuntimePackageDependency(propertyName, JsonUtility.ParseVersionRange(jsonReader.ReadNextTokenAsString()));

dependencies.Add(dependency);
}
Expand Down
18 changes: 9 additions & 9 deletions src/NuGet.Core/NuGet.ProjectModel/JsonPackageSpecReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public static partial class JsonPackageSpecReader
{
private static readonly char[] DelimitedStringSeparators = { ' ', ',' };
private static readonly char[] VersionSeparators = new[] { ';' };
private const char VersionSeparator = ';';
public static readonly string RestoreOptions = "restore";
public static readonly string RestoreSettings = "restoreSettings";
public static readonly string HideWarningsAndErrors = "hideWarningsAndErrors";
Expand Down Expand Up @@ -75,10 +76,9 @@ internal static PackageSpec GetPackageSpec(JsonTextReader jsonReader, string pac
return GetPackageSpec(jsonReader, name: null, packageSpecPath, snapshotValue: null);
}

internal static PackageSpec GetPackageSpec(Stream stream, string name, string packageSpecPath, string snapshotValue, IEnvironmentVariableReader environmentVariableReader)
internal static PackageSpec GetPackageSpec(Stream stream, string name, string packageSpecPath, string snapshotValue, IEnvironmentVariableReader environmentVariableReader, bool bypassCache = false)
{
var useNj = environmentVariableReader.GetEnvironmentVariable("NUGET_EXPERIMENTAL_USE_NJ_FOR_FILE_PARSING");
if (string.IsNullOrEmpty(useNj) || useNj.Equals("false", StringComparison.OrdinalIgnoreCase))
if (!JsonUtility.UseNewstonSoftJsonForParsing(environmentVariableReader, bypassCache))
{
return GetPackageSpecUtf8JsonStreamReader(stream, name, packageSpecPath, snapshotValue);
}
Expand Down Expand Up @@ -306,7 +306,7 @@ private static void ReadCentralPackageVersions(
filePath);
}

centralPackageVersions[propertyName] = new CentralPackageVersion(propertyName, VersionRange.Parse(version));
centralPackageVersions[propertyName] = new CentralPackageVersion(propertyName, JsonUtility.ParseVersionRange(version));
});
}

Expand Down Expand Up @@ -449,7 +449,7 @@ private static void ReadDependencies(
{
try
{
versionOverride = VersionRange.Parse((string)jsonReader.Value);
versionOverride = JsonUtility.ParseVersionRange((string)jsonReader.Value);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -478,7 +478,7 @@ private static void ReadDependencies(
{
try
{
dependencyVersionRange = VersionRange.Parse(dependencyVersionValue);
dependencyVersionRange = JsonUtility.ParseVersionRange(dependencyVersionValue);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -620,7 +620,7 @@ internal static void ReadCentralTransitiveDependencyGroup(
{
try
{
dependencyVersionRange = VersionRange.Parse(dependencyVersionValue);
dependencyVersionRange = JsonUtility.ParseVersionRange(dependencyVersionValue);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -737,7 +737,7 @@ private static void ReadDownloadDependencies(
{
try
{
VersionRange version = VersionRange.Parse(singleVersionValue);
VersionRange version = JsonUtility.ParseVersionRange(singleVersionValue);

downloadDependencies.Add(new DownloadDependency(name, version));
}
Expand Down Expand Up @@ -1502,7 +1502,7 @@ static RuntimeDependencySet ReadRuntimeDependencySet(JsonTextReader jsonReader,
{
dependencies ??= new List<RuntimePackageDependency>();

var dependency = new RuntimePackageDependency(propertyName, VersionRange.Parse(jsonReader.ReadNextTokenAsString()));
var dependency = new RuntimePackageDependency(propertyName, JsonUtility.ParseVersionRange(jsonReader.ReadNextTokenAsString()));

dependencies.Add(dependency);
});
Expand Down
62 changes: 61 additions & 1 deletion src/NuGet.Core/NuGet.ProjectModel/JsonUtility.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,19 @@
using System.IO;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using NuGet.Common;
using NuGet.Packaging.Core;
using NuGet.Versioning;

namespace NuGet.ProjectModel
{
internal static class JsonUtility
{
private static readonly Dictionary<string, NuGetVersion> NuGetVersionCache = new();
private static readonly Dictionary<string, VersionRange> VersionRangeCache = new();

internal const string NUGET_EXPERIMENTAL_USE_NJ_FOR_FILE_PARSING = nameof(NUGET_EXPERIMENTAL_USE_NJ_FOR_FILE_PARSING);
internal static bool? UseNewtonsoftJson = null;
internal static readonly char[] PathSplitChars = new[] { LockFile.DirectorySeparatorChar };

/// <summary>
Expand Down Expand Up @@ -43,12 +49,35 @@ internal static JObject LoadJson(TextReader reader)
}
}

internal static T LoadJson<T>(Stream stream, IUtf8JsonStreamReaderConverter<T> converter)
{
var streamingJsonReader = new Utf8JsonStreamReader(stream);
return converter.Read(ref streamingJsonReader);
}

internal static PackageDependency ReadPackageDependency(string property, JToken json)
{
var versionStr = json.Value<string>();
return new PackageDependency(
property,
versionStr == null ? null : VersionRange.Parse(versionStr));
versionStr == null ? null : JsonUtility.ParseVersionRange(versionStr));
}

internal static bool UseNewstonSoftJsonForParsing(IEnvironmentVariableReader environmentVariableReader, bool bypassCache)
{
if (!UseNewtonsoftJson.HasValue || bypassCache)
{
if (bool.TryParse(environmentVariableReader.GetEnvironmentVariable(NUGET_EXPERIMENTAL_USE_NJ_FOR_FILE_PARSING), out var useNj))
{
UseNewtonsoftJson = useNj;
}
else
{
UseNewtonsoftJson = false;
}
}

return UseNewtonsoftJson.Value;
}

internal static JProperty WritePackageDependencyWithLegacyString(PackageDependency item)
Expand Down Expand Up @@ -141,5 +170,36 @@ internal static JToken WriteString(string item)
{
return item != null ? new JValue(item) : JValue.CreateNull();
}

internal static NuGetVersion ParseNugetVersion(string value)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

#5529 (comment)

@davkean @nkolev92 continuing this conversation here. Would we want the cache of version/version range to live only during the parsing of the file?

Copy link
Member

Choose a reason for hiding this comment

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

I think defining a great lifetime for these caches would be challenging.

I'm comfortable if this change starts with the assets file read scope, but we should look into expanding it.

Copy link
Member

Choose a reason for hiding this comment

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

The title of this PR is to migrate to System.Text.Json, and the existing Newtonsoft.Json code doesn't do caching, so can you bump caching work to a different PR?

I'm behind on my other priorities, but ideally I should start work on NuGet/Home#12124 in the next sprint or two. For that work, I need this PR merged into the feature branch, and the feature branch merged into the dev branch.

If merging the assets file System.Text.Json work is delayed, then that will delay my ability to start on this other work. Therefore, I'd like to minimize scope creep that blocks other work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I love that, so I went ahead and undid the caching stuff and saved a stash of it.

{
if (!NuGetVersionCache.ContainsKey(value))
{
var result = NuGetVersion.Parse(value);
NuGetVersionCache[value] = result;
}
return NuGetVersionCache[value];
}

internal static bool TryParseNugetVersion(string value, out NuGetVersion version)
{
if (!NuGetVersionCache.ContainsKey(value))
{
_ = NuGetVersion.TryParse(value, out version);
NuGetVersionCache[value] = version;
}
version = NuGetVersionCache[value];
return version is not null;
}

internal static VersionRange ParseVersionRange(string value)
{
if (!VersionRangeCache.ContainsKey(value))
{
var result = VersionRange.Parse(value);
VersionRangeCache[value] = result;
}
return VersionRangeCache[value];
}
}
}
138 changes: 138 additions & 0 deletions src/NuGet.Core/NuGet.ProjectModel/LazyStringSplit.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
#nullable enable

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;

namespace NuGet.ProjectModel
{
/// <summary>
/// Splits a string by a delimiter, producing substrings lazily during enumeration.
/// Skips empty items, behaving equivalently to <see cref="string.Split(char[])"/> with
/// <see cref="StringSplitOptions.RemoveEmptyEntries"/>.
/// </summary>
/// <remarks>
/// Unlike <see cref="string.Split(char[])"/> and overloads, <see cref="LazyStringSplit"/>
/// does not allocate an array for the return, and allocates strings on demand during
/// enumeration. A custom enumerator type is used so that the only allocations made are
/// the substrings themselves. We also avoid the large internal arrays assigned by the
/// methods on <see cref="string"/>.
/// </remarks>
internal readonly struct LazyStringSplit : IEnumerable<string>
{
private readonly string _input;
private readonly char _delimiter;

public LazyStringSplit(string input, char delimiter)
{
if (input is null)
{
throw new ArgumentNullException(nameof(input));
}

_input = input;
_delimiter = delimiter;
}

public Enumerator GetEnumerator() => new(this);

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

IEnumerator<string> IEnumerable<string>.GetEnumerator() => GetEnumerator();

public IEnumerable<T> Select<T>(Func<string, T> func)
{
foreach (string value in this)
{
yield return func(value);
}
}

public string First()
{
return FirstOrDefault() ?? throw new InvalidOperationException("Sequence is empty.");
}

public string? FirstOrDefault()
{
var enumerator = new Enumerator(this);
return enumerator.MoveNext() ? enumerator.Current : null;
}

public struct Enumerator : IEnumerator<string>
{
private readonly string _input;
private readonly char _delimiter;
private int _index;

internal Enumerator(in LazyStringSplit split)
{
_index = 0;
_input = split._input;
_delimiter = split._delimiter;
Current = null!;
}

public string Current { get; private set; }

public bool MoveNext()
{
while (_index != _input.Length)
{
int delimiterIndex = _input.IndexOf(_delimiter, _index);

if (delimiterIndex == -1)
{
Current = _input.Substring(_index);
_index = _input.Length;
return true;
}

int length = delimiterIndex - _index;

if (length == 0)
{
_index++;
continue;
}

Current = _input.Substring(_index, length);
_index = delimiterIndex + 1;
return true;
}

return false;
}

object IEnumerator.Current => Current;

void IEnumerator.Reset()
{
_index = 0;
Current = null!;
}

void IDisposable.Dispose() { }
}
}

internal static class LazyStringSplitExtensions
{
/// <remarks>
/// This extension method has special knowledge of the <see cref="LazyStringSplit"/> type and
/// can compute its result without allocation.
/// </remarks>
/// <inheritdoc cref="Enumerable.FirstOrDefault{TSource}(IEnumerable{TSource})"/>
public static string? FirstOrDefault(this LazyStringSplit lazyStringSplit)
{
LazyStringSplit.Enumerator enumerator = lazyStringSplit.GetEnumerator();

return enumerator.MoveNext()
? enumerator.Current
: null;
}
}
}
Loading