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.

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

Replace JObjects in PackageMetadataResourceV3 with strong type for better performance, lower memory consumption #3462

Merged
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
b991d88
No need dictionary, same thing can be with HashSet which takes lower …
erdembayar Jun 16, 2020
e5e9622
Replacing slugish JsonObject with strong type.
erdembayar Jun 18, 2020
d491fb5
Revert back unneccessary change.
erdembayar Jun 18, 2020
00efe67
Add descriptions for new methods added.
erdembayar Jun 18, 2020
9ab130d
Add back MetadataReferenceCache cache just in case.
erdembayar Jun 18, 2020
d6629c3
Replace get single package version method too.
erdembayar Jun 18, 2020
597b0b7
Minor code readability change.
erdembayar Jun 18, 2020
598592d
Improve MetadataReferenceCache implementation.
erdembayar Jun 19, 2020
9453b79
Address code comment by Donnie.
erdembayar Jun 19, 2020
6aee560
Address after Andy's code review.
erdembayar Jun 19, 2020
708fd8f
Address more code review comments.
erdembayar Jun 19, 2020
1d572a4
Git push minor comment text change.
erdembayar Jun 19, 2020
8436bdd
Address code review by Nkolche
erdembayar Jun 24, 2020
c5ab81e
Fix space
erdembayar Jun 24, 2020
09c188b
Address more code review comments.
erdembayar Jun 24, 2020
44779ac
Address comment by Andy.
erdembayar Jun 25, 2020
17ce59a
Merge branch 'dev' into dev-eryondon-ReplaceJObjectsInmemoryForBetter…
erdembayar Jun 25, 2020
082f07f
Merge branch 'dev' into dev-eryondon-ReplaceJObjectsInmemoryForBetter…
erdembayar Jun 25, 2020
1158fb1
Address more comments by Nkolche.
erdembayar Jun 26, 2020
ca867ed
Merge branch 'dev' into dev-eryondon-ReplaceJObjectsInmemoryForBetter…
erdembayar Jun 26, 2020
a2e28fd
Address new comment by Andy.
erdembayar Jun 26, 2020
84eed9c
Address another comment by Andy for ValueTuple.
erdembayar Jun 27, 2020
ddad073
Forgot to delete file RegistrationIndexResult.cs which no longer need.
erdembayar Jun 27, 2020
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 @@ -3,14 +3,13 @@

using System;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Newtonsoft.Json.Linq;
using NuGet.Common;
using NuGet.Protocol.Core.Types;
using NuGet.Protocol.Extensions;
using NuGet.Versioning;

