Create an AsyncPriorityWorkQueue type

[jasonmalinowski]

This is similar to AsyncBatchingWorkQueue, but allows for items to have priorities that can also be raised to allow work items to be processed earlier. The model for running the work is also slightly different: rather than being given a list of work as a immutable list, instead the do-work function is given an enumerator that only lets the work function grab one item at a time. This allows for more items to be added to a batch wile it's running, and for priorities to be raised for the yet-unprocessed items.

This will get used to implement smarter loading logic in the C# Extension project system, where we will prioritize the loading of certain projects (like ones related to open files), while projects we could load cached data for will be deprioritized. For now this PR implements just the helper; the use will come later once some other PRs also merge.

Microsoft Reviewers: Open in CodeFlow

[azure-pipelines]

Azure Pipelines:
Successfully started running 2 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Introduces AsyncPriorityWorkQueue<TItem>, a new shared utility for prioritized, promotable, batched async work processing in the Workspaces layer (intended for future smarter project-system loading scenarios).

Changes:

• Added AsyncPriorityWorkQueue<TItem> implementation that processes higher-priority items first and supports priority promotion during an active batch via a pull-based enumerator.
• Added comprehensive unit tests validating priority ordering, promotion behavior, concurrency, fault tolerance, and disposal semantics.
• Wired the new utility into the shared CompilerExtensions projitems and updated related documentation/comments.

Show a summary per file

