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

Use ITextDifferencingService.DiffSnapshotSpans instead of allocating full buffer text during codeaction previews #74114

Merged
merged 4 commits into from
Jun 26, 2024
+52 −30
Merged

Use ITextDifferencingService.DiffSnapshotSpans instead of allocating full buffer text during codeaction previews #74114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9d9c38b
Use DiffSnapshotSpans instead of allocating full buffer text during c…
ToddGrun Jun 22, 2024
db2a0d5
use extension method
ToddGrun Jun 26, 2024
b6d4bc5
Move TextDifferencingServiceExtensions to a more applicable location
ToddGrun Jun 26, 2024
df6ff92
Merge branch 'dotnet:main' into dev/toddgrun/UseDiffSnapshotSpans
ToddGrun Jun 26, 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
12 changes: 8 additions & 4 deletions src/EditorFeatures/Core/Preview/AbstractPreviewFactoryService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.Editor.Implementation.TextDiffing;
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.Editor.Shared.Preview;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
Expand Down Expand Up @@ -56,6 +57,11 @@ internal abstract class AbstractPreviewFactoryService<TDifferenceViewer>(
private readonly IDifferenceBufferFactoryService _differenceBufferService = differenceBufferService;
private readonly ITextDocumentFactoryService _textDocumentFactoryService = textDocumentFactoryService;

private static readonly StringDifferenceOptions s_differenceOptions = new()
{
DifferenceType = StringDifferenceTypes.Word | StringDifferenceTypes.Line,
};

protected readonly IThreadingContext ThreadingContext = threadingContext;

public SolutionPreviewResult? GetSolutionPreviews(Solution oldSolution, Solution? newSolution, CancellationToken cancellationToken)
Expand Down Expand Up @@ -778,10 +784,8 @@ private IHierarchicalDifferenceCollection ComputeEditDifferences(TextDocument ol
oldDocument.Project.Services.GetRequiredService<IContentTypeLanguageService>().GetDefaultContentType());

diffService ??= _differenceSelectorService.DefaultTextDifferencingService;
return diffService.DiffStrings(oldText.ToString(), newText.ToString(), new StringDifferenceOptions()
{
DifferenceType = StringDifferenceTypes.Word | StringDifferenceTypes.Line,
});

return diffService.DiffSourceTexts(oldText, newText, s_differenceOptions);
}

private static NormalizedSpanCollection GetOriginalSpans(IHierarchicalDifferenceCollection diffResult, CancellationToken cancellationToken)
Expand Down
9 changes: 1 addition & 8 deletions src/EditorFeatures/Core/TextDiffing/EditorTextDifferencingService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Text;
using Microsoft.CodeAnalysis.Text.Shared.Extensions;
using Microsoft.VisualStudio.Text.Differencing;

namespace Microsoft.CodeAnalysis.Editor.Implementation.TextDiffing;
Expand All @@ -38,13 +37,7 @@ public async Task<ImmutableArray<TextChange>> GetTextChangesAsync(Document oldDo

var differenceOptions = GetDifferenceOptions(preferredDifferenceType);

var oldTextSnapshot = oldText.FindCorrespondingEditorTextSnapshot();
var newTextSnapshot = newText.FindCorrespondingEditorTextSnapshot();
var useSnapshots = oldTextSnapshot != null && newTextSnapshot != null;

var diffResult = useSnapshots
? diffService.DiffSnapshotSpans(oldTextSnapshot.GetFullSpan(), newTextSnapshot.GetFullSpan(), differenceOptions)
: diffService.DiffStrings(oldText.ToString(), newText.ToString(), differenceOptions);
var diffResult = diffService.DiffSourceTexts(oldText, newText, differenceOptions);

return diffResult.Differences.Select(d =>
new TextChange(
Expand Down
25 changes: 25 additions & 0 deletions src/EditorFeatures/Core/TextDiffing/TextDifferencingServiceExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using Microsoft.CodeAnalysis.Text;
using Microsoft.CodeAnalysis.Text.Shared.Extensions;
using Microsoft.VisualStudio.Text.Differencing;

namespace Microsoft.CodeAnalysis.Editor.Implementation.TextDiffing;

internal static class TextDifferencingServiceExtensions
{
public static IHierarchicalDifferenceCollection DiffSourceTexts(this ITextDifferencingService diffService, SourceText oldText, SourceText newText, StringDifferenceOptions options)
{
var oldTextSnapshot = oldText.FindCorrespondingEditorTextSnapshot();
var newTextSnapshot = newText.FindCorrespondingEditorTextSnapshot();
var useSnapshots = oldTextSnapshot != null && newTextSnapshot != null;

var diffResult = useSnapshots
? diffService.DiffSnapshotSpans(oldTextSnapshot!.GetFullSpan(), newTextSnapshot!.GetFullSpan(), options)
: diffService.DiffStrings(oldText.ToString(), newText.ToString(), options);

return diffResult;
}
}
22 changes: 11 additions & 11 deletions src/EditorFeatures/Text/Extensions.SnapshotSourceText.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ private class SnapshotSourceText : SourceText
private readonly Encoding? _encoding;
private readonly TextBufferContainer? _container;

private SnapshotSourceText(ITextBufferCloneService? textBufferCloneService, ITextSnapshot editorSnapshot, SourceHashAlgorithm checksumAlgorithm, TextBufferContainer container)
private SnapshotSourceText(ITextBufferCloneService? textBufferCloneService, ITextSnapshot editorSnapshot, Encoding? encoding, SourceHashAlgorithm checksumAlgorithm, TextBufferContainer container)
Copy link
Member

Choose a reason for hiding this comment

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

i'm missing the callsite that passed in a non-null value here. or was this just to make it explicit that all callsites are trying to pass that in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ChangedSourceText.ctor was previously calling the ITextImage version of this ctor, and passing in an encoding. I wanted to match that behavior.

: base(checksumAlgorithm: checksumAlgorithm)
{
Contract.ThrowIfNull(editorSnapshot);

_textBufferCloneService = textBufferCloneService;
this.TextImage = RecordReverseMapAndGetImage(editorSnapshot);
_encoding = editorSnapshot.TextBuffer.GetEncodingOrUTF8();
_encoding = encoding ?? editorSnapshot.TextBuffer.GetEncodingOrUTF8();
_container = container;
}

Expand Down Expand Up @@ -83,7 +83,7 @@ public static SourceText From(ITextBufferCloneService? textBufferCloneService, I

// Avoid capturing `textBufferCloneServiceOpt` on the fast path
var tempTextBufferCloneService = textBufferCloneService;
snapshot = s_textSnapshotMap.GetValue(editorSnapshot, s => new SnapshotSourceText(tempTextBufferCloneService, s, SourceHashAlgorithms.OpenDocumentChecksumAlgorithm, container));
snapshot = s_textSnapshotMap.GetValue(editorSnapshot, s => new SnapshotSourceText(tempTextBufferCloneService, s, encoding: null, SourceHashAlgorithms.OpenDocumentChecksumAlgorithm, container));
}

return snapshot;
Expand All @@ -100,7 +100,7 @@ internal static SourceText From(ITextBufferCloneService? textBufferCloneService,
}

Contract.ThrowIfFalse(editorSnapshot.TextBuffer == container.GetTextBuffer());
return s_textSnapshotMap.GetValue(editorSnapshot, s => new SnapshotSourceText(textBufferCloneService, s, SourceHashAlgorithms.OpenDocumentChecksumAlgorithm, container));
return s_textSnapshotMap.GetValue(editorSnapshot, s => new SnapshotSourceText(textBufferCloneService, s, encoding: null, SourceHashAlgorithms.OpenDocumentChecksumAlgorithm, container));
}

public override Encoding? Encoding
Expand Down Expand Up @@ -220,8 +220,8 @@ public override SourceText WithChanges(IEnumerable<TextChange> changes)
return new ChangedSourceText(
textBufferCloneService: _textBufferCloneService,
baseText: this,
baseSnapshot: ((ITextSnapshot2)baseSnapshot).TextImage,
currentSnapshot: ((ITextSnapshot2)buffer.CurrentSnapshot).TextImage);
baseSnapshot: baseSnapshot,
currentSnapshot: buffer.CurrentSnapshot);
}

