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

Switch to using index operator #72911

Merged
merged 43 commits into from
Apr 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
2975b86
improve asset hints to speed up searching
CyrusNajmabadi Apr 6, 2024
9204f25
update pinning
CyrusNajmabadi Apr 6, 2024
51681b6
in progress
CyrusNajmabadi Apr 6, 2024
f685cb1
in progress
CyrusNajmabadi Apr 6, 2024
c04ebe7
In progress
CyrusNajmabadi Apr 6, 2024
3290c29
special test value
CyrusNajmabadi Apr 6, 2024
1a2ee5a
fix'
CyrusNajmabadi Apr 6, 2024
df48653
Update tests
CyrusNajmabadi Apr 6, 2024
60bdda6
Update docs
CyrusNajmabadi Apr 6, 2024
b76ced5
fix
CyrusNajmabadi Apr 6, 2024
d312052
in progress
CyrusNajmabadi Apr 6, 2024
65befaa
Simplify
CyrusNajmabadi Apr 6, 2024
9b71463
Simplify
CyrusNajmabadi Apr 6, 2024
aa3c405
Simplify
CyrusNajmabadi Apr 6, 2024
7163426
comment
CyrusNajmabadi Apr 6, 2024
616478a
restore
CyrusNajmabadi Apr 6, 2024
4a27911
docs
CyrusNajmabadi Apr 6, 2024
5a20b41
Simplify
CyrusNajmabadi Apr 6, 2024
d729f63
lint
CyrusNajmabadi Apr 6, 2024
502fccb
remove
CyrusNajmabadi Apr 6, 2024
63665c9
Switch to using index operator
CyrusNajmabadi Apr 6, 2024
98eec08
revert
CyrusNajmabadi Apr 6, 2024
39900f7
make readonly
CyrusNajmabadi Apr 6, 2024
800ed1d
rename type
CyrusNajmabadi Apr 6, 2024
805d0ea
rename type
CyrusNajmabadi Apr 6, 2024
4b7c554
Use enum
CyrusNajmabadi Apr 6, 2024
dfe6eec
revert
CyrusNajmabadi Apr 7, 2024
f32087a
Slight change to AssetPath
ToddGrun Apr 7, 2024
f30f0ae
Feedback
CyrusNajmabadi Apr 7, 2024
30a40a7
Merge branch 'main' into indexOperator
CyrusNajmabadi Apr 7, 2024
11e6ec2
shift left
CyrusNajmabadi Apr 7, 2024
8f007b6
IN progress
CyrusNajmabadi Apr 7, 2024
0e1ef0a
fixups
CyrusNajmabadi Apr 7, 2024
241a51d
Cleanup
CyrusNajmabadi Apr 7, 2024
e268200
Docs
CyrusNajmabadi Apr 7, 2024
6b1802d
Docs
CyrusNajmabadi Apr 7, 2024
d63de5a
Add docs
CyrusNajmabadi Apr 7, 2024
ab2ba2e
Bulk sync
CyrusNajmabadi Apr 7, 2024
6d4249b
Simplify
CyrusNajmabadi Apr 7, 2024
b604aa5
Merge remote-tracking branch 'upstream/main' into assetHints
CyrusNajmabadi Apr 7, 2024
00e0831
Doc
CyrusNajmabadi Apr 7, 2024
47c491f
Doc
CyrusNajmabadi Apr 7, 2024
e4d590e
Merge branch 'assetHints' into indexOperator
CyrusNajmabadi Apr 7, 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 @@ -344,7 +344,7 @@ private static void CheckCharacters(VirtualCharSequence virtualChars, ref int po

private static string And(params string[] regexes)
{
var conj = $"({regexes[regexes.Length - 1]})";
var conj = $"({regexes[^1]})";
for (var i = regexes.Length - 2; i >= 0; i--)
conj = $"(?({regexes[i]}){conj}|[0-[0]])";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ private static async Task<ContainerElement> BuildInteractiveContentAsync(

// Stack the first paragraph of the documentation comments with the last line of the description
// to avoid vertical padding between the two.
var lastElement = elements[elements.Count - 1];
elements[elements.Count - 1] = new ContainerElement(
var lastElement = elements[^1];
elements[^1] = new ContainerElement(
ContainerElementStyle.Stacked,
lastElement,
element);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ private static void ProcessNodeList<T>(SyntaxList<T> children, ArrayBuilder<Text
{
if (!seenSeparator)
{
var nextToLast = children[children.Count - 2];
var nextToLast = children[^2];
AddLineSeparatorSpanForNode(nextToLast, spans, cancellationToken);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public RegexBraceMatcher()
return null;

var firstChar = trivia.Value.VirtualChars[0];
var lastChar = trivia.Value.VirtualChars[trivia.Value.VirtualChars.Length - 1];
var lastChar = trivia.Value.VirtualChars[^1];
return firstChar != '(' || lastChar != ')'
? null
: new BraceMatchingResult(firstChar.Span, lastChar.Span);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public string GetDisplayName()

public bool HasKeyName()
{
return Index >= 0 && Name != null && Name.Length >= 2 && Name[0] == '[' && Name[Name.Length - 1] == ']';
return Index >= 0 && Name != null && Name.Length >= 2 && Name[0] == '[' && Name[^1] == ']';
}

public bool AppendAsCollectionEntry(Builder result)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ private void FormatMultidimensionalArrayElements(Builder result, Array array, bo
string _;
FormatObjectRecursive(result, array.GetValue(indices), isRoot: false, debuggerDisplayName: out _);

indices[indices.Length - 1]++;
indices[^1]++;
flatIndex++;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/VisualStudio/Core/Def/Library/VsNavInfo/NavInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public NavInfo(
_basePresentationNodes = CreateNodes(expandDottedNames: false);

_symbolType = _basePresentationNodes.Length > 0
? _basePresentationNodes[_basePresentationNodes.Length - 1].ListType
? _basePresentationNodes[^1].ListType
: 0;
}

Expand Down
2 changes: 1 addition & 1 deletion src/VisualStudio/Core/Def/Preview/FileChange.cs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ private static string GetDisplayText(string excerpt)
var split = excerpt.Split(new[] { "\r\n" }, StringSplitOptions.RemoveEmptyEntries);
if (split.Length > 1)
{
return string.Format("{0} ... {1}", split[0].Trim(), split[split.Length - 1].Trim());
return string.Format("{0} ... {1}", split[0].Trim(), split[^1].Trim());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ private TreeViewItemBase GetPreviousItem()
{
if (ValueTrackingTreeView.SelectedItem is null)
{
return (TreeViewItemBase)ValueTrackingTreeView.Items[ValueTrackingTreeView.Items.Count - 1];
return (TreeViewItemBase)ValueTrackingTreeView.Items[^1];
}

var item = (TreeViewItemBase)ValueTrackingTreeView.SelectedItem;
Expand Down
2 changes: 1 addition & 1 deletion src/VisualStudio/Core/Def/Venus/ContainedDocument.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1073,7 +1073,7 @@ private bool IsCodeBlock(ITextSnapshot surfaceSnapshot, int position, char ch)

private static bool CheckCode(ITextSnapshot snapshot, int position, char ch, string tag, bool checkAt = true)
{
if (ch != tag[tag.Length - 1] || position < tag.Length)
if (ch != tag[^1] || position < tag.Length)
{
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ protected abstract class AbstractNodeNameGenerator
protected static void AppendDotIfNeeded(StringBuilder builder)
{
if (builder.Length > 0 &&
char.IsLetterOrDigit(builder[builder.Length - 1]))
char.IsLetterOrDigit(builder[^1]))
{
builder.Append('.');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Serialization;
using Microsoft.VisualStudio.PlatformUI;
using Roslyn.Test.Utilities;
Expand All @@ -21,15 +22,23 @@ namespace Microsoft.CodeAnalysis.Remote.UnitTests
{
internal sealed class SerializationValidator
{
private sealed class AssetProvider : AbstractAssetProvider
private sealed class AssetProvider(SerializationValidator validator) : AbstractAssetProvider
{
private readonly SerializationValidator _validator;
public override async ValueTask<T> GetAssetAsync<T>(AssetPath assetPath, Checksum checksum, CancellationToken cancellationToken)
=> await validator.GetValueAsync<T>(checksum).ConfigureAwait(false);

public AssetProvider(SerializationValidator validator)
=> _validator = validator;
public override async ValueTask<ImmutableArray<(Checksum checksum, T asset)>> GetAssetsAsync<T>(AssetPath assetPath, HashSet<Checksum> checksums, CancellationToken cancellationToken)
{
using var _ = ArrayBuilder<(Checksum checksum, T asset)>.GetInstance(out var result);

foreach (var checksum in checksums)
{
var value = await GetAssetAsync<T>(assetPath, checksum, cancellationToken).ConfigureAwait(false);
result.Add((checksum, value));
}

public override async ValueTask<T> GetAssetAsync<T>(AssetHint assetHint, Checksum checksum, CancellationToken cancellationToken)
=> await _validator.GetValueAsync<T>(checksum).ConfigureAwait(false);
return result.ToImmutable();
}
}

internal sealed class ChecksumObjectCollection<T> : IEnumerable<T>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ private static async Task TestAssetAsync(object data)
var assetSource = new SimpleAssetSource(workspace.Services.GetService<ISerializerService>(), new Dictionary<Checksum, object>() { { checksum, data } });

var provider = new AssetProvider(sessionId, storage, assetSource, remoteWorkspace.Services.GetService<ISerializerService>());
var stored = await provider.GetAssetAsync<object>(assetHint: AssetHint.None, checksum, CancellationToken.None);
var stored = await provider.GetAssetAsync<object>(AssetPath.FullLookupForTesting, checksum, CancellationToken.None);
Assert.Equal(data, stored);

var stored2 = await provider.GetAssetsAsync<object>(assetHint: AssetHint.None, [checksum], CancellationToken.None);
var stored2 = await provider.GetAssetsAsync<object>(AssetPath.FullLookupForTesting, new HashSet<Checksum> { checksum }, CancellationToken.None);
Assert.Equal(1, stored2.Length);

Assert.Equal(checksum, stored2[0].Item1);
Expand All @@ -83,7 +83,7 @@ public async Task TestAssetSynchronization()
var assetSource = new SimpleAssetSource(workspace.Services.GetService<ISerializerService>(), map);

var service = new AssetProvider(sessionId, storage, assetSource, remoteWorkspace.Services.GetService<ISerializerService>());
await service.SynchronizeAssetsAsync(assetHint: AssetHint.None, new HashSet<Checksum>(map.Keys), results: null, CancellationToken.None);
await service.SynchronizeAssetsAsync(AssetPath.FullLookupForTesting, new HashSet<Checksum>(map.Keys), results: null, CancellationToken.None);

foreach (var kv in map)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ private async Task UpdatePathsToRemoteFilesAsync(CollaborationSession session)
#pragma warning disable CS8602 // Dereference of a possibly null reference. (Can localRoot be null here?)
var splitRoot = localRoot.TrimEnd('\\').Split('\\');
#pragma warning restore CS8602 // Dereference of a possibly null reference.
splitRoot[splitRoot.Length - 1] = "~external";
splitRoot[^1] = "~external";
var externalPath = string.Join("\\", splitRoot) + "\\";

remoteRootPaths.Add(localRoot);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,13 @@ internal static class DocumentBasedFixAllProviderHelpers

// TODO: consider computing this in parallel.
var singleContextDocIdToNewRootOrText = await getFixedDocumentsAsync(fixAllContext, progressTracker).ConfigureAwait(false);
allContextsDocIdToNewRootOrText.AddRange(singleContextDocIdToNewRootOrText);

// Note: it is safe to blindly add the dictionary for a particular context to the full dictionary. Each
// dictionary will only update documents within that context, and each context represents a distinct
// project, so these should all be distinct without collisions. However, to be very safe, we use an
// overwriting policy here to ensure nothing causes any problems here.
foreach (var kvp in singleContextDocIdToNewRootOrText)
allContextsDocIdToNewRootOrText[kvp.Key] = kvp.Value;
}
}

Expand Down
106 changes: 98 additions & 8 deletions src/Workspaces/Core/Portable/Workspace/Solution/AssetHint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,120 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Runtime.Serialization;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Serialization;

/// <summary>
/// Optional information passed with an asset synchronization request to allow the request to be scoped down to a
/// particular <see cref="Project"/> or <see cref="Document"/>.
/// Required information passed with an asset synchronization request to tell the host where to scope the request to. In
/// particular, this is often used to scope to a particular <see cref="Project"/> or <see cref="Document"/> to avoid
/// having to search the entire solution.
/// </summary>
[DataContract]
internal readonly struct AssetHint
internal readonly struct AssetPath
{
public static readonly AssetHint None = default;
/// <summary>
/// Instance that will only look up solution-level data when searching for checksums.
/// </summary>
public static readonly AssetPath SolutionOnly = new(AssetPathKind.Solution);

/// <summary>
/// Instance that will only look up solution-level, as well as the top level nodes for projects when searching for
/// checksums. It will not descend into projects.
/// </summary>
public static readonly AssetPath SolutionAndTopLevelProjectsOnly = new(AssetPathKind.Solution | AssetPathKind.TopLevelProjects);

/// <summary>
/// Special instance, allowed only in tests/debug-asserts, that can do a full lookup across the entire checksum
/// tree. Should not be used in normal release-mode product code.
/// </summary>
public static readonly AssetPath FullLookupForTesting = new(AssetPathKind.Solution | AssetPathKind.TopLevelProjects | AssetPathKind.Projects | AssetPathKind.Documents | AssetPathKind.Testing);

[DataMember(Order = 0)]
public readonly ProjectId? ProjectId;
private readonly AssetPathKind _kind;
[DataMember(Order = 1)]
public readonly ProjectId? ProjectId;
[DataMember(Order = 2)]
public readonly DocumentId? DocumentId;

private AssetHint(ProjectId? projectId, DocumentId? documentId)
private AssetPath(AssetPathKind kind, ProjectId? projectId = null, DocumentId? documentId = null)
{
_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 static implicit operator AssetHint(ProjectId projectId) => new(projectId, documentId: null);
public static implicit operator AssetHint(DocumentId documentId) => new(documentId.ProjectId, documentId);
public bool IncludeSolution => (_kind & AssetPathKind.Solution) == AssetPathKind.Solution;
public bool IncludeTopLevelProjects => (_kind & AssetPathKind.TopLevelProjects) == AssetPathKind.TopLevelProjects;
public bool IncludeProjects => (_kind & AssetPathKind.Projects) == AssetPathKind.Projects;
public bool IncludeDocuments => (_kind & AssetPathKind.Documents) == AssetPathKind.Documents;

/// <summary>
/// Searches only for information about this project.
/// </summary>
public static implicit operator AssetPath(ProjectId projectId) => new(AssetPathKind.Projects, projectId, documentId: null);

/// <summary>
/// Searches only for information about this document.
/// </summary>
public static implicit operator AssetPath(DocumentId documentId) => new(AssetPathKind.Documents, documentId.ProjectId, documentId);

/// <summary>
/// Searches the requested project, and all documents underneath it. Used only in tests.
/// </summary>
/// <param name="projectId"></param>
/// <returns></returns>
public static AssetPath SolutionAndProjectForTesting(ProjectId projectId)
=> new(AssetPathKind.Solution | AssetPathKind.Projects | AssetPathKind.Testing, projectId);

/// <summary>
/// Searches the requested project, and all documents underneath it. used during normal sync when bulk syncing a
/// project.
/// </summary>
/// <param name="projectId"></param>
/// <returns></returns>
public static AssetPath ProjectAndDocuments(ProjectId projectId)
=> new(AssetPathKind.Projects | AssetPathKind.Documents, projectId);

[Flags]
private enum AssetPathKind
{
/// <summary>
/// Search solution-level information.
/// </summary>
Solution = 1 << 0,

/// <summary>
/// Search projects, without descending into them. In effect, only finding direct ProjectStateChecksum children
/// of the solution.
/// </summary>
TopLevelProjects = 1 << 1,

/// <summary>
/// Search projects for results.
/// </summary>
Projects = 1 << 2,

/// <summary>
/// Search documents for results.
/// </summary>
Documents = 1 << 3,

/// <summary>
/// Indicates that this is a special search performed during testing. These searches are allowed to search
/// everything for expediency purposes.
/// </summary>
Testing = 1 << 4,
}
}
Loading
Loading