Made the Visual Studio parser smarter about overlapping reparse requests.

- Prior to this the parser would think that a non-latest reparse request was the latest because the only way we would check to see if a change reference was "latest" would be to do an equality check on the snapshot and `SourceChange`; the issue here was that `SourceChange` was null but the snapshots were the same. Problems could arise with this due to project context changes.
- Added tests to validate the new reparse behavior.
- Renamed `Edit` in the `BackgroundParser` to `ChangeReference` also refactored all the "edit" text in `BackgroundParser` to be `ChangerReference` like.
- Added a new event args to be the DTO between the internal and external parser implementations. This is how we could pas additional information in order to determine "latest" change reference.

#2336
This commit is contained in:
N. Taylor Mullen 2018-05-08 16:46:07 -07:00
parent a412299881
commit 7c70207594
4 changed files with 109 additions and 67 deletions

View File

@ -30,7 +30,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
/// <summary> /// <summary>
/// Fired on the main thread. /// Fired on the main thread.
/// </summary> /// </summary>
public event EventHandler<DocumentStructureChangedEventArgs> ResultsReady; public event EventHandler<BackgroundParserResultsReadyEventArgs> ResultsReady;
public bool IsIdle public bool IsIdle
{ {
@ -47,10 +47,11 @@ namespace Microsoft.VisualStudio.Editor.Razor
_main.Cancel(); _main.Cancel();
} }
public void QueueChange(SourceChange change, ITextSnapshot snapshot) public ChangeReference QueueChange(SourceChange change, ITextSnapshot snapshot)
{ {
var edit = new Edit(change, snapshot); var changeReference = new ChangeReference(change, snapshot);
_main.QueueChange(edit); _main.QueueChange(changeReference);
return changeReference;
} }
public void Dispose() public void Dispose()
@ -63,7 +64,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
return _main.Lock(); return _main.Lock();
} }
protected virtual void OnResultsReady(DocumentStructureChangedEventArgs args) protected virtual void OnResultsReady(BackgroundParserResultsReadyEventArgs args)
{ {
using (SynchronizeMainThreadState()) using (SynchronizeMainThreadState())
{ {
@ -115,7 +116,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
private string _fileName; private string _fileName;
private readonly object _stateLock = new object(); private readonly object _stateLock = new object();
private IList<Edit> _changes = new List<Edit>(); private IList<ChangeReference> _changes = new List<ChangeReference>();
public MainThreadState(string fileName) public MainThreadState(string fileName)
{ {
@ -124,7 +125,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
SetThreadId(Thread.CurrentThread.ManagedThreadId); SetThreadId(Thread.CurrentThread.ManagedThreadId);
} }
public event EventHandler<DocumentStructureChangedEventArgs> ResultsReady; public event EventHandler<BackgroundParserResultsReadyEventArgs> ResultsReady;
public CancellationToken CancelToken public CancellationToken CancelToken
{ {
@ -154,7 +155,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
return new DisposableAction(() => Monitor.Exit(_stateLock)); return new DisposableAction(() => Monitor.Exit(_stateLock));
} }
public void QueueChange(Edit edit) public void QueueChange(ChangeReference change)
{ {
// Any thread can queue a change. // Any thread can queue a change.
@ -166,7 +167,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
_currentParcelCancelSource.Cancel(); _currentParcelCancelSource.Cancel();
} }
_changes.Add(edit); _changes.Add(change);
_hasParcel.Set(); _hasParcel.Set();
} }
} }
@ -182,12 +183,12 @@ namespace Microsoft.VisualStudio.Editor.Razor
_currentParcelCancelSource = new CancellationTokenSource(); _currentParcelCancelSource = new CancellationTokenSource();
var changes = _changes; var changes = _changes;
_changes = new List<Edit>(); _changes = new List<ChangeReference>();
return new WorkParcel(changes, _currentParcelCancelSource.Token); return new WorkParcel(changes, _currentParcelCancelSource.Token);
} }
} }
public void ReturnParcel(DocumentStructureChangedEventArgs args) public void ReturnParcel(BackgroundParserResultsReadyEventArgs args)
{ {
lock (_stateLock) lock (_stateLock)
{ {
@ -242,7 +243,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
private CancellationToken _shutdownToken; private CancellationToken _shutdownToken;
private RazorProjectEngine _projectEngine; private RazorProjectEngine _projectEngine;
private RazorSyntaxTree _currentSyntaxTree; private RazorSyntaxTree _currentSyntaxTree;
private IList<Edit> _previouslyDiscarded = new List<Edit>(); private IList<ChangeReference> _previouslyDiscarded = new List<ChangeReference>();
public BackgroundThread(MainThreadState main, RazorProjectEngine projectEngine, string filePath, string projectDirectory) public BackgroundThread(MainThreadState main, RazorProjectEngine projectEngine, string filePath, string projectDirectory)
{ {
@ -274,30 +275,30 @@ namespace Microsoft.VisualStudio.Editor.Razor
{ {
// Grab the parcel of work to do // Grab the parcel of work to do
var parcel = _main.GetParcel(); var parcel = _main.GetParcel();
if (parcel.Edits.Any()) if (parcel.Changes.Any())
{ {
try try
{ {
DocumentStructureChangedEventArgs args = null; BackgroundParserResultsReadyEventArgs args = null;
using (var linkedCancel = CancellationTokenSource.CreateLinkedTokenSource(_shutdownToken, parcel.CancelToken)) using (var linkedCancel = CancellationTokenSource.CreateLinkedTokenSource(_shutdownToken, parcel.CancelToken))
{ {
if (!linkedCancel.IsCancellationRequested) if (!linkedCancel.IsCancellationRequested)
{ {
// Collect ALL changes // Collect ALL changes
List<Edit> allEdits; List<ChangeReference> allChanges;
if (_previouslyDiscarded != null) if (_previouslyDiscarded != null)
{ {
allEdits = Enumerable.Concat(_previouslyDiscarded, parcel.Edits).ToList(); allChanges = Enumerable.Concat(_previouslyDiscarded, parcel.Changes).ToList();
} }
else else
{ {
allEdits = parcel.Edits.ToList(); allChanges = parcel.Changes.ToList();
} }
var finalEdit = allEdits.Last(); var finalChange = allChanges.Last();
var results = ParseChange(finalEdit.Snapshot, linkedCancel.Token); var results = ParseChange(finalChange.Snapshot, linkedCancel.Token);
if (results != null && !linkedCancel.IsCancellationRequested) if (results != null && !linkedCancel.IsCancellationRequested)
{ {
@ -307,15 +308,12 @@ namespace Microsoft.VisualStudio.Editor.Razor
_currentSyntaxTree = results.GetSyntaxTree(); _currentSyntaxTree = results.GetSyntaxTree();
// Build Arguments // Build Arguments
args = new DocumentStructureChangedEventArgs( args = new BackgroundParserResultsReadyEventArgs(finalChange, results);
finalEdit.Change,
finalEdit.Snapshot,
results);
} }
else else
{ {
// Parse completed but we were cancelled in the mean time. Add these to the discarded changes set // Parse completed but we were cancelled in the mean time. Add these to the discarded changes set
_previouslyDiscarded = allEdits; _previouslyDiscarded = allChanges;
} }
} }
} }
@ -378,20 +376,20 @@ namespace Microsoft.VisualStudio.Editor.Razor
private class WorkParcel private class WorkParcel
{ {
public WorkParcel(IList<Edit> changes, CancellationToken cancelToken) public WorkParcel(IList<ChangeReference> changes, CancellationToken cancelToken)
{ {
Edits = changes; Changes = changes;
CancelToken = cancelToken; CancelToken = cancelToken;
} }
public CancellationToken CancelToken { get; } public CancellationToken CancelToken { get; }
public IList<Edit> Edits { get; } public IList<ChangeReference> Changes { get; }
} }
private class Edit internal class ChangeReference
{ {
public Edit(SourceChange change, ITextSnapshot snapshot) public ChangeReference(SourceChange change, ITextSnapshot snapshot)
{ {
Change = change; Change = change;
Snapshot = snapshot; Snapshot = snapshot;
@ -399,7 +397,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
public SourceChange Change { get; } public SourceChange Change { get; }
public ITextSnapshot Snapshot { get; set; } public ITextSnapshot Snapshot { get; }
} }
} }
} }

View File

@ -0,0 +1,22 @@
// 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;
using Microsoft.AspNetCore.Razor.Language;
using static Microsoft.VisualStudio.Editor.Razor.BackgroundParser;
namespace Microsoft.VisualStudio.Editor.Razor
{
internal class BackgroundParserResultsReadyEventArgs : EventArgs
{
public BackgroundParserResultsReadyEventArgs(ChangeReference edit, RazorCodeDocument codeDocument)
{
ChangeReference = edit;
CodeDocument = codeDocument;
}
public ChangeReference ChangeReference { get; }
public RazorCodeDocument CodeDocument { get; }
}
}

View File

@ -12,6 +12,7 @@ using Microsoft.AspNetCore.Razor.Language.Legacy;
using Microsoft.CodeAnalysis.Razor; using Microsoft.CodeAnalysis.Razor;
using Microsoft.CodeAnalysis.Razor.Editor; using Microsoft.CodeAnalysis.Razor.Editor;
using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.Text;
using static Microsoft.VisualStudio.Editor.Razor.BackgroundParser;
using ITextBuffer = Microsoft.VisualStudio.Text.ITextBuffer; using ITextBuffer = Microsoft.VisualStudio.Text.ITextBuffer;
using Timer = System.Threading.Timer; using Timer = System.Threading.Timer;
@ -317,8 +318,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
{ {
_dispatcher.AssertForegroundThread(); _dispatcher.AssertForegroundThread();
_latestChangeReference = new ChangeReference(change, snapshot); _latestChangeReference = _parser.QueueChange(change, snapshot);
_parser.QueueChange(change, snapshot);
} }
private void OnNotifyForegroundIdle() private void OnNotifyForegroundIdle()
@ -357,7 +357,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
} }
} }
private void OnResultsReady(object sender, DocumentStructureChangedEventArgs args) private void OnResultsReady(object sender, BackgroundParserResultsReadyEventArgs args)
{ {
_dispatcher.AssertBackgroundThread(); _dispatcher.AssertBackgroundThread();
@ -375,21 +375,25 @@ namespace Microsoft.VisualStudio.Editor.Razor
return; return;
} }
var args = (DocumentStructureChangedEventArgs)state; var backgroundParserArgs = (BackgroundParserResultsReadyEventArgs)state;
if (_latestChangeReference == null || // extra hardening if (_latestChangeReference == null || // extra hardening
!_latestChangeReference.IsAssociatedWith(args) || _latestChangeReference != backgroundParserArgs.ChangeReference ||
args.Snapshot != TextBuffer.CurrentSnapshot) backgroundParserArgs.ChangeReference.Snapshot != TextBuffer.CurrentSnapshot)
{ {
// In the middle of parsing a newer change or about to parse a newer change. // In the middle of parsing a newer change or about to parse a newer change.
return; return;
} }
_latestChangeReference = null; _latestChangeReference = null;
_codeDocument = args.CodeDocument; _codeDocument = backgroundParserArgs.CodeDocument;
_snapshot = args.Snapshot; _snapshot = backgroundParserArgs.ChangeReference.Snapshot;
_partialParser = new RazorSyntaxTreePartialParser(CodeDocument.GetSyntaxTree()); _partialParser = new RazorSyntaxTreePartialParser(CodeDocument.GetSyntaxTree());
DocumentStructureChanged?.Invoke(this, args); var documentStructureChangedArgs = new DocumentStructureChangedEventArgs(
backgroundParserArgs.ChangeReference.Change,
backgroundParserArgs.ChangeReference.Snapshot,
backgroundParserArgs.CodeDocument);
DocumentStructureChanged?.Invoke(this, documentStructureChangedArgs);
} }
private void ConfigureProjectEngine(RazorProjectEngineBuilder builder) private void ConfigureProjectEngine(RazorProjectEngineBuilder builder)
@ -432,25 +436,5 @@ namespace Microsoft.VisualStudio.Editor.Razor
return _tagHelpers; return _tagHelpers;
} }
} }
// Internal for testing
internal class ChangeReference
{
public ChangeReference(SourceChange change, ITextSnapshot snapshot)
{
Change = change;
Snapshot = snapshot;
}
public SourceChange Change { get; }
public ITextSnapshot Snapshot { get; }
public bool IsAssociatedWith(DocumentStructureChangedEventArgs other)
{
return Change == other.SourceChange &&
Snapshot == other.Snapshot;
}
}
} }
} }

View File

@ -108,10 +108,9 @@ namespace Microsoft.VisualStudio.Editor.Razor
{ {
var called = false; var called = false;
parser.DocumentStructureChanged += (sender, e) => called = true; parser.DocumentStructureChanged += (sender, e) => called = true;
parser._latestChangeReference = new DefaultVisualStudioRazorParser.ChangeReference(null, new StringTextSnapshot(string.Empty)); parser._latestChangeReference = new BackgroundParser.ChangeReference(null, new StringTextSnapshot(string.Empty));
var args = new DocumentStructureChangedEventArgs( var args = new BackgroundParserResultsReadyEventArgs(
new SourceChange(0, 0, string.Empty), new BackgroundParser.ChangeReference(new SourceChange(0, 0, string.Empty), new StringTextSnapshot(string.Empty)),
new StringTextSnapshot(string.Empty),
TestRazorCodeDocument.CreateEmpty()); TestRazorCodeDocument.CreateEmpty());
// Act // Act
@ -138,12 +137,11 @@ namespace Microsoft.VisualStudio.Editor.Razor
parser.DocumentStructureChanged += (sender, e) => called = true; parser.DocumentStructureChanged += (sender, e) => called = true;
var latestChange = new SourceChange(0, 0, string.Empty); var latestChange = new SourceChange(0, 0, string.Empty);
var latestSnapshot = documentTracker.TextBuffer.CurrentSnapshot; var latestSnapshot = documentTracker.TextBuffer.CurrentSnapshot;
parser._latestChangeReference = new DefaultVisualStudioRazorParser.ChangeReference(latestChange, latestSnapshot); parser._latestChangeReference = new BackgroundParser.ChangeReference(latestChange, latestSnapshot);
var codeDocument = TestRazorCodeDocument.CreateEmpty(); var codeDocument = TestRazorCodeDocument.CreateEmpty();
codeDocument.SetSyntaxTree(RazorSyntaxTree.Parse(TestRazorSourceDocument.Create())); codeDocument.SetSyntaxTree(RazorSyntaxTree.Parse(TestRazorSourceDocument.Create()));
var args = new DocumentStructureChangedEventArgs( var args = new BackgroundParserResultsReadyEventArgs(
latestChange, parser._latestChangeReference,
latestSnapshot,
codeDocument); codeDocument);
// Act // Act
@ -154,6 +152,46 @@ namespace Microsoft.VisualStudio.Editor.Razor
} }
} }
[ForegroundFact]
public void OnDocumentStructureChanged_FiresForOnlyLatestTextBufferReparseEdit()
{
// Arrange
var documentTracker = CreateDocumentTracker();
using (var parser = new DefaultVisualStudioRazorParser(
Dispatcher,
documentTracker,
ProjectEngineFactory,
new DefaultErrorReporter(),
Mock.Of<VisualStudioCompletionBroker>()))
{
var called = false;
parser.DocumentStructureChanged += (sender, e) => called = true;
var latestSnapshot = documentTracker.TextBuffer.CurrentSnapshot;
parser._latestChangeReference = new BackgroundParser.ChangeReference(null, latestSnapshot);
var codeDocument = TestRazorCodeDocument.CreateEmpty();
codeDocument.SetSyntaxTree(RazorSyntaxTree.Parse(TestRazorSourceDocument.Create()));
var badArgs = new BackgroundParserResultsReadyEventArgs(
// This is a different reparse edit, shouldn't be fired for this call
new BackgroundParser.ChangeReference(null, latestSnapshot),
codeDocument);
var goodArgs = new BackgroundParserResultsReadyEventArgs(
parser._latestChangeReference,
codeDocument);
// Act - 1
parser.OnDocumentStructureChanged(badArgs);
// Assert - 1
Assert.False(called);
// Act - 2
parser.OnDocumentStructureChanged(goodArgs);
// Assert - 2
Assert.True(called);
}
}
[ForegroundFact] [ForegroundFact]
public void StartIdleTimer_DoesNotRestartTimerWhenAlreadyRunning() public void StartIdleTimer_DoesNotRestartTimerWhenAlreadyRunning()
{ {