private static ITextImage RecordReverseMapAndGetImage(ITextSnapshot editorSnapshot)
Expand Down Expand Up @@ -276,13 +276,13 @@ public ClosedSnapshotSourceText(ITextBufferCloneService? textBufferCloneService,
private class ChangedSourceText : SnapshotSourceText
{
private readonly SnapshotSourceText _baseText;
private readonly ITextImage _baseSnapshot;
private readonly ITextImage _baseTextImage;

public ChangedSourceText(ITextBufferCloneService? textBufferCloneService, SnapshotSourceText baseText, ITextImage baseSnapshot, ITextImage currentSnapshot)
: base(textBufferCloneService, currentSnapshot, baseText.Encoding, baseText.ChecksumAlgorithm, container: null)
public ChangedSourceText(ITextBufferCloneService? textBufferCloneService, SnapshotSourceText baseText, ITextSnapshot baseSnapshot, ITextSnapshot currentSnapshot)
: base(textBufferCloneService, currentSnapshot, baseText.Encoding, baseText.ChecksumAlgorithm, container: TextBufferContainer.From(currentSnapshot.TextBuffer))
{
_baseText = baseText;
_baseSnapshot = baseSnapshot;
_baseTextImage = ((ITextSnapshot2)baseSnapshot).TextImage;
}

public override IReadOnlyList<TextChangeRange> GetChangeRanges(SourceText oldText)
Expand All @@ -303,7 +303,7 @@ public override IReadOnlyList<TextChangeRange> GetChangeRanges(SourceText oldTex
return [new TextChangeRange(new TextSpan(0, oldText.Length), this.Length)];
}

return GetChangeRanges(_baseSnapshot, _baseSnapshot.Length, this.TextImage);
return GetChangeRanges(_baseTextImage, _baseTextImage.Length, this.TextImage);
}
}

Expand Down
14 changes: 7 additions & 7 deletions src/VisualStudio/Core/Def/Preview/FileChange.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Threading;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Editor;
using Microsoft.CodeAnalysis.Editor.Implementation.TextDiffing;
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Text;
Expand All @@ -32,6 +33,11 @@ internal class FileChange : AbstractChange
private readonly ITextBuffer _buffer;
private readonly IVsImageService2 _imageService;

private static readonly StringDifferenceOptions s_differenceOptions = new()
{
DifferenceType = StringDifferenceTypes.Line,
};

public FileChange(TextDocument left,
TextDocument right,
IComponentModel componentModel,
Expand Down Expand Up @@ -238,12 +244,6 @@ private static IHierarchicalDifferenceCollection ComputeDiffSpans(ITextDifferenc
var oldText = left.GetTextSynchronously(cancellationToken);
var newText = right.GetTextSynchronously(cancellationToken);

var oldString = oldText.ToString();
var newString = newText.ToString();

return diffService.DiffStrings(oldString, newString, new StringDifferenceOptions()
{
DifferenceType = StringDifferenceTypes.Line,
});
return diffService.DiffSourceTexts(oldText, newText, s_differenceOptions);
}
}
Loading