namespace NuGet.Protocol
Expand Down Expand Up @@ -70,7 +69,7 @@ public async static Task<IEnumerable<JObject>> LoadRanges(
var lower = NuGetVersion.Parse(item["lower"].ToString());
var upper = NuGetVersion.Parse(item["upper"].ToString());

if (IsItemRangeRequired(range, lower, upper))
if (range.DoesRangeSatisfies(lower, upper))
{
JToken items;
if (!item.TryGetValue("items", out items))
Expand Down Expand Up @@ -103,20 +102,5 @@ public async static Task<IEnumerable<JObject>> LoadRanges(

return rangeTasks.Select((t) => t.Result);
}

private static bool IsItemRangeRequired(VersionRange dependencyRange, NuGetVersion catalogItemLower, NuGetVersion catalogItemUpper)
{
var catalogItemVersionRange = new VersionRange(minVersion: catalogItemLower, includeMinVersion: true,
maxVersion: catalogItemUpper, includeMaxVersion: true);

if (dependencyRange.HasLowerAndUpperBounds) // Mainly to cover the '!dependencyRange.IsMaxInclusive && !dependencyRange.IsMinInclusive' case
{
return catalogItemVersionRange.Satisfies(dependencyRange.MinVersion) || catalogItemVersionRange.Satisfies(dependencyRange.MaxVersion);
}
else
{
return dependencyRange.Satisfies(catalogItemLower) || dependencyRange.Satisfies(catalogItemUpper);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// 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.

using NuGet.Versioning;

namespace NuGet.Protocol.Extensions
{
internal static class VersionRangeExtensions
{
public static bool DoesRangeSatisfies(this VersionRange dependencyRange, NuGetVersion catalogItemLower, NuGetVersion catalogItemUpper)
erdembayar marked this conversation as resolved.
Show resolved Hide resolved
{
if (dependencyRange.HasLowerAndUpperBounds) // Mainly to cover the '!dependencyRange.IsMaxInclusive && !dependencyRange.IsMinInclusive' case
{
var catalogItemVersionRange = new VersionRange(minVersion: catalogItemLower, includeMinVersion: true,
maxVersion: catalogItemUpper, includeMaxVersion: true);

return catalogItemVersionRange.Satisfies(dependencyRange.MinVersion) || catalogItemVersionRange.Satisfies(dependencyRange.MaxVersion);
}
else
{
return dependencyRange.Satisfies(catalogItemLower) || dependencyRange.Satisfies(catalogItemUpper);
}
}
}
}
2 changes: 2 additions & 0 deletions src/NuGet.Core/NuGet.Protocol/GlobalSuppressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -301,3 +301,5 @@
[assembly: SuppressMessage("Build", "CA1001:Type 'ServiceIndexResourceV3Provider' owns disposable field(s) '_semaphore' but is not disposable", Justification = "<Pending>", Scope = "type", Target = "~T:NuGet.Protocol.ServiceIndexResourceV3Provider")]
[assembly: SuppressMessage("Build", "CA1052:Type 'V2FeedUtilities' is a static holder type but is neither static nor NotInheritable", Justification = "<Pending>", Scope = "type", Target = "~T:NuGet.Protocol.V2FeedUtilities")]
[assembly: SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase", Justification = "We intentionally making to lower case because we're creating web api query.", Scope = "member", Target = "~M:NuGet.Protocol.PackageSearchResourceV3.SearchPage``1(System.Func{System.Uri,System.Threading.Tasks.Task{``0}},System.String,NuGet.Protocol.Core.Types.SearchFilter,System.Int32,System.Int32,NuGet.Common.ILogger,System.Threading.CancellationToken)~System.Threading.Tasks.Task{``0}")]
[assembly: SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase", Justification = "We convert packageId to lower case for api call.", Scope = "member", Target = "~M:NuGet.Protocol.PackageMetadataResourceV3.LoadRegistrationIndexAsync(NuGet.Protocol.HttpSource,System.Uri,System.String,NuGet.Protocol.Core.Types.SourceCacheContext,System.Func{NuGet.Protocol.HttpSourceResult,System.Threading.Tasks.Task{NuGet.Protocol.Model.RegistrationIndex}},NuGet.Common.ILogger,System.Threading.CancellationToken)~System.Threading.Tasks.Task{NuGet.Protocol.Model.RegistrationIndexResult}")]
[assembly: SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase", Justification = "We convert packageId to lower case for api call.", Scope = "member", Target = "~M:NuGet.Protocol.PackageMetadataResourceV3.GetRegistratioIndexPageAsync(NuGet.Protocol.HttpSource,System.String,System.String,NuGet.Versioning.NuGetVersion,NuGet.Versioning.NuGetVersion,NuGet.Protocol.Core.Types.HttpSourceCacheContext,NuGet.Common.ILogger,System.Threading.CancellationToken)~System.Threading.Tasks.Task{NuGet.Protocol.Model.RegistrationPage}")]
17 changes: 17 additions & 0 deletions src/NuGet.Core/NuGet.Protocol/Model/RegistrationIndex.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// 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.

using System.Collections.Generic;
using Newtonsoft.Json;

namespace NuGet.Protocol.Model
{
/// <summary>
/// Source: https://docs.microsoft.com/en-us/nuget/api/registration-base-url-resource#registration-index
/// </summary>
internal class RegistrationIndex
{
[JsonProperty("items")]
public List<RegistrationPage> Items { get; set; }
}
}
13 changes: 13 additions & 0 deletions src/NuGet.Core/NuGet.Protocol/Model/RegistrationIndexResult.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// 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.

using NuGet.Protocol.Core.Types;

namespace NuGet.Protocol.Model
{
internal class RegistrationIndexResult
erdembayar marked this conversation as resolved.
Show resolved Hide resolved
{
public RegistrationIndex Index { get; set; }
public HttpSourceCacheContext CacheContext { get; set; }
}
}
16 changes: 16 additions & 0 deletions src/NuGet.Core/NuGet.Protocol/Model/RegistrationLeafItem.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// 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.

using Newtonsoft.Json;

namespace NuGet.Protocol.Model
{
/// <summary>
/// Source: https://docs.microsoft.com/en-us/nuget/api/registration-base-url-resource#registration-leaf-object-in-a-page
/// </summary>
internal class RegistrationLeafItem
{
[JsonProperty("catalogEntry")]
public PackageSearchMetadataRegistration CatalogEntry { get; set; }
}
}
33 changes: 33 additions & 0 deletions src/NuGet.Core/NuGet.Protocol/Model/RegistrationPage.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// 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.

using System.Collections.Generic;
using Newtonsoft.Json;
namespace NuGet.Protocol.Model
{
/// <summary>
/// This model is used for both the registration page item (found in a registration index) and for a registration
/// page fetched on its own.
/// Source: https://docs.microsoft.com/en-us/nuget/api/registration-base-url-resource#registration-page
/// Source: https://docs.microsoft.com/en-us/nuget/api/registration-base-url-resource#registration-page-object
/// </summary>
internal class RegistrationPage
{
[JsonProperty("@id")]
public string Url { get; set; }

/// <summary>
/// This property can be null when this model is used as an item in <see cref="RegistrationIndex.Items"/> when
/// the server decided not to inline the leaf items. In this case, the <see cref="Url"/> property can be used
/// fetch another <see cref="RegistrationPage"/> instance with the <see cref="Items"/> property filled in.
/// </summary>
[JsonProperty("items")]
public List<RegistrationLeafItem> Items { get; set; }

[JsonProperty("lower")]
public string Lower { get; set; }

[JsonProperty("upper")]
public string Upper { get; set; }
}
}
Loading