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

Sync less project attributes. #72930

Merged
merged 7 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -27,12 +27,12 @@ private sealed class AssetProvider(SerializationValidator validator) : AbstractA
public override async ValueTask<T> GetAssetAsync<T>(AssetPath assetPath, Checksum checksum, CancellationToken cancellationToken)
=> await validator.GetValueAsync<T>(checksum).ConfigureAwait(false);

public override async ValueTask GetAssetsAsync<T, TArg>(AssetPath assetPath, HashSet<Checksum> checksums, Action<Checksum, T, TArg> callback, TArg arg, CancellationToken cancellationToken)
public override async ValueTask GetAssetsAsync<T, TArg>(AssetPath assetPath, HashSet<Checksum> checksums, Action<Checksum, T, TArg>? callback, TArg? arg, CancellationToken cancellationToken) where TArg : default
{
foreach (var checksum in checksums)
{
var value = await GetAssetAsync<T>(assetPath, checksum, cancellationToken).ConfigureAwait(false);
callback(checksum, value, arg);
callback?.Invoke(checksum, value, arg!);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,6 @@ private AssetPath(AssetPathKind kind, ProjectId? projectId = null, DocumentId? d
_kind = kind;
ProjectId = projectId;
DocumentId = documentId;

// If this isn't a test lookup, and we're searching into projects or documents, then we must have at least a
// projectId to limit the search. If we don't, that risks very expensive searches where we look into *every*
// project in the solution for matches.
if ((kind & AssetPathKind.Testing) == 0)
{
if (IncludeProjects || IncludeDocuments)
Contract.ThrowIfNull(projectId);
}
}

public bool IncludeSolution => (_kind & AssetPathKind.Solution) == AssetPathKind.Solution;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,13 @@ public async Task FindAsync(
if (projectCone != null && !projectCone.Contains(projectId))
continue;

if (projectState.TryGetStateChecksums(out var projectStateChecksums) &&
searchingChecksumsLeft.Remove(projectStateChecksums.Checksum))
if (projectState.TryGetStateChecksums(out var projectStateChecksums))
{
result[projectStateChecksums.Checksum] = projectStateChecksums;
if (searchingChecksumsLeft.Remove(projectStateChecksums.Checksum))
result[projectStateChecksums.Checksum] = projectStateChecksums;

if (searchingChecksumsLeft.Remove(projectStateChecksums.Info))
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
result[projectStateChecksums.Info] = projectState.Attributes;
}
}
}
Expand Down Expand Up @@ -435,19 +438,15 @@ public async Task FindAsync(
if (assetPath.IncludeProjects)
{
if (searchingChecksumsLeft.Remove(Checksum))
{
result[Checksum] = this;
}

// It's normal for callers to just want to sync a single ProjectStateChecksum. So quickly check this, without
// doing all the expensive linear work below if we can bail out early here.
if (searchingChecksumsLeft.Count == 0)
return;

if (searchingChecksumsLeft.Remove(Info))
{
result[Info] = state.ProjectInfo.Attributes;
}

if (searchingChecksumsLeft.Remove(CompilationOptions))
{
Expand Down
2 changes: 1 addition & 1 deletion src/Workspaces/Remote/Core/AbstractAssetProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ internal abstract class AbstractAssetProvider
/// return data of type T whose checksum is the given checksum
/// </summary>
public abstract ValueTask<T> GetAssetAsync<T>(AssetPath assetPath, Checksum checksum, CancellationToken cancellationToken);
public abstract ValueTask GetAssetsAsync<T, TArg>(AssetPath assetPath, HashSet<Checksum> checksums, Action<Checksum, T, TArg> callback, TArg arg, CancellationToken cancellationToken);
public abstract ValueTask GetAssetsAsync<T, TArg>(AssetPath assetPath, HashSet<Checksum> checksums, Action<Checksum, T, TArg>? callback, TArg? arg, CancellationToken cancellationToken);

public async Task<SolutionInfo> CreateSolutionInfoAsync(Checksum solutionChecksum, CancellationToken cancellationToken)
{
Expand Down
5 changes: 3 additions & 2 deletions src/Workspaces/Remote/ServiceHub/Host/AssetProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Runtime.InteropServices;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Internal.Log;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Serialization;
using Microsoft.CodeAnalysis.Shared.Utilities;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Remote;
Expand Down Expand Up @@ -51,7 +52,7 @@ await this.SynchronizeAssetsAsync<T, ArrayBuilder<T>>(
}

public override async ValueTask GetAssetsAsync<T, TArg>(
AssetPath assetPath, HashSet<Checksum> checksums, Action<Checksum, T, TArg> callback, TArg arg, CancellationToken cancellationToken)
AssetPath assetPath, HashSet<Checksum> checksums, Action<Checksum, T, TArg>? callback, TArg? arg, CancellationToken cancellationToken) where TArg : default
{
await this.SynchronizeAssetsAsync(assetPath, checksums, callback, arg, cancellationToken).ConfigureAwait(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,15 @@ private async Task<Solution> UpdateProjectsAsync(
Dictionary<ProjectId, ProjectStateChecksums> newProjectIdToStateChecksums,
CancellationToken cancellationToken)
{
// Note: it's common to see a whole lot of project-infos change. So attempt to collect that in one go
// if we can.
using var _ = PooledHashSet<Checksum>.GetInstance(out var projectInfoChecksums);
foreach (var (projectId, newProjectChecksums) in newProjectIdToStateChecksums)
projectInfoChecksums.Add(newProjectChecksums.Info);

await _assetProvider.GetAssetsAsync<ProjectInfo.ProjectAttributes, VoidResult>(
assetPath: AssetPath.SolutionAndTopLevelProjectsOnly, projectInfoChecksums, callback: null, arg: default, cancellationToken).ConfigureAwait(false);

// added project
foreach (var (projectId, newProjectChecksums) in newProjectIdToStateChecksums)
{
Expand Down
Loading