File	Description
src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncPriorityWorkQueue.cs	New prioritized async work-queue implementation with promotable items and pull-based batch consumption.
src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems	Includes the new utility in the shared compiler extensions items list.
src/Workspaces/CoreTest/UtilityTest/AsyncPriorityWorkQueueTests.cs	Adds unit tests covering ordering, promotion, concurrency, disposal, and faulted batches.
src/Dependencies/Threading/AsyncBatchingWorkQueue`2.cs	Minor comment wording update for consistency with the new queue’s terminology.
.github/memory/FILE_MAP.md	Updates Workspaces entry description to mention shared-utility test coverage location.

Review details

Suppressed comments (2)

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncPriorityWorkQueue.cs:14

• This using appears unused in this file and may trigger IDE0005 (unused using) warnings. Remove it if not needed.

using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.Shared.TestHooks;
using Microsoft.CodeAnalysis.Threading;

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncPriorityWorkQueue.cs:226

• The enumerator XML doc has a missing separator and is hard to parse. Consider rephrasing to clearly state it doesn’t implement IEnumerable and that it’s safe for multi-threaded consumers.

    /// <summary>
    /// An enumerator that will return the items of the queue in priority order. This doesn't implement IEnumerable the interface can be used from
    /// multiple threads.
    /// </summary>

• Files reviewed: 5/5 changed files
• Comments generated: 6
• Review effort level: Lite

[CyrusNajmabadi]

this indicates reprioritization of an existing item is possible... but i'm not seeing that. am i misreading the docs? perhaps it's better to ay that priorities are fixed, but that means new work can come in at higher priorities and be worked on before lower priority items?

[jasonmalinowski]

See in AddWork where when we add work, we'll remove it from lower priorities if it's there.

[jasonmalinowski]

There's not a method for moving up existing work (without adding it if it's not present). We might need that at some point, not sure yet.

[CyrusNajmabadi]

Overal i like this and can see benefit to it. My complaint is primarily around code duplication. it took a while to land on an impl for ABWQ that was correct and had sound semantics we all liked. i think (but haven't done the metnal exercise) to have a shared impl somehow...

[CyrusNajmabadi]

here's an idea: you can pass in a callback that is passed the Enumerator. or a callback that gets the ImmArray. The latter trivially sits on the former by pulling items from the Enumerator into an array for hte reciver to use...

that sort of thing.

[jasonmalinowski]

@CyrusNajmabadi At this point neither is a clean replacement for the other. The only outright shared code is the "task in flight" and queueing the next task in, which I might pull out into a helper (although it think we used to have that helper and then deleted it...). I'm otherwise erring on the side of "two independent pieces of code can each do their own thing well" is the better tradeoff here for now. Your point that we might be able to treat a batching work queue as a priority queue with one priority and it just pulls on everything might be possible...eventually. There's some interesting things the existing batching queue does around return values that doesn't fit well into this.

Also: I don't quite know how much this will change yet. I was getting this PR out while some other PRs get in, so I won't have code consuming this for another week or so. One thing we know we're going to want is some "wait for things of priority X or higher" operation which will change a bit how the enumerator works, I think.

[Copilot]

Review details

Suppressed comments (3)

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncPriorityWorkQueue.cs:23

• The XML doc says the batch callback is given an IEnumerable of items, but the API actually passes an Enumerator. This is misleading for callers trying to understand the intended consumption model.

/// <item>Each item has a priority, higher priorities (defined as a larger number away from zero) are processed first.</item>
/// <item>Each item has a single priority and only appears in the queue once; unlike <see cref="AsyncBatchingWorkQueue{TItem}"/> where deduplication is optional.</item>
/// <item>When a batch is started, it is given an IEnumerable of the items in the batch, rather than a list. This allows items to be reprioritized while a batch is underway.</item>

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncPriorityWorkQueue.cs:231

• The Enumerator summary has a grammatical error (missing punctuation) and reads as if it should implement IEnumerable. Clarifying the sentence makes the intended usage clearer.

    /// <summary>
    /// An enumerator that will return the items of the queue in priority order. This doesn't implement IEnumerable the interface can be used from
    /// multiple threads.
    /// </summary>

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncPriorityWorkQueue.cs:95

• priority is used to index _itemsByPriority but is never validated. A negative value or a value greater than maximumPriority will throw IndexOutOfRangeException and can leave the queue in a surprising state for callers.

            if (_entireQueueCancellationTokenSource.IsCancellationRequested)
                return;

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (4)

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncPriorityWorkQueue.cs:24

• The XML doc says the batch callback receives an IEnumerable, but the API actually passes an AsyncPriorityWorkQueue.Enumerator. The doc should match the actual contract to avoid confusing callers.

/// <item>Each item has a priority, higher priorities (defined as a larger number away from zero) are processed first.</item>
/// <item>Each item has a single priority and only appears in the queue once; unlike <see cref="AsyncBatchingWorkQueue{TItem}"/> where deduplication is optional.</item>
/// <item>When a batch is started, it is given an IEnumerable of the items in the batch, rather than a list. This allows items to be reprioritized while a batch is underway.</item>
/// <item>We don't support cancelling currently queued work, since the users of this don't have a need. If that becomes a need, that can be added.</item>

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncPriorityWorkQueue.cs:93

• AddWork validates only priority >= _itemsByPriority.Length, but allows negative priorities, which will throw IndexOutOfRangeException when indexing _itemsByPriority[priority]. This should throw ArgumentOutOfRangeException for priority < 0 as well.

    public void AddWork(TItem item, int priority)
    {
        if (priority >= _itemsByPriority.Length)
            throw new ArgumentOutOfRangeException(paramName: nameof(priority), message: $"Priority must be between 0 and {_itemsByPriority.Length - 1}");

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncPriorityWorkQueue.cs:170

• This comment implies the queue being empty means work was cancelled, but the queue can also be empty because all items were already processed. Consider rewording to avoid misleading future readers.

            // If we don't have any items left, then the work was cancelled and we can immediately be done.
            if (_itemsByPriority.All(static s => s.Count == 0))
            {
                _taskInFlight = false;
                return;

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncPriorityWorkQueue.cs:234

• XML doc sentence is missing punctuation and reads like two sentences ran together ("doesn't implement IEnumerable the interface..."). Rewording improves readability and clarity about thread-safety.

    /// <summary>
    /// An enumerator that will return the items of the queue in priority order. This doesn't implement IEnumerable the interface can be used from
    /// multiple threads.
    /// </summary>

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (3)

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncPriorityWorkQueue.cs:13

• The Microsoft.CodeAnalysis.Collections using directive is unused in this file and will produce an unnecessary warning/noise. Please remove it.

using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.Shared.TestHooks;
using Microsoft.CodeAnalysis.Threading;

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncPriorityWorkQueue.cs:147

• This comment has a few grammatical issues (missing punctuation and a missing verb), which makes it hard to understand. Please reword for clarity.

                // Ensure that we always yield the current thread this is necessary for correctness as we are called
                // inside a lock that _taskInFlight to true.  We must ensure that the work to process the next batch
                // must be on another thread that runs afterwards, can only grab the thread once we release it and will
                // then reset that bool back to false

src/Workspaces/CoreTest/UtilityTest/AsyncPriorityWorkQueueTests.cs:108

• Typo in the test name: "Readding" should be "ReAdding" (or "Re-adding") for readability and searchability.

    public async Task ReaddingAtLowerPriorityDoesNotDemote()

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (1)

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncPriorityWorkQueue.cs:13

• using Microsoft.CodeAnalysis.Collections; appears to be unused in this file, which will typically trigger the repo's unused-using analyzers (and adds an unnecessary dependency/import).

using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.Shared.TestHooks;
using Microsoft.CodeAnalysis.Threading;

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (1)

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/AsyncPriorityWorkQueue.cs:237

• The XML doc says "the interface can be used" but this type isn’t an interface; tightening this wording will make the intended threading/usage constraint clearer.

    /// <summary>
    /// An enumerator that will return the items of the queue in priority order. This doesn't implement IEnumerable so the interface can be used from
    /// multiple threads.
    /// </summary>

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite