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

Background analysis support #1507

Merged
merged 41 commits into from
Jul 10, 2019
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
c0a1550
Refactored work queue to only wait for single document.
savpek May 16, 2019
d508c4b
Prototype of new queue.
savpek May 18, 2019
357a395
Initial prototype of foreground/background where only foreground is s…
savpek May 18, 2019
83c209c
Fixed queue tests to compile.
savpek May 18, 2019
0840a7d
Fixes for queue.
savpek May 19, 2019
f3a0a72
Fixed issue with completing work when queue is empty.
savpek May 21, 2019
6eca1f5
Testfixes.
savpek May 21, 2019
493f260
Basic endpoint for reanalysis and test.
savpek May 22, 2019
608e7af
Merge remote-tracking branch 'upstream/master' into feature/backgroun…
savpek May 25, 2019
d1e3ea5
Added message on project analyzed to events.
savpek May 25, 2019
e036d04
Fixed mistake.
savpek May 25, 2019
5b79b77
Added test for emitted event about project analyzed.
savpek May 25, 2019
26ad1fa
Renamed property at event message.
savpek May 25, 2019
9a6b3ac
Support for project scoped analysis.
savpek May 26, 2019
bfe17de
Testfixes and support for project file path as analysis context.
savpek May 26, 2019
48efad8
Testfixes.
savpek May 26, 2019
eddac0e
Refactored background analysis event to send information of start end…
savpek May 26, 2019
2f83340
Improved logging.
savpek May 26, 2019
eea1ec6
Merge branch 'master' into feature/background-analysis
savpek Jun 1, 2019
e44a498
Merge branch 'master' into feature/background-analysis
savpek Jun 16, 2019
e937aad
Resolving merge conflicts and few small tweaks.
savpek Jun 20, 2019
e8512b0
Merge branch 'feature/background-analysis' of https://github.com/savp…
savpek Jun 20, 2019
20ad709
Additiona merge fix.
savpek Jun 20, 2019
9540a9e
Merge branch 'master' into feature/background-analysis
david-driscoll Jun 23, 2019
6d1d9a1
Merge branch 'master' into feature/background-analysis
savpek Jun 23, 2019
553f698
Merge remote-tracking branch 'upstream/master' into feature/backgroun…
savpek Jun 26, 2019
481f679
Attempt to fix test.
savpek Jun 26, 2019
7863828
Testing build stability
savpek Jun 26, 2019
9073b9d
Testing build stability
savpek Jun 26, 2019
e612da7
Merge branch 'master' into feature/background-analysis
savpek Jun 27, 2019
6bd1ce2
Testing build stability
savpek Jun 27, 2019
f1c887f
Merge branch 'feature/background-analysis' of https://github.com/savp…
savpek Jun 27, 2019
870cde5
Merge branch 'master' into feature/background-analysis
savpek Jun 28, 2019
78feb7e
Merge branch 'master' into feature/background-analysis
savpek Jul 7, 2019
6a7a739
Renames etc based on reviews.
savpek Jul 7, 2019
0c76642
Updated re-analyze request as SimpleFileRequest.
savpek Jul 7, 2019
842e9fd
Added support for configurable timeout on analysis.
savpek Jul 7, 2019
95a5303
Rename for parameter.
savpek Jul 7, 2019
d439a1d
Merge branch 'master' into feature/background-analysis
savpek Jul 10, 2019
47d72f1
Merge branch 'master' into feature/background-analysis
savpek Jul 10, 2019
8acbd35
Merge branch 'master' into feature/background-analysis
filipw Jul 10, 2019
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
2 changes: 2 additions & 0 deletions src/OmniSharp.Abstractions/Models/Events/EventTypes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,7 @@ public static class EventTypes
public const string PackageRestoreFinished = nameof(PackageRestoreFinished);
public const string UnresolvedDependencies = nameof(UnresolvedDependencies);
public const string ProjectConfiguration = nameof(ProjectConfiguration);
public const string ProjectDiagnosticStatus = nameof(ProjectDiagnosticStatus);

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
namespace OmniSharp.Models.Events
{
public class ProjectDiagnosticStatusMessage
{
public ProjectDiagnosticStatus Status { get; set; }
public string ProjectFilePath { get; set; }
public string Type = "background";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
namespace OmniSharp.Models.Events
{
public enum ProjectDiagnosticStatus
{
Started = 0,
Ready = 1
}
}
13 changes: 13 additions & 0 deletions src/OmniSharp.Abstractions/Models/v1/ReAnalyze/ReanalyzeRequest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
using OmniSharp.Mef;
using OmniSharp.Models;

namespace OmniSharp.Abstractions.Models.V1.ReAnalyze
{
[OmniSharpEndpoint(OmniSharpEndpoints.ReAnalyze, typeof(ReAnalyzeRequest), typeof(ReanalyzeResponse))]
public class ReAnalyzeRequest: IRequest
savpek marked this conversation as resolved.
Show resolved Hide resolved
{
// This document path is used as context to resolve which project should be analyzed. Simplifies
// clients since they don't have to figure out what is correct project for current open file.
public string CurrentOpenFilePathAsContext { get; set; }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using OmniSharp.Models;

namespace OmniSharp.Abstractions.Models.V1.ReAnalyze
{
public class ReanalyzeResponse : IAggregateResponse
{
public IAggregateResponse Merge(IAggregateResponse response) { return response; }
}
}
2 changes: 2 additions & 0 deletions src/OmniSharp.Abstractions/OmniSharpEndpoints.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ public static class OmniSharpEndpoints
public const string Close = "/close";
public const string Diagnostics = "/diagnostics";

public const string ReAnalyze = "/reanalyze";

public static class V2
{
public const string GetCodeActions = "/v2/getcodeactions";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
using System;
using System.Collections.Immutable;
using System.Composition;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.Extensions.Logging;
using OmniSharp.Abstractions.Models.V1.ReAnalyze;
using OmniSharp.Mef;
using OmniSharp.Models;
using OmniSharp.Models.Diagnostics;
using OmniSharp.Roslyn.CSharp.Workers.Diagnostics;

namespace OmniSharp.Roslyn.CSharp.Services.Diagnostics
{
[OmniSharpHandler(OmniSharpEndpoints.ReAnalyze, LanguageNames.CSharp)]
public class ReAnalyzeService : IRequestHandler<ReAnalyzeRequest, ReanalyzeResponse>
{
private readonly ICsDiagnosticWorker _diagWorker;
private readonly OmniSharpWorkspace _workspace;
private readonly ILogger<ReAnalyzeService> _logger;

[ImportingConstructor]
public ReAnalyzeService(ICsDiagnosticWorker diagWorker, OmniSharpWorkspace workspace, ILoggerFactory loggerFactory)
{
_diagWorker = diagWorker;
_workspace = workspace;
_logger = loggerFactory.CreateLogger<ReAnalyzeService>();
}

public Task<ReanalyzeResponse> Handle(ReAnalyzeRequest request)
{

if(!string.IsNullOrEmpty(request.CurrentOpenFilePathAsContext))
{
var currentSolution = _workspace.CurrentSolution;

var projectIds = WhenRequestIsProjectFileItselfGetFilesFromIt(request.CurrentOpenFilePathAsContext, currentSolution)
?? GetDocumentsFromProjectWhereDocumentIs(request.CurrentOpenFilePathAsContext, currentSolution);

_logger.LogInformation($"Queue analysis for project(s) {string.Join(", ", projectIds)}");

_diagWorker.QueueDocumentsOfProjectsForDiagnostics(projectIds);
}
else
{
_logger.LogInformation($"Queue analysis for all projects.");
_diagWorker.QueueAllDocumentsForDiagnostics();
}

return Task.FromResult(new ReanalyzeResponse());
}

private ImmutableArray<ProjectId>? WhenRequestIsProjectFileItselfGetFilesFromIt(string currentOpenFilePathAsContext, Solution currentSolution)
{
var projects = currentSolution.Projects.Where(x => CompareProjectPath(currentOpenFilePathAsContext, x)).Select(x => x.Id).ToImmutableArray();

if(!projects.Any())
return null;

return projects;
}

private static bool CompareProjectPath(string currentOpenFilePathAsContext, Project x)
{
return String.Compare(
x.FilePath,
currentOpenFilePathAsContext,
StringComparison.InvariantCultureIgnoreCase) == 0;
}

private static ImmutableArray<ProjectId> GetDocumentsFromProjectWhereDocumentIs(string currentOpenFilePathAsContext, Solution currentSolution)
savpek marked this conversation as resolved.
Show resolved Hide resolved
{
return currentSolution
.GetDocumentIdsWithFilePath(currentOpenFilePathAsContext)
.Select(docId => currentSolution.GetDocument(docId).Project.Id)
.ToImmutableArray();
savpek marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
130 changes: 75 additions & 55 deletions src/OmniSharp.Roslyn.CSharp/Workers/Diagnostics/AnalyzerWorkQueue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,98 +12,118 @@ namespace OmniSharp.Roslyn.CSharp.Workers.Diagnostics
{
public class AnalyzerWorkQueue
david-driscoll marked this conversation as resolved.
Show resolved Hide resolved
{
private readonly int _throttlingMs = 300;
private class Queue
{
public Queue(TimeSpan throttling)
{
Throttling = throttling;
}

private readonly ConcurrentDictionary<DocumentId, (DateTime modified, CancellationTokenSource workDoneSource)> _workQueue =
new ConcurrentDictionary<DocumentId, (DateTime modified, CancellationTokenSource workDoneSource)>();
public ImmutableHashSet<DocumentId> WorkWaitingToExecute { get; set; } = ImmutableHashSet<DocumentId>.Empty;
public ImmutableHashSet<DocumentId> WorkExecuting { get; set; } = ImmutableHashSet<DocumentId>.Empty;
public DateTime LastThrottlingBegan { get; set; } = DateTime.UtcNow;
public TimeSpan Throttling { get; }
public CancellationTokenSource WorkPendingToken { get; set; }
}

private readonly ConcurrentDictionary<DocumentId, (DateTime modified, CancellationTokenSource workDoneSource)> _currentWork =
new ConcurrentDictionary<DocumentId, (DateTime modified, CancellationTokenSource workDoneSource)>();
private readonly Dictionary<AnalyzerWorkType, Queue> _queues = null;

private readonly ILogger<AnalyzerWorkQueue> _logger;
private readonly Func<DateTime> _utcNow;
private readonly int _maximumDelayWhenWaitingForResults;
private readonly ILogger<AnalyzerWorkQueue> _logger;
private readonly object _queueLock = new Object();

public AnalyzerWorkQueue(ILoggerFactory loggerFactory, Func<DateTime> utcNow = null, int timeoutForPendingWorkMs = 15*1000)
public AnalyzerWorkQueue(ILoggerFactory loggerFactory, Func<DateTime> utcNow = null, int timeoutForPendingWorkMs = 15 * 1000)
Copy link
Member

Choose a reason for hiding this comment

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

should this timeout be configurable in omnisharp.json?

Copy link
Contributor Author

@savpek savpek Jun 28, 2019

Choose a reason for hiding this comment

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

In previous version i think it could, but now in this version it only applies to request for diagnostics from single file. It's basically just failsafe to prevent api hang in this version.

However i am not sure is this needed any more at all since theres timeout for file analysis too on worker side which should prevent that situation. Timeout for single file analysis in worker could indeed be configurable and that way it's implemented only on single location.

{
utcNow = utcNow ?? (() => DateTime.UtcNow);
_queues = new Dictionary<AnalyzerWorkType, Queue>
{
{ AnalyzerWorkType.Foreground, new Queue(TimeSpan.FromMilliseconds(150)) },
{ AnalyzerWorkType.Background, new Queue(TimeSpan.FromMilliseconds(1500)) }
};

_logger = loggerFactory.CreateLogger<AnalyzerWorkQueue>();
_utcNow = utcNow;
_utcNow = utcNow ?? (() => DateTime.UtcNow);
_maximumDelayWhenWaitingForResults = timeoutForPendingWorkMs;
}

public void PutWork(DocumentId documentId)
public void PutWork(IReadOnlyCollection<DocumentId> documentIds, AnalyzerWorkType workType)
{
_workQueue.AddOrUpdate(documentId,
(modified: DateTime.UtcNow, new CancellationTokenSource()),
(_, oldValue) => (modified: DateTime.UtcNow, oldValue.workDoneSource));
lock (_queueLock)
{
var queue = _queues[workType];

if (queue.WorkWaitingToExecute.IsEmpty)
queue.LastThrottlingBegan = _utcNow();

if (queue.WorkPendingToken == null)
queue.WorkPendingToken = new CancellationTokenSource();

queue.WorkWaitingToExecute = queue.WorkWaitingToExecute.Union(documentIds);
}
}

public ImmutableArray<DocumentId> TakeWork()
public IReadOnlyCollection<DocumentId> TakeWork(AnalyzerWorkType workType)
{
lock (_workQueue)
lock (_queueLock)
{
var now = _utcNow();
var currentWork = _workQueue
.Where(x => ThrottlingPeriodNotActive(x.Value.modified, now))
.OrderByDescending(x => x.Value.modified)
.Take(50)
.ToImmutableArray();

foreach (var work in currentWork)
{
_workQueue.TryRemove(work.Key, out _);
_currentWork.TryAdd(work.Key, work.Value);
}

return currentWork.Select(x => x.Key).ToImmutableArray();
var queue = _queues[workType];

if (IsThrottlingActive(queue) || queue.WorkWaitingToExecute.IsEmpty)
return ImmutableHashSet<DocumentId>.Empty;

queue.WorkExecuting = queue.WorkWaitingToExecute;
queue.WorkWaitingToExecute = ImmutableHashSet<DocumentId>.Empty;
return queue.WorkExecuting;
}
}

private bool ThrottlingPeriodNotActive(DateTime modified, DateTime now)
private bool IsThrottlingActive(Queue queue)
{
return (now - modified).TotalMilliseconds >= _throttlingMs;
return (_utcNow() - queue.LastThrottlingBegan).TotalMilliseconds <= queue.Throttling.TotalMilliseconds;
}

public void MarkWorkAsCompleteForDocumentId(DocumentId documentId)
public void WorkComplete(AnalyzerWorkType workType)
{
if(_currentWork.TryGetValue(documentId, out var work))
lock (_queueLock)
{
work.workDoneSource.Cancel();
_currentWork.TryRemove(documentId, out _);
if(_queues[workType].WorkExecuting.IsEmpty)
return;

_queues[workType].WorkPendingToken?.Cancel();
_queues[workType].WorkPendingToken = null;
_queues[workType].WorkExecuting = ImmutableHashSet<DocumentId>.Empty;
}
}

// Omnisharp V2 api expects that it can request current information of diagnostics any time,
// Omnisharp V2 api expects that it can request current information of diagnostics any time (single file/current document),
// however analysis is worker based and is eventually ready. This method is used to make api look
// like it's syncronous even that actual analysis may take a while.
public async Task WaitForResultsAsync(ImmutableArray<DocumentId> documentIds)
public Task WaitForegroundWorkComplete()
{
var items = new List<(DateTime modified, CancellationTokenSource workDoneSource)>();
var queue = _queues[AnalyzerWorkType.Foreground];

if (queue.WorkPendingToken == null || (queue.WorkPendingToken == null && queue.WorkWaitingToExecute.IsEmpty))
return Task.CompletedTask;

foreach (var documentId in documentIds)
return Task.Delay(_maximumDelayWhenWaitingForResults, queue.WorkPendingToken.Token)
.ContinueWith(task => LogTimeouts(task));
}

public bool TryPromote(DocumentId id)
{

if (_queues[AnalyzerWorkType.Background].WorkWaitingToExecute.Contains(id) || _queues[AnalyzerWorkType.Background].WorkExecuting.Contains(id))
{
if (_currentWork.ContainsKey(documentId))
{
items.Add(_currentWork[documentId]);
}
else if (_workQueue.ContainsKey(documentId))
{
items.Add(_workQueue[documentId]);
}
PutWork(new[] { id }, AnalyzerWorkType.Foreground);
return true;
}

await Task.WhenAll(items.Select(item =>
Task.Delay(_maximumDelayWhenWaitingForResults, item.workDoneSource.Token)
.ContinueWith(task => LogTimeouts(task, documentIds))));
return false;
}

// This logs wait's for documentId diagnostics that continue without getting current version from analyzer.
// This happens on larger solutions during initial load or situations where analysis slows down remarkably.
private void LogTimeouts(Task task, IEnumerable<DocumentId> documentIds)
private void LogTimeouts(Task task)
{
if (!task.IsCanceled) _logger.LogDebug($"Timeout before work got ready for one of documents {string.Join(",", documentIds)}.");
if (!task.IsCanceled) _logger.LogWarning($"Timeout before work got ready for foreground analysis queue.");
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
namespace OmniSharp.Roslyn.CSharp.Workers.Diagnostics
{
public enum AnalyzerWorkType
{
Background, Foreground
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,14 @@ private static async Task<ImmutableArray<Diagnostic>> GetDiagnosticsForDocument(

public ImmutableArray<DocumentId> QueueAllDocumentsForDiagnostics()
{
var documents = _workspace.CurrentSolution.Projects.SelectMany(x => x.Documents).ToImmutableArray();
var documents = _workspace.CurrentSolution.Projects.SelectMany(x => x.Documents);
QueueForDiagnosis(documents.Select(x => x.FilePath).ToImmutableArray());
return documents.Select(x => x.Id).ToImmutableArray();
}

public ImmutableArray<DocumentId> QueueDocumentsOfProjectsForDiagnostics(ImmutableArray<ProjectId> projectIds)
savpek marked this conversation as resolved.
Show resolved Hide resolved
{
var documents = projectIds.SelectMany(projectId => _workspace.CurrentSolution.GetProject(projectId).Documents);
QueueForDiagnosis(documents.Select(x => x.FilePath).ToImmutableArray());
return documents.Select(x => x.Id).ToImmutableArray();
}
Expand Down
Loading