Timeout after lack of activity - related #598

Author: GrahamTheCoder

CHANGELOG.md

@@ -4,9 +4,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ## [Unreleased]
 
+* Timeout cosmetic operations (formatting/comments) after 15 minutes of inactivity [#598](https://github.com/icsharpcode/CodeConverter/issues/598)
 
 ### Vsix
-
+* Options page to adjust timeout
 
 ### VB -> C#
 * Convert parameterized properties with optional parameters [#597](https://github.com/icsharpcode/CodeConverter/issues/597)
@@ -15,7 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 * Don't generate unnecessary properties for WithEvents fields [#572](https://github.com/icsharpcode/CodeConverter/issues/572)
 
 ### C# -> VB
-
+* Performance increase for large files/projects
 
 ## [8.1.6] - 2020-07-12
 

CodeConverter/CSharp/VBToCSConversion.cs

@@ -29,7 +29,6 @@ public class VBToCSConversion : ILanguageConversion
 
         public ConversionOptions ConversionOptions { get; set; }
 
-
         public async Task<IProjectContentsConverter> CreateProjectContentsConverterAsync(Project project, IProgress<ConversionProgress> progress, CancellationToken cancellationToken)
         {
             _progress = progress;

CodeConverter/CSharp/VBToCSProjectContentsConverter.cs

@@ -36,8 +36,11 @@ public VBToCSProjectContentsConverter(ConversionOptions conversionOptions, bool
             this._useProjectLevelWinformsAdjustments = useProjectLevelWinformsAdjustments;
             _progress = progress;
             _cancellationToken = cancellationToken;
+            OptionalOperations = new OptionalOperations(conversionOptions.AbandonOptionalTasksAfter, progress, cancellationToken);
         }
 
+        public OptionalOperations OptionalOperations { get; }
+
         public string RootNamespace => _conversionOptions.RootNamespaceOverride ??
                                        ((VisualBasicCompilationOptions)SourceProject.CompilationOptions).RootNamespace;
 
@@ -65,7 +68,7 @@ private async Task<Project> WithProjectLevelWinformsAdjustmentsAsync(Project pro
 
         public async Task<SyntaxNode> SingleFirstPassAsync(Document document)
         {
-            return await VisualBasicConverter.ConvertCompilationTreeAsync(document, _csharpViewOfVbSymbols, _csharpReferenceProject, _cancellationToken);
+            return await VisualBasicConverter.ConvertCompilationTreeAsync(document, _csharpViewOfVbSymbols, _csharpReferenceProject, OptionalOperations, _cancellationToken);
         }
 
         public async Task<(Project project, List<WipFileConversion<DocumentId>> firstPassDocIds)> GetConvertedProjectAsync(WipFileConversion<SyntaxNode>[] firstPassResults)

CodeConverter/CSharp/VisualBasicConverter.cs

@@ -8,13 +8,15 @@
 using VBSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax;
 using CSS = Microsoft.CodeAnalysis.CSharp.Syntax;
 using System.Threading;
+using ICSharpCode.CodeConverter.VB;
 
 namespace ICSharpCode.CodeConverter.CSharp
 {
     internal static class VisualBasicConverter
     {
         public static async Task<SyntaxNode> ConvertCompilationTreeAsync(Document document,
-            CSharpCompilation csharpViewOfVbSymbols, Project csharpReferenceProject, CancellationToken cancellationToken)
+            CSharpCompilation csharpViewOfVbSymbols, Project csharpReferenceProject,
+            OptionalOperations optionalOperations, CancellationToken cancellationToken)
         {
             document = await document.WithExpandedRootAsync(cancellationToken);
             var root = await document.GetSyntaxRootAsync(cancellationToken) as VBSyntax.CompilationUnitSyntax ??
@@ -30,13 +32,7 @@ public static async Task<SyntaxNode> ConvertCompilationTreeAsync(Document docume
                 DeclarationNodeVisitor(document, compilation, semanticModel, csharpViewOfVbSymbols, csSyntaxGenerator);
             var converted = (CSS.CompilationUnitSyntax)await root.AcceptAsync(visualBasicSyntaxVisitor.TriviaConvertingDeclarationVisitor);
 
-            try {
-                // This call is very expensive for large documents. Should look for a more performant version, e.g. Is NormalizeWhitespace good enough?
-                converted = (CSS.CompilationUnitSyntax)Formatter.Format(converted, document.Project.Solution.Workspace, cancellationToken: cancellationToken);
-                return LineTriviaMapper.MapSourceTriviaToTarget(root, converted);
-            } catch (Exception) { //TODO log
-                return converted;
-            }
+            return optionalOperations.MapSourceTriviaToTargetHandled(root, converted, document);
         }
 
         private static string NullRootError(Document document)

CodeConverter/Shared/ConversionOptions.cs

@@ -1,10 +1,12 @@
-using Microsoft.CodeAnalysis;
+using System;
+using Microsoft.CodeAnalysis;
 
 namespace ICSharpCode.CodeConverter.Shared
 {
     public class ConversionOptions
     {
         public string RootNamespaceOverride { get; set; }
         public CompilationOptions TargetCompilationOptionsOverride { get; set; }
+        public TimeSpan AbandonOptionalTasksAfter { get; set; } = TimeSpan.FromMinutes(30);
     }
 }

CodeConverter/Shared/IProjectContentsConverter.cs

@@ -12,6 +12,7 @@ public interface IProjectContentsConverter
         string LanguageVersion { get; }
         string RootNamespace { get; }
         Project SourceProject { get; }
+        OptionalOperations OptionalOperations { get; }
         Task<SyntaxNode> SingleFirstPassAsync(Document document);
         Task<(Project project, List<WipFileConversion<DocumentId>> firstPassDocIds)> GetConvertedProjectAsync(WipFileConversion<SyntaxNode>[] firstPassResults);
         public IAsyncEnumerable<ConversionResult> GetAdditionalConversionResults(IReadOnlyCollection<TextDocument> additionalDocumentsToConvert, CancellationToken cancellationToken);

CodeConverter/Shared/OptionalOperations.cs

@@ -0,0 +1,133 @@
+using System;
+using System.Threading;
+using System.Timers;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Formatting;
+
+namespace ICSharpCode.CodeConverter.Shared
+{
+    /// <summary>
+    /// https://github.com/icsharpcode/CodeConverter/issues/598#issuecomment-663773878
+    /// </summary>
+    public class OptionalOperations
+    {
+        private readonly IProgress<ConversionProgress> _progress;
+        private readonly CancellationToken _wholeTaskCancellationToken;
+        private readonly ActivityMonitor _activityMonitor;
+        private readonly CancellationTokenSource _optionalTaskCts;
+
+        public OptionalOperations(TimeSpan abandonTasksIfNoActivityFor, IProgress<ConversionProgress> progress,
+            CancellationToken wholeTaskCancellationToken)
+        {
+            _progress = progress;
+            _wholeTaskCancellationToken = wholeTaskCancellationToken;
+            _optionalTaskCts = CancellationTokenSource.CreateLinkedTokenSource(wholeTaskCancellationToken);
+            _activityMonitor = new ActivityMonitor(abandonTasksIfNoActivityFor, _optionalTaskCts);
+        }
+
+        public SyntaxNode MapSourceTriviaToTargetHandled<TSource, TTarget>(TSource root,
+            TTarget converted, Document document)
+            where TSource : SyntaxNode, ICompilationUnitSyntax where TTarget : SyntaxNode, ICompilationUnitSyntax
+        {
+            try
+            {
+                converted = (TTarget) Format(converted, document);
+                return LineTriviaMapper.MapSourceTriviaToTarget(root, converted);
+            }
+            catch (Exception e)
+            {
+                _progress.Report(new ConversionProgress($"Error while formatting and converting comments: {e}"));
+                return converted;
+            }
+        }
+
+        public SyntaxNode Format(SyntaxNode node, Document document)
+        {
+            if (!_optionalTaskCts.IsCancellationRequested) {
+                try {
+                    _optionalTaskCts.Token.ThrowIfCancellationRequested();
+                    _activityMonitor.ActivityStarted();
+                    // This call is very expensive for large documents. Should look for a more performant version, e.g. Is NormalizeWhitespace good enough?
+                    return Formatter.Format(node, document.Project.Solution.Workspace,
+                        cancellationToken: _optionalTaskCts.Token);
+
+                } catch (OperationCanceledException) {
+                    if (!_wholeTaskCancellationToken.IsCancellationRequested) {
+                        _progress.Report(new ConversionProgress(
+                            "Aborting all further formatting and comment mapping, you can increase the timeout for this in Tools -> Options -> Code Converter."));
+                    }
+                } finally {
+                    _activityMonitor.ActivityFinished();
+                }
+            }
+            return node.NormalizeWhitespace();
+        }
+
+        /// <summary>
+        /// Reasonably lightweight check that there's been some activity within the given timeout.
+        /// </summary>
+        private class ActivityMonitor
+        {
+            private readonly TimeSpan _timeout;
+            private readonly CancellationTokenSource _cts;
+            private volatile int _activeOperations;
+            /// <summary>
+            /// Must check <see cref="_activeOperations"/> within the lock before changed timer.Enabled
+            /// This avoids race conditions between the last task of a set finishing and the first of a new set starting
+            /// </summary>
+            private readonly object _timerEnabledWriteLock = new object();
+            private static System.Timers.Timer _timer;
+
+
+            private void OnTimedEvent(object source, ElapsedEventArgs e)
+            {
+                if (!_cts.IsCancellationRequested && _timer.Enabled) {
+                    _cts.Cancel();
+                }
+            }
+
+            public ActivityMonitor(TimeSpan timeout, CancellationTokenSource cts)
+            {
+                _timeout = timeout;
+                _cts = cts;
+                _timer = new System.Timers.Timer(timeout.TotalMilliseconds) {AutoReset = true};
+                _timer.Elapsed += OnTimedEvent;
+            }
+
+            public void ActivityStarted()
+            {
+                if (Interlocked.Increment(ref _activeOperations) == 1) {
+                    lock (_timerEnabledWriteLock) {
+                        if (_activeOperations > 0) {
+                            _timer.Enabled = true;
+                        }
+                    }
+                }
+                ActivityObserved();
+            }
+
+            public void ActivityFinished()
+            {
+                ActivityObserved();
+                if (Interlocked.Decrement(ref _activeOperations) == 0) {
+                    lock (_timerEnabledWriteLock) {
+                        if (_activeOperations == 0) {
+                            _timer.Enabled = false;
+                        }
+                    }
+                }
+            }
+
+            private void ActivityObserved()
+            {
+                try {
+                    _timer.Interval = _timeout.TotalMilliseconds;
+                } catch (ObjectDisposedException e) {
+                    // Race condition if we try to set the interval after disabling the timer
+                } catch (NullReferenceException e) {
+                    // Race condition if we try to set the interval after disabling the timer
+                }
+            }
+        }
+    }
+}

CodeConverter/Shared/ProjectConversion.cs

@@ -10,9 +10,7 @@
 using ICSharpCode.CodeConverter.CSharp;
 using ICSharpCode.CodeConverter.Util;
 using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.Formatting;
 using Microsoft.CodeAnalysis.Text;
-using Microsoft.VisualStudio.Threading;
 
 namespace ICSharpCode.CodeConverter.Shared
 {
@@ -69,7 +67,7 @@ private ProjectConversion(IProjectContentsConverter projectContentsConverter, IE
             document = projectContentsConverter.SourceProject.GetDocument(document.Id);
 
             var conversion = new ProjectConversion(projectContentsConverter, new[] { document }, Enumerable.Empty<TextDocument>(), languageConversion, cancellationToken, conversionOptions.ShowCompilationErrors, returnSelectedNode);
-            var conversionResults = await conversion.Convert(progress).ToArrayAsync();
+            var conversionResults = await conversion.Convert(progress).ToArrayAsync(cancellationToken);
             return GetSingleResultForDocument(conversionResults, document);
         }
 
@@ -93,7 +91,7 @@ public static async IAsyncEnumerable<ConversionResult> ConvertProject(Project pr
             var convertProjectContents = ConvertProjectContents(projectContentsConverter, languageConversion, progress, cancellationToken);
 
             var results = WithProjectFile(projectContentsConverter, languageConversion, sourceFilePaths, convertProjectContents, replacements);
-            await foreach (var result in results) yield return result;
+            await foreach (var result in results.WithCancellation(cancellationToken)) yield return result;
         }
 
         /// <remarks>Perf: Keep lazy so that we don't keep an extra copy of all files in memory at once</remarks>
@@ -164,23 +162,23 @@ private static async IAsyncEnumerable<ConversionResult> ConvertProjectContents(
         {
             var documentsWithLengths = await projectContentsConverter.SourceProject.Documents
                 .Where(d => !BannedPaths.Any(d.FilePath.Contains))
-                .SelectAsync(async d => (Doc: d, Length: (await d.GetTextAsync()).Length));
+                .SelectAsync(async d => (Doc: d, Length: (await d.GetTextAsync(cancellationToken)).Length));
 
             //Perf heuristic: Decrease memory pressure on the simplification phase by converting large files first https://github.com/icsharpcode/CodeConverter/issues/524#issuecomment-590301594
             var documentsToConvert = documentsWithLengths.OrderByDescending(d => d.Length).Select(d => d.Doc);
 
             var projectConversion = new ProjectConversion(projectContentsConverter, documentsToConvert, projectContentsConverter.SourceProject.AdditionalDocuments, languageConversion, cancellationToken, false);
 
             var results = projectConversion.Convert(progress);
-            await foreach (var result in results) yield return result;
+            await foreach (var result in results.WithCancellation(cancellationToken)) yield return result;
         }
 
 
         private async IAsyncEnumerable<ConversionResult> Convert(IProgress<ConversionProgress> progress)
         {
             var phaseProgress = StartPhase(progress, "Phase 1 of 2:");
             var firstPassResults = _documentsToConvert.ParallelSelectAwait(d => FirstPassLoggedAsync(d, phaseProgress), Env.MaxDop, _cancellationToken);
-            var (proj1, docs1) = await _projectContentsConverter.GetConvertedProjectAsync(await firstPassResults.ToArrayAsync());
+            var (proj1, docs1) = await _projectContentsConverter.GetConvertedProjectAsync(await firstPassResults.ToArrayAsync(_cancellationToken));
 
             var warnings = await GetProjectWarningsAsync(_projectContentsConverter.SourceProject, proj1);
             if (!string.IsNullOrWhiteSpace(warnings)) {
@@ -190,7 +188,7 @@ private async IAsyncEnumerable<ConversionResult> Convert(IProgress<ConversionPro
 
             phaseProgress = StartPhase(progress, "Phase 2 of 2:");
             var secondPassResults = proj1.GetDocuments(docs1).ParallelSelectAwait(d => SecondPassLoggedAsync(d, phaseProgress), Env.MaxDop, _cancellationToken);
-            await foreach (var result in secondPassResults.Select(CreateConversionResult)) {
+            await foreach (var result in secondPassResults.Select(CreateConversionResult).WithCancellation(_cancellationToken)) {
                 yield return result;
             }
             await foreach (var result in _projectContentsConverter.GetAdditionalConversionResults(_additionalDocumentsToConvert, _cancellationToken)) {
@@ -233,29 +231,29 @@ private async Task<WipFileConversion<SyntaxNode>> SecondPassLoggedAsync(WipFileC
                     selectedNode = await GetSelectedNodeAsync(document);
                     var extraLeadingTrivia = selectedNode.GetFirstToken().GetPreviousToken().TrailingTrivia;
                     var extraTrailingTrivia = selectedNode.GetLastToken().GetNextToken().LeadingTrivia;
-                    selectedNode = Formatter.Format(selectedNode, document.Project.Solution.Workspace);
+                    selectedNode = _projectContentsConverter.OptionalOperations.Format(selectedNode, document);
                     if (extraLeadingTrivia.Any(t => !t.IsWhitespaceOrEndOfLine())) selectedNode = selectedNode.WithPrependedLeadingTrivia(extraLeadingTrivia);
                     if (extraTrailingTrivia.Any(t => !t.IsWhitespaceOrEndOfLine())) selectedNode = selectedNode.WithAppendedTrailingTrivia(extraTrailingTrivia);
                 } else {
-                    selectedNode = await document.GetSyntaxRootAsync();
-                    selectedNode = Formatter.Format(selectedNode, document.Project.Solution.Workspace);
+                    selectedNode = await document.GetSyntaxRootAsync(_cancellationToken);
+                    selectedNode = _projectContentsConverter.OptionalOperations.Format(selectedNode, document);
                     var convertedDoc = document.WithSyntaxRoot(selectedNode);
-                    selectedNode = await convertedDoc.GetSyntaxRootAsync();
+                    selectedNode = await convertedDoc.GetSyntaxRootAsync(_cancellationToken);
                 }
             } catch (Exception e) {
                 errors = new[] { e.ToString() };
             }
 
-            var convertedNode = selectedNode ?? await convertedDocument.GetSyntaxRootAsync();
+            var convertedNode = selectedNode ?? await convertedDocument.GetSyntaxRootAsync(_cancellationToken);
             return (convertedNode, errors);
         }
 
         private async Task<string> GetProjectWarningsAsync(Project source, Project converted)
         {
             if (!_showCompilationErrors) return null;
 
-            var sourceCompilation = await source.GetCompilationAsync();
-            var convertedCompilation = await converted.GetCompilationAsync();
+            var sourceCompilation = await source.GetCompilationAsync(_cancellationToken);
+            var convertedCompilation = await converted.GetCompilationAsync(_cancellationToken);
             return CompilationWarnings.WarningsForCompilation(sourceCompilation, "source") + CompilationWarnings.WarningsForCompilation(convertedCompilation, "target");
         }
 
@@ -297,7 +295,7 @@ private static async Task<Document> WithAnnotatedSelectionAsync(Document documen
 
         private async Task<SyntaxNode> GetSelectedNodeAsync(Document document)
         {
-            var resultNode = await document.GetSyntaxRootAsync();
+            var resultNode = await document.GetSyntaxRootAsync(_cancellationToken);
             var selectedNode = resultNode.GetAnnotatedNodes(AnnotationConstants.SelectedNodeAnnotationKind)
                 .FirstOrDefault();
             if (selectedNode != null) {