Skip to content

Commit

Permalink
Fix microsoft#914: Multiple fast opening of files causes GetAnalysisA…
Browse files Browse the repository at this point in the history 
…sync to return old analysis
  • Loading branch information
@AlexanderSher
AlexanderSher committed Apr 10, 2019
1 parent a718093 commit 24e101c
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 38 deletions.
54 changes: 30 additions & 24 deletions src/Analysis/Ast/Impl/Analyzer/PythonAnalyzerSession.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public void Start(bool analyzeEntry) {
if (analyzeEntry && _entry != null) {
Task.Run(() => Analyze(_entry, Version, _cts.Token), _cts.Token).DoNotWait();
} else {
StartAsync(_walker).ContinueWith(_startNextSession).DoNotWait();
StartAsync().ContinueWith(_startNextSession).DoNotWait();
}
}

Expand All @@ -109,11 +109,11 @@ public void Cancel() {
}
}

private async Task StartAsync(IDependencyChainWalker<AnalysisModuleKey, PythonAnalyzerEntry> walker) {
_progress.ReportRemaining(walker.Remaining);
private async Task StartAsync() {
_progress.ReportRemaining(_walker.Remaining);

lock (_syncObj) {
var notAnalyzed = walker.AffectedValues.Count(e => e.NotAnalyzed);
var notAnalyzed = _walker.AffectedValues.Count(e => e.NotAnalyzed);

if (_isCanceled && notAnalyzed < _maxTaskRunning) {
_state = State.Completed;
Expand All @@ -124,15 +124,15 @@ private async Task StartAsync(IDependencyChainWalker<AnalysisModuleKey, PythonAn

var cancellationToken = _cts.Token;
var stopWatch = Stopwatch.StartNew();
foreach (var affectedEntry in walker.AffectedValues) {
foreach (var affectedEntry in _walker.AffectedValues) {
affectedEntry.Invalidate(Version);
}

var originalRemaining = walker.Remaining;
var originalRemaining = _walker.Remaining;
var remaining = originalRemaining;
try {
_log?.Log(TraceEventType.Verbose, $"Analysis version {walker.Version} of {originalRemaining} entries has started.");
remaining = await AnalyzeAffectedEntriesAsync(walker, stopWatch, cancellationToken);
_log?.Log(TraceEventType.Verbose, $"Analysis version {_walker.Version} of {originalRemaining} entries has started.");
remaining = await AnalyzeAffectedEntriesAsync(stopWatch, cancellationToken);
} finally {
_cts.Dispose();
stopWatch.Stop();
Expand All @@ -150,8 +150,8 @@ private async Task StartAsync(IDependencyChainWalker<AnalysisModuleKey, PythonAn

var elapsed = stopWatch.Elapsed.TotalMilliseconds;

SendTelemetry(elapsed, originalRemaining, remaining, walker.Version);
LogResults(elapsed, originalRemaining, remaining, walker.Version);
SendTelemetry(elapsed, originalRemaining, remaining, _walker.Version);
LogResults(elapsed, originalRemaining, remaining, _walker.Version);
}

private void SendTelemetry(double elapsed, int originalRemaining, int remaining, int version) {
Expand Down Expand Up @@ -201,10 +201,12 @@ private void LogResults(double elapsed, int originalRemaining, int remaining, in
}
}

private async Task<int> AnalyzeAffectedEntriesAsync(IDependencyChainWalker<AnalysisModuleKey, PythonAnalyzerEntry> walker, Stopwatch stopWatch, CancellationToken cancellationToken) {
private async Task<int> AnalyzeAffectedEntriesAsync(Stopwatch stopWatch, CancellationToken cancellationToken) {
IDependencyChainNode<PythonAnalyzerEntry> node;
var remaining = 0;
while ((node = await walker.GetNextAsync(cancellationToken)) != null) {
var ace = new AsyncCountdownEvent(0);

while ((node = await _walker.GetNextAsync(cancellationToken)) != null) {
bool isCanceled;
lock (_syncObj) {
isCanceled = _isCanceled;
Expand All @@ -216,14 +218,16 @@ private async Task<int> AnalyzeAffectedEntriesAsync(IDependencyChainWalker<Analy
continue;
}

if (Interlocked.Increment(ref _runningTasks) >= _maxTaskRunning || walker.Remaining == 1) {
Analyze(walker, node, stopWatch, cancellationToken);
if (Interlocked.Increment(ref _runningTasks) >= _maxTaskRunning || _walker.Remaining == 1) {
Analyze(node, null, stopWatch, cancellationToken);
} else {
StartAnalysis(walker, node, stopWatch, cancellationToken).DoNotWait();
StartAnalysis(node, ace, stopWatch, cancellationToken).DoNotWait();
}
}

if (walker.MissingKeys.All(k => k.IsTypeshed)) {
await ace.WaitAsync(cancellationToken);

if (_walker.MissingKeys.All(k => k.IsTypeshed)) {
Interlocked.Exchange(ref _runningTasks, 0);
bool isCanceled;
lock (_syncObj) {
Expand All @@ -239,36 +243,37 @@ private async Task<int> AnalyzeAffectedEntriesAsync(IDependencyChainWalker<Analy
}


private Task StartAnalysis(IDependencyChainWalker<AnalysisModuleKey, PythonAnalyzerEntry> walker, IDependencyChainNode<PythonAnalyzerEntry> node, Stopwatch stopWatch, CancellationToken cancellationToken)
=> Task.Run(() => Analyze(walker, node, stopWatch, cancellationToken), cancellationToken);
private Task StartAnalysis(IDependencyChainNode<PythonAnalyzerEntry> node, AsyncCountdownEvent ace, Stopwatch stopWatch, CancellationToken cancellationToken)
=> Task.Run(() => Analyze(node, ace, stopWatch, cancellationToken), cancellationToken);

/// <summary>
/// Performs analysis of the document. Returns document global scope
/// with declared variables and inner scopes. Does not analyze chain
/// of dependencies, it is intended for the single file analysis.
/// </summary>
private void Analyze(IDependencyChainWalker<AnalysisModuleKey, PythonAnalyzerEntry> walker, IDependencyChainNode<PythonAnalyzerEntry> node, Stopwatch stopWatch, CancellationToken cancellationToken) {
private void Analyze(IDependencyChainNode<PythonAnalyzerEntry> node, AsyncCountdownEvent ace, Stopwatch stopWatch, CancellationToken cancellationToken) {
IPythonModule module;
try {
ace?.AddOne();
var entry = node.Value;
if (!entry.IsValidVersion(walker.Version, out module, out var ast)) {
if (!entry.IsValidVersion(_walker.Version, out module, out var ast)) {
_log?.Log(TraceEventType.Verbose, $"Analysis of {module.Name}({module.ModuleType}) canceled.");
node.Skip();
return;
}
var startTime = stopWatch.Elapsed;
AnalyzeEntry(entry, module, ast, walker.Version, cancellationToken);
AnalyzeEntry(entry, module, ast, _walker.Version, cancellationToken);
node.Commit();

_log?.Log(TraceEventType.Verbose, $"Analysis of {module.Name}({module.ModuleType}) completed in {(stopWatch.Elapsed - startTime).TotalMilliseconds} ms.");
} catch (OperationCanceledException oce) {
node.Value.TryCancel(oce, walker.Version);
node.Value.TryCancel(oce, _walker.Version);
node.Skip();
module = node.Value.Module;
_log?.Log(TraceEventType.Verbose, $"Analysis of {module.Name}({module.ModuleType}) canceled.");
} catch (Exception exception) {
module = node.Value.Module;
node.Value.TrySetException(exception, walker.Version);
node.Value.TrySetException(exception, _walker.Version);
node.Commit();
_log?.Log(TraceEventType.Verbose, $"Analysis of {module.Name}({module.ModuleType}) failed.");
} finally {
Expand All @@ -278,8 +283,9 @@ private void Analyze(IDependencyChainWalker<AnalysisModuleKey, PythonAnalyzerEnt
}

if (!isCanceled) {
_progress.ReportRemaining(walker.Remaining);
_progress.ReportRemaining(_walker.Remaining);
}
ace?.Signal();
Interlocked.Decrement(ref _runningTasks);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Analysis/Ast/Impl/Documents/Definitions/IDocument.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public interface IDocument: IPythonModule, IDisposable {
/// <summary>
/// Returns document analysis.
/// </summary>
Task<IDocumentAnalysis> GetAnalysisAsync(int waitTime, CancellationToken cancellationToken = default);
Task<IDocumentAnalysis> GetAnalysisAsync(int waitTime = 200, CancellationToken cancellationToken = default);

/// <summary>
/// Returns last known document AST. The AST may be out of date or null.
Expand Down
2 changes: 1 addition & 1 deletion src/Analysis/Ast/Impl/Modules/PythonModule.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ private void InitializeContent(string content, int version) {
lock (AnalysisLock) {
LoadContent(content, version);

var startParse = ContentState < State.Parsing && _parsingTask == null;
var startParse = ContentState < State.Parsing && (_parsingTask == null || version > 0);
if (startParse) {
Parse();
}
Expand Down
6 changes: 4 additions & 2 deletions src/Analysis/Ast/Impl/Types/LocatedMember.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using System.Collections.Generic;
using System.Linq;
using Microsoft.Python.Analysis.Modules;
using Microsoft.Python.Core;

namespace Microsoft.Python.Analysis.Types {
internal abstract class LocatedMember : ILocatedMember {
Expand Down Expand Up @@ -56,8 +57,9 @@ public virtual IReadOnlyList<LocationInfo> References {
}

var refs = _references
.GroupBy(x => x.LocationInfo.DocumentUri)
.SelectMany(g => g.Select(x => x.LocationInfo).OrderBy(x => x.Span));
.Select(x => x.LocationInfo)
.OrderBy(x => x.DocumentUri.AbsolutePath, StringExtensions.PathsStringComparer)
.ThenBy(x => x.Span);
return Enumerable.Repeat(Definition, 1).Concat(refs).ToArray();
}
}
Expand Down
17 changes: 11 additions & 6 deletions src/LanguageServer/Impl/Sources/ReferenceSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,16 +142,20 @@ private IEnumerable<string> ScanFiles(IDictionary<string, PythonAst> closedFiles

private async Task AnalyzeFiles(IEnumerable<Uri> files, CancellationToken cancellationToken) {
var rdt = _services.GetService<IRunningDocumentTable>();
var analysisTasks = new List<Task>();
foreach (var f in files) {
Analyze(f, rdt);
var module = Analyze(f, rdt);
if (module != null) {
analysisTasks.Add(module.GetAnalysisAsync(cancellationToken: cancellationToken));
}
}
var analyzer = _services.GetService<IPythonAnalyzer>();
await analyzer.WaitForCompleteAnalysisAsync(cancellationToken);

await Task.WhenAll(analysisTasks);
}

private static void Analyze(Uri uri, IRunningDocumentTable rdt) {
private static IDocument Analyze(Uri uri, IRunningDocumentTable rdt) {
if (rdt.GetDocument(uri) != null) {
return; // Already opened by another analysis.
return null; // Already opened by another analysis.
}

var filePath = uri.ToAbsolutePath();
Expand All @@ -161,7 +165,8 @@ private static void Analyze(Uri uri, IRunningDocumentTable rdt) {
Uri = uri,
ModuleType = ModuleType.User
};
rdt.AddModule(mco);

return rdt.AddModule(mco);
}

private ILocatedMember GetRootDefinition(ILocatedMember lm) {
Expand Down
9 changes: 5 additions & 4 deletions src/LanguageServer/Test/ReferencesTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,14 @@ def func(x):
y = func(x)
x = 2
";
var uri1 = TestData.GetDefaultModuleUri();
var uri2 = TestData.GetNextModuleUri();

var code2 = $@"
from {Path.GetFileNameWithoutExtension(uri1.AbsolutePath)} import x
from module1 import x
y = x
";
var uri1 = await TestData.CreateTestSpecificFileAsync("module1.py", code1);
var uri2 = await TestData.CreateTestSpecificFileAsync("module2.py", code2);

await CreateServicesAsync(PythonVersions.LatestAvailable3X, uri1.AbsolutePath);

var rdt = Services.GetService<IRunningDocumentTable>();
Expand All @@ -98,7 +99,7 @@ def func(x):
refs[2].range.Should().Be(7, 0, 7, 1);
refs[2].uri.Should().Be(uri1);

refs[3].range.Should().Be(1, 19, 1, 20);
refs[3].range.Should().Be(1, 20, 1, 21);
refs[3].uri.Should().Be(uri2);
refs[4].range.Should().Be(2, 4, 2, 5);
refs[4].uri.Should().Be(uri2);
Expand Down

0 comments on commit 24e101c

Please sign in to comment.