fixed a bug on solution crawler progress report. (#24708)

* fixed a bug on solution crawler progress report.

found case where retry can mess up Start/Stop call. instead trying being smart to reduce Start/Stop call, made it simpler and rely on ref counting on reporter. and the fact that all enqeued work must run at some point.

* updated comment

* renamed and added comments

src/Features/Core/Portable/SolutionCrawler/WorkCoordinator.AsyncWorkItemQueue.cs

@@ -57,25 +57,6 @@ public bool HasAnyWork
                     }
                 }
 
-                public void RemoveCancellationSource(object key)
-                {
-                    lock (_gate)
-                    {
-                        // just remove cancellation token from the map.
-                        // the cancellation token might be passed out to other service
-                        // so don't call cancel on the source only because we are done using it.
-                        _cancellationMap.Remove(key);
-
-                        if (!HasAnyWork_NoLock)
-                        {
-                            Contract.Requires(_cancellationMap.Count == 0);
-
-                            // last work is done.
-                            _progressReporter.Stop();
-                        }
-                    }
-                }
-
                 public virtual Task WaitAsync(CancellationToken cancellationToken)
                 {
                     return _semaphore.WaitAsync(cancellationToken);
@@ -85,19 +66,23 @@ public bool AddOrReplace(WorkItem item)
                 {
                     lock (_gate)
                     {
-                        // we need to check both work enqueued and work sent
-                        // out that are still running before start new progress
-                        // report otherwise, we can start progress again
-                        // while last work item is still running causing
-                        // progress bar to be broken
-                        if (!HasAnyWork_NoLock && _cancellationMap.Count == 0)
+                        if (AddOrReplace_NoLock(item))
                         {
-                            // first work is added.
+                            // the item is new item that got added to the queue.
+                            // let solution crawler progress report to know about new item enqueued.
+                            // progress reporter will take care of nested/overlapped works by itself
+                            // 
+                            // order of events is as follow
+                            // 1. first item added by AddOrReplace which is the point where progress start.
+                            // 2. bunch of other items added or replaced (workitem in the queue > 0)
+                            // 3. items start dequeued to be processed by TryTake or TryTakeAnyWork
+                            // 4. once item is done processed, it is marked as done by MarkWorkItemDoneFor
+                            // 5. all items in the queue are dequeued (workitem in the queue == 0) 
+                            //    but there can be still work in progress
+                            // 6. all works are considered done when last item is marked done by MarkWorkItemDoneFor
+                            //    and at the point, we will set progress to stop.
                             _progressReporter.Start();
-                        }
 
-                        if (AddOrReplace_NoLock(item))
-                        {
                             // increase count 
                             _semaphore.Release();
                             return true;
@@ -107,6 +92,22 @@ public bool AddOrReplace(WorkItem item)
                     }
                 }
 
+                public void MarkWorkItemDoneFor(object key)
+                {
+                    lock (_gate)
+                    {
+                        // just remove cancellation token from the map.
+                        // the cancellation token might be passed out to other service
+                        // so don't call cancel on the source only because we are done using it.
+                        _cancellationMap.Remove(key);
+
+                        // every works enqueued by "AddOrReplace" will be processed
+                        // at some point, and when it is processed, this method will be called to mark
+                        // work has been done.
+                        _progressReporter.Stop();
+                    }
+                }
+
                 public void RequestCancellationOnRunningTasks()
                 {
                     List<CancellationTokenSource> cancellations;

src/Features/Core/Portable/SolutionCrawler/WorkCoordinator.HighPriorityProcessor.cs

@@ -187,7 +187,7 @@ private async Task ProcessDocumentAsync(Solution solution, ImmutableArray<IIncre
                             SolutionCrawlerLogger.LogProcessActiveFileDocument(_processor._logAggregator, documentId.Id, processedEverything);
 
                             // remove one that is finished running
-                            _workItemQueue.RemoveCancellationSource(workItem.DocumentId);
+                            _workItemQueue.MarkWorkItemDoneFor(workItem.DocumentId);
                         }
                     }
 

src/Features/Core/Portable/SolutionCrawler/WorkCoordinator.LowPriorityProcessor.cs

@@ -173,7 +173,7 @@ private async Task ProcessProjectAsync(ImmutableArray<IIncrementalAnalyzer> anal
                             SolutionCrawlerLogger.LogProcessProject(this.Processor._logAggregator, projectId.Id, processedEverything);
 
                             // remove one that is finished running
-                            _workItemQueue.RemoveCancellationSource(projectId);
+                            _workItemQueue.MarkWorkItemDoneFor(projectId);
                         }
                     }
 

src/Features/Core/Portable/SolutionCrawler/WorkCoordinator.NormalPriorityProcessor.cs

@@ -364,7 +364,7 @@ private async Task ProcessDocumentAsync(ImmutableArray<IIncrementalAnalyzer> ana
                             SolutionCrawlerLogger.LogProcessDocument(this.Processor._logAggregator, documentId.Id, processedEverything);
 
                             // remove one that is finished running
-                            _workItemQueue.RemoveCancellationSource(workItem.DocumentId);
+                            _workItemQueue.MarkWorkItemDoneFor(workItem.DocumentId);
                         }
                     }