From e4dc0e932ac5cd440a76681350932969b1d2490f Mon Sep 17 00:00:00 2001 From: David Kean Date: Mon, 19 Oct 2026 11:50:36 +1100 Subject: [PATCH] Reduce JTF dependency graph lock contention Shorten dependency registration critical sections without increasing allocations. Keep small dependency maps inline, avoid redundant completion locks, and reuse notification buffers while preserving graph and queue semantics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../InternalUtilities.cs | 45 +++ .../JoinableTask.cs | 11 + .../JoinableTaskDependencyGraph.cs | 132 +++---- .../SmallDependencyMap`1.cs | 325 ++++++++++++++++++ .../InternalUtilitiesTests.cs | 152 ++++++++ .../JoinableTaskCollectionTests.cs | 91 +++++ ...rosoft.VisualStudio.Threading.Tests.csproj | 3 + .../SmallDependencyMapTests.cs | 274 +++++++++++++++ 8 files changed, 953 insertions(+), 80 deletions(-) create mode 100644 src/Microsoft.VisualStudio.Threading/SmallDependencyMap`1.cs create mode 100644 test/Microsoft.VisualStudio.Threading.Tests/SmallDependencyMapTests.cs diff --git a/src/Microsoft.VisualStudio.Threading/InternalUtilities.cs b/src/Microsoft.VisualStudio.Threading/InternalUtilities.cs index f9eaba00a..0a89f2a44 100644 --- a/src/Microsoft.VisualStudio.Threading/InternalUtilities.cs +++ b/src/Microsoft.VisualStudio.Threading/InternalUtilities.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; +using System.Threading; namespace Microsoft.VisualStudio.Threading; @@ -10,6 +11,50 @@ namespace Microsoft.VisualStudio.Threading; /// internal static class InternalUtilities { + /// + /// Takes a list from a single-slot pool, allocating only when the slot is empty or its capacity is insufficient. + /// + /// The type of list elements. + /// The slot containing an available, empty list. + /// The minimum required capacity. + /// An exclusively owned, empty list with the requested capacity. + internal static List RentList(ref List? pool, int capacity) + { + Requires.Range(capacity >= 0, nameof(capacity)); + + List? list = Interlocked.Exchange(ref pool, null); + if (list is null) + { + return new List(capacity); + } + + if (list.Capacity < capacity) + { + list.Capacity = capacity; + } + + return list; + } + + /// + /// Clears a list and returns it to a single-slot pool if its capacity is within the retention limit. + /// + /// The type of list elements. + /// The slot to return the list to if empty. + /// The exclusively owned list to return. + /// The maximum capacity of a list to retain. + internal static void ReturnList(ref List? pool, List list, int maximumRetainedCapacity) + { + Requires.NotNull(list, nameof(list)); + Requires.Range(maximumRetainedCapacity >= 0, nameof(maximumRetainedCapacity)); + + list.Clear(); + if (list.Capacity <= maximumRetainedCapacity) + { + Interlocked.CompareExchange(ref pool, list, null); + } + } + /// /// Removes an element from the middle of a queue without disrupting the other elements. /// diff --git a/src/Microsoft.VisualStudio.Threading/JoinableTask.cs b/src/Microsoft.VisualStudio.Threading/JoinableTask.cs index 2cf498fdc..21e334a0b 100644 --- a/src/Microsoft.VisualStudio.Threading/JoinableTask.cs +++ b/src/Microsoft.VisualStudio.Threading/JoinableTask.cs @@ -275,6 +275,17 @@ internal bool IsFullyCompleted { get { + JoinableTaskFlags currentState = this.state; + if ((currentState & JoinableTaskFlags.CompleteFinalized) != 0) + { + return true; + } + + if ((currentState & JoinableTaskFlags.CompleteRequested) == 0) + { + return false; + } + using (this.JoinableTaskContext.NoMessagePumpSynchronizationContext.Apply()) { lock (this.JoinableTaskContext.SyncContextLock) diff --git a/src/Microsoft.VisualStudio.Threading/JoinableTaskDependencyGraph.cs b/src/Microsoft.VisualStudio.Threading/JoinableTaskDependencyGraph.cs index c0de4ac3f..e58316341 100644 --- a/src/Microsoft.VisualStudio.Threading/JoinableTaskDependencyGraph.cs +++ b/src/Microsoft.VisualStudio.Threading/JoinableTaskDependencyGraph.cs @@ -16,8 +16,12 @@ namespace Microsoft.VisualStudio.Threading; /// internal static class JoinableTaskDependencyGraph { + private const int MaxRetainedNotificationListCapacity = 64; + private static readonly HashSet EmptySet = new HashSet(); + private static List? notificationListPool; + /// /// Gets a value indicating whether there is no child depenent item. /// This method is expected to be used with the JTF lock. @@ -297,7 +301,7 @@ internal struct JoinableTaskDependentData /// /// When the value in an entry is decremented to 0, the entry is removed from the map. /// - private Dictionary childDependentNodes; + private SmallDependencyMap childDependentNodes; /// /// The head of a singly linked list of records to track which task may process events of this task. @@ -348,58 +352,56 @@ internal static JoinableTaskCollection.JoinRelease AddDependency(IJoinableTaskDe using (parentTaskOrCollection.JoinableTaskContext.NoMessagePumpSynchronizationContext.Apply()) { List? eventsNeedNotify = null; - lock (parentTaskOrCollection.JoinableTaskContext.SyncContextLock) + try { - var joinableTask = joinChild as JoinableTask; - if (joinableTask?.IsFullyCompleted == true) + lock (parentTaskOrCollection.JoinableTaskContext.SyncContextLock) { - return default(JoinableTaskCollection.JoinRelease); - } + var joinableTask = joinChild as JoinableTask; + if (joinableTask?.IsFullyCompleted == true) + { + return default(JoinableTaskCollection.JoinRelease); + } - ref JoinableTaskDependentData data = ref parentTaskOrCollection.GetJoinableTaskDependentData(); - if (data.childDependentNodes is null) - { - data.childDependentNodes = new Dictionary(capacity: 2); - } + ref JoinableTaskDependentData data = ref parentTaskOrCollection.GetJoinableTaskDependentData(); + if (data.childDependentNodes is null) + { + data.childDependentNodes = new SmallDependencyMap(); + } - if (data.childDependentNodes.TryGetValue(joinChild, out int refCount) && !parentTaskOrCollection.NeedRefCountChildDependencies) - { - return default(JoinableTaskCollection.JoinRelease); + if (data.childDependentNodes.TryGetValue(joinChild, out int refCount) && !parentTaskOrCollection.NeedRefCountChildDependencies) + { + return default(JoinableTaskCollection.JoinRelease); + } + + data.childDependentNodes[joinChild] = ++refCount; + if (refCount == 1) + { + // This constitutes a significant change, so we should apply synchronous task tracking to the new child. + joinChild.OnAddedToDependency(parentTaskOrCollection); + AddDependingSynchronousTaskToChild(parentTaskOrCollection, joinChild, ref eventsNeedNotify); + + parentTaskOrCollection.OnDependencyAdded(joinChild); + } } - data.childDependentNodes[joinChild] = ++refCount; - if (refCount == 1) + // We explicitly do this outside our lock. + if (eventsNeedNotify is object) { - // This constitutes a significant change, so we should apply synchronous task tracking to the new child. - joinChild.OnAddedToDependency(parentTaskOrCollection); - IReadOnlyCollection? tasksNeedNotify = AddDependingSynchronousTaskToChild(parentTaskOrCollection, joinChild); - if (tasksNeedNotify.Count > 0) + foreach (AsyncManualResetEvent? queueEvent in eventsNeedNotify) { - eventsNeedNotify = new List(tasksNeedNotify.Count); - foreach (PendingNotification taskToNotify in tasksNeedNotify) - { - AsyncManualResetEvent? notifyEvent = taskToNotify.SynchronousTask.RegisterPendingEventsForSynchrousTask(taskToNotify.TaskHasPendingMessages, taskToNotify.NewPendingMessagesCount); - if (notifyEvent is object) - { - eventsNeedNotify.Add(notifyEvent); - } - } + queueEvent.PulseAll(); } - - parentTaskOrCollection.OnDependencyAdded(joinChild); } - } - // We explicitly do this outside our lock. - if (eventsNeedNotify is object) + return new JoinableTaskCollection.JoinRelease(parentTaskOrCollection, joinChild); + } + finally { - foreach (AsyncManualResetEvent? queueEvent in eventsNeedNotify) + if (eventsNeedNotify is object) { - queueEvent.PulseAll(); + InternalUtilities.ReturnList(ref notificationListPool, eventsNeedNotify, MaxRetainedNotificationListCapacity); } } - - return new JoinableTaskCollection.JoinRelease(parentTaskOrCollection, joinChild); } } @@ -478,7 +480,7 @@ internal static void AddSelfAndDescendentOrJoinedJobs(IJoinableTaskDependent tas } } - Dictionary? childDependentNodes = taskOrCollection.GetJoinableTaskDependentData().childDependentNodes; + SmallDependencyMap? childDependentNodes = taskOrCollection.GetJoinableTaskDependentData().childDependentNodes; if (childDependentNodes is object) { foreach (KeyValuePair item in childDependentNodes) @@ -562,7 +564,7 @@ internal static void ComputeSelfAndDescendentOrJoinedJobsAndRemainTasks(IJoinabl return; } - Dictionary? dependencies = taskOrCollection.GetJoinableTaskDependentData().childDependentNodes; + SmallDependencyMap? dependencies = taskOrCollection.GetJoinableTaskDependentData().childDependentNodes; if (dependencies is object) { foreach (KeyValuePair item in dependencies) @@ -690,7 +692,7 @@ internal void OnTaskCompleted(IJoinableTaskDependent thisDependentNode) if (this.childDependentNodes is object) { - Dictionary.KeyCollection? childrenTasks = this.childDependentNodes.Keys; + IReadOnlyCollection childrenTasks = this.childDependentNodes.Keys; while (existingTaskTracking is object) { RemoveDependingSynchronousTaskFrom(childrenTasks, existingTaskTracking.SynchronousTask, force: existingTaskTracking.SynchronousTask == thisDependentNode); @@ -763,8 +765,8 @@ internal IReadOnlyCollection GetDependingSynchronousTasks(bool for /// /// The current joinableTask or collection owns the data. /// The new child task. - /// Pairs of synchronous tasks we need notify and the event source triggering it, plus the number of pending events. - private static IReadOnlyCollection AddDependingSynchronousTaskToChild(IJoinableTaskDependent dependentNode, IJoinableTaskDependent child) + /// Receives the events to pulse after releasing the JTF lock. + private static void AddDependingSynchronousTaskToChild(IJoinableTaskDependent dependentNode, IJoinableTaskDependent child, ref List? eventsNeedNotify) { Requires.NotNull(dependentNode, nameof(dependentNode)); Requires.NotNull(child, nameof(child)); @@ -774,10 +776,9 @@ private static IReadOnlyCollection AddDependingSynchronousT int count = data.CountOfDependingSynchronousTasks(); if (count == 0) { - return Array.Empty(); + return; } - var tasksNeedNotify = new List(count); DependentSynchronousTask? existingTaskTracking = data.dependingSynchronousTaskTracking; while (existingTaskTracking is object) { @@ -785,13 +786,16 @@ private static IReadOnlyCollection AddDependingSynchronousT JoinableTask? eventTriggeringTask = AddDependingSynchronousTask(child, existingTaskTracking.SynchronousTask, ref totalEventNumber); if (eventTriggeringTask is object) { - tasksNeedNotify.Add(new PendingNotification(existingTaskTracking.SynchronousTask, eventTriggeringTask, totalEventNumber)); + AsyncManualResetEvent? notifyEvent = existingTaskTracking.SynchronousTask.RegisterPendingEventsForSynchrousTask(eventTriggeringTask, totalEventNumber); + if (notifyEvent is object) + { + eventsNeedNotify ??= InternalUtilities.RentList(ref notificationListPool, count); + eventsNeedNotify.Add(notifyEvent); + } } existingTaskTracking = existingTaskTracking.Next; } - - return tasksNeedNotify; } /// @@ -1051,38 +1055,6 @@ private void RemoveDependingSynchronousTaskFromChild(IJoinableTaskDependent chil } } - /// - /// The record of a pending notification we need send to the synchronous task that we have some new messages to process. - /// - private readonly struct PendingNotification - { - internal PendingNotification(JoinableTask synchronousTask, JoinableTask taskHasPendingMessages, int newPendingMessagesCount) - { - Requires.NotNull(synchronousTask, nameof(synchronousTask)); - Requires.NotNull(taskHasPendingMessages, nameof(taskHasPendingMessages)); - - this.SynchronousTask = synchronousTask; - this.TaskHasPendingMessages = taskHasPendingMessages; - this.NewPendingMessagesCount = newPendingMessagesCount; - } - - /// - /// Gets the synchronous task which need process new messages. - /// - internal JoinableTask SynchronousTask { get; } - - /// - /// Gets one JoinableTask which may have pending messages. We may have multiple new JoinableTasks which contains pending messages. - /// This is just one of them. It gives the synchronous task a way to start quickly without searching all messages. - /// - internal JoinableTask TaskHasPendingMessages { get; } - - /// - /// Gets the total number of new pending messages. The real number could be less than that, but should not be more than that. - /// - internal int NewPendingMessagesCount { get; } - } - /// /// A single linked list to maintain synchronous JoinableTask depends on the current task, /// which may process the queue of the current task. diff --git a/src/Microsoft.VisualStudio.Threading/SmallDependencyMap`1.cs b/src/Microsoft.VisualStudio.Threading/SmallDependencyMap`1.cs new file mode 100644 index 000000000..ca4832009 --- /dev/null +++ b/src/Microsoft.VisualStudio.Threading/SmallDependencyMap`1.cs @@ -0,0 +1,325 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections; +using System.Collections.Generic; + +namespace Microsoft.VisualStudio.Threading; + +/// +/// Maps dependency keys to reference counts. +/// +/// The type of dependency keys. +/// +/// This collection is not thread-safe. All access must be synchronized by the caller. +/// +internal sealed class SmallDependencyMap : IReadOnlyCollection + where TKey : class +{ + private Dictionary? dictionary; + private TKey? key0; + private TKey? key1; + private TKey? key2; + private int value0; + private int value1; + private int value2; + private int count; + private int used; + private int freeList = -1; + private int version; + + /// + /// Gets the number of dependencies. + /// + public int Count => this.dictionary?.Count ?? this.count; + + /// + /// Gets the dependency keys. + /// + internal IReadOnlyCollection Keys => this; + + /// + /// Gets or sets the reference count for a dependency. + /// + /// The dependency key. + internal int this[TKey key] + { + get + { + if (this.TryGetValue(key, out int value)) + { + return value; + } + + throw new KeyNotFoundException(); + } + + set + { + Requires.NotNull(key, nameof(key)); + if (this.dictionary is object) + { + this.dictionary[key] = value; + return; + } + + int index = this.FindEntry(key); + if (index < 0) + { + if (this.count == 3) + { + // A dictionary initially sized for two entries grows from three to seven on the fourth add. + // Keeping the first three entries here avoids its initial bucket and entry arrays entirely. + var expanded = new Dictionary(capacity: 4) + { + [this.key0!] = this.value0, + [this.key1!] = this.value1, + [this.key2!] = this.value2, + [key] = value, + }; + this.dictionary = expanded; + this.key0 = this.key1 = this.key2 = null; + this.version++; + return; + } + + if (this.freeList >= 0) + { + index = this.freeList; + this.freeList = this.GetValue(index); + } + else + { + index = this.used++; + } + + this.count++; + } + + this.SetEntry(index, key, value); + this.version++; + } + } + + /// + /// Gets an allocation-free enumerator over dependencies and reference counts. + /// + /// An enumerator over the entries. + public Enumerator GetEnumerator() => new Enumerator(this); + + IEnumerator IEnumerable.GetEnumerator() + { + return this.dictionary is object + ? ((IEnumerable)this.dictionary.Keys).GetEnumerator() + : new KeyEnumerator(this); + } + + IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable)this).GetEnumerator(); + + /// + /// Gets the reference count for a dependency if present. + /// + /// The dependency key. + /// Receives the reference count, or zero if the dependency is absent. + /// Whether the dependency is present. + internal bool TryGetValue(TKey key, out int value) + { + Requires.NotNull(key, nameof(key)); + if (this.dictionary is object) + { + return this.dictionary.TryGetValue(key, out value); + } + + int index = this.FindEntry(key); + value = index >= 0 ? this.GetValue(index) : 0; + return index >= 0; + } + + /// + /// Checks whether a dependency is present. + /// + /// The dependency key. + /// Whether the dependency is present. + internal bool ContainsKey(TKey key) => this.TryGetValue(key, out _); + + /// + /// Removes a dependency. + /// + /// The dependency key. + /// Whether the dependency was removed. + internal bool Remove(TKey key) + { + Requires.NotNull(key, nameof(key)); + if (this.dictionary is object) + { + return this.dictionary.Remove(key); + } + + int index = this.FindEntry(key); + if (index < 0) + { + return false; + } + + this.SetEntry(index, null, this.freeList); + this.freeList = index; + this.count--; + this.version++; + return true; + } + + private int FindEntry(TKey key) + { + EqualityComparer comparer = EqualityComparer.Default; + if (this.key0 is object && comparer.Equals(this.key0, key)) + { + return 0; + } + + if (this.key1 is object && comparer.Equals(this.key1, key)) + { + return 1; + } + + if (this.key2 is object && comparer.Equals(this.key2, key)) + { + return 2; + } + + return -1; + } + + private TKey? GetKey(int index) => index == 0 ? this.key0 : index == 1 ? this.key1 : this.key2; + + private int GetValue(int index) => index == 0 ? this.value0 : index == 1 ? this.value1 : this.value2; + + private void SetEntry(int index, TKey? key, int value) + { + switch (index) + { + case 0: + this.key0 = key; + this.value0 = value; + break; + case 1: + this.key1 = key; + this.value1 = value; + break; + default: + this.key2 = key; + this.value2 = value; + break; + } + } + + private void CheckVersion(int expectedVersion) + { + if (this.version != expectedVersion) + { + throw new InvalidOperationException(); + } + } + + /// + /// Enumerates dependencies and reference counts without allocating. + /// + internal struct Enumerator + { + private readonly SmallDependencyMap owner; + private readonly int version; + private Dictionary.Enumerator dictionaryEnumerator; + private int index; + + /// + /// Initializes a new instance of the struct. + /// + /// The map to enumerate. + internal Enumerator(SmallDependencyMap owner) + { + this.owner = owner; + this.version = owner.version; + this.dictionaryEnumerator = owner.dictionary?.GetEnumerator() ?? default; + this.index = -1; + this.Current = default; + } + + /// + /// Gets the current entry. + /// + public KeyValuePair Current { get; private set; } + + /// + /// Advances to the next entry. + /// + /// Whether another entry is available. + public bool MoveNext() + { + this.owner.CheckVersion(this.version); + if (this.owner.dictionary is object) + { + bool found = this.dictionaryEnumerator.MoveNext(); + this.Current = this.dictionaryEnumerator.Current; + return found; + } + + while (++this.index < this.owner.used) + { + if (this.owner.GetKey(this.index) is TKey key) + { + this.Current = new KeyValuePair(key, this.owner.GetValue(this.index)); + return true; + } + } + + this.Current = default; + return false; + } + } + + private struct KeyEnumerator : IEnumerator + { + private readonly SmallDependencyMap owner; + private readonly int version; + private int index; + + internal KeyEnumerator(SmallDependencyMap owner) + { + this.owner = owner; + this.version = owner.version; + this.index = -1; + this.Current = null!; + } + + public TKey Current { get; private set; } + + object IEnumerator.Current => this.Current ?? throw new InvalidOperationException(); + + public bool MoveNext() + { + this.owner.CheckVersion(this.version); + while (++this.index < this.owner.used) + { + if (this.owner.GetKey(this.index) is TKey key) + { + this.Current = key; + return true; + } + } + + this.Current = null!; + return false; + } + + public void Reset() + { + this.owner.CheckVersion(this.version); + this.index = -1; + this.Current = null!; + } + + public void Dispose() + { + } + } +} diff --git a/test/Microsoft.VisualStudio.Threading.Tests/InternalUtilitiesTests.cs b/test/Microsoft.VisualStudio.Threading.Tests/InternalUtilitiesTests.cs index bec84f2c4..e9bc7ee36 100644 --- a/test/Microsoft.VisualStudio.Threading.Tests/InternalUtilitiesTests.cs +++ b/test/Microsoft.VisualStudio.Threading.Tests/InternalUtilitiesTests.cs @@ -1,11 +1,163 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System; using System.Collections.Generic; using System.Linq; +using System.Threading; +using System.Threading.Tasks; public class InternalUtilitiesTests { + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(64)] + public void RentListAllocatesRequestedCapacityWhenEmpty(int capacity) + { + List? pool = null; + List list = InternalUtilities.RentList(ref pool, capacity); + Assert.Empty(list); + Assert.Equal(capacity, list.Capacity); + Assert.Null(pool); + } + + [Fact] + public void ReturnListClearsItemsAndRetainsCapacity() + { + List? pool = null; + var list = new List(4) { new object() }; + InternalUtilities.ReturnList(ref pool, list, maximumRetainedCapacity: 4); + + Assert.Same(list, pool); + Assert.Empty(list); + Assert.Equal(4, list.Capacity); + Assert.Same(list, InternalUtilities.RentList(ref pool, capacity: 4)); + Assert.Null(pool); + } + + [Fact] + public void RentListGrowsToRequestedCapacity() + { + var cached = new List(1); + List? pool = cached; + List list = InternalUtilities.RentList(ref pool, capacity: 5); + Assert.Same(cached, list); + Assert.Equal(5, list.Capacity); + Assert.Null(pool); + } + + [Fact] + public void ReturnListDoesNotRetainOversizedLists() + { + List? pool = null; + var list = new List(5) { new object() }; + InternalUtilities.ReturnList(ref pool, list, maximumRetainedCapacity: 4); + Assert.Null(pool); + Assert.Empty(list); + } + + [Fact] + public void ReturnListDoesNotReplaceAvailableList() + { + var cached = new List(1); + List? pool = cached; + var list = new List(1) { new object() }; + InternalUtilities.ReturnList(ref pool, list, maximumRetainedCapacity: 4); + Assert.Same(cached, pool); + Assert.Empty(list); + } + + [Fact] + public void RentListKeepsNestedRentalsSeparate() + { + List? pool = new List(1); + List outer = InternalUtilities.RentList(ref pool, capacity: 1); + var value = new object(); + outer.Add(value); + List inner = InternalUtilities.RentList(ref pool, capacity: 1); + Assert.NotSame(outer, inner); + inner.Add(new object()); + InternalUtilities.ReturnList(ref pool, inner, maximumRetainedCapacity: 4); + Assert.Same(value, Assert.Single(outer)); + InternalUtilities.ReturnList(ref pool, outer, maximumRetainedCapacity: 4); + Assert.Same(inner, pool); + } + + [Fact] + public void RentListRejectsNegativeCapacityWithoutTakingAvailableList() + { + var cached = new List(1); + List? pool = cached; + Assert.Throws(() => InternalUtilities.RentList(ref pool, capacity: -1)); + Assert.Same(cached, pool); + } + + [Fact] + public void ReturnListRejectsNegativeRetentionLimit() + { + List? pool = null; + var list = new List(1) { new object() }; + Assert.Throws(() => InternalUtilities.ReturnList(ref pool, list, maximumRetainedCapacity: -1)); + Assert.Null(pool); + Assert.Single(list); + } + + [Fact] + public async Task RentListKeepsConcurrentRentalsSeparate() + { + const int Concurrency = 4; + List? pool = new List(1); + using var barrier = new Barrier(Concurrency); + var tasks = new Task[Concurrency]; + for (int i = 0; i < tasks.Length; i++) + { + int value = i; + tasks[i] = Task.Run(delegate + { + List list = InternalUtilities.RentList(ref pool, capacity: 1); + try + { + list.Add(value); + Assert.True(barrier.SignalAndWait(TimeSpan.FromSeconds(30))); + Assert.Equal(value, Assert.Single(list)); + } + finally + { + InternalUtilities.ReturnList(ref pool, list, maximumRetainedCapacity: 4); + } + }); + } + + await Task.WhenAll(tasks); + } + +#if NET + [Fact] + public void RentAndReturnListDoNotAllocateWhenWarm() + { + List? pool = new List(1); + var value = new object(); + for (int i = 0; i < 100; i++) + { + List list = InternalUtilities.RentList(ref pool, capacity: 1); + list.Add(value); + InternalUtilities.ReturnList(ref pool, list, maximumRetainedCapacity: 4); + } + + long allocatedBefore = GC.GetAllocatedBytesForCurrentThread(); + for (int i = 0; i < 1000; i++) + { + List list = InternalUtilities.RentList(ref pool, capacity: 1); + list.Add(value); + InternalUtilities.ReturnList(ref pool, list, maximumRetainedCapacity: 4); + } + + long allocatedBytes = GC.GetAllocatedBytesForCurrentThread() - allocatedBefore; + Assert.Equal(0, allocatedBytes); + } +#endif + [Fact] public void RemoveMidQueue_Empty() { diff --git a/test/Microsoft.VisualStudio.Threading.Tests/JoinableTaskCollectionTests.cs b/test/Microsoft.VisualStudio.Threading.Tests/JoinableTaskCollectionTests.cs index 1cba90a05..ebe9f20b4 100644 --- a/test/Microsoft.VisualStudio.Threading.Tests/JoinableTaskCollectionTests.cs +++ b/test/Microsoft.VisualStudio.Threading.Tests/JoinableTaskCollectionTests.cs @@ -125,6 +125,75 @@ public void JoinTillEmptyAsync_CancellationToken() }); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public void JoinWithPendingMainThreadWork(bool hasPendingWork) + { + JoinableTaskCollection collection = this.context.CreateCollection(); + JoinableTaskFactory factory = this.context.CreateFactory(collection); + var workQueued = new AsyncManualResetEvent(); + var finishWork = new AsyncManualResetEvent(); + using var resumeMainThread = new ManualResetEventSlim(); + var waitingFactory = new PausedWaitJoinableTaskFactory(this.context, resumeMainThread, this.TimeoutToken); + Task childStarted = Task.Run(() => factory.RunAsync(async delegate + { + if (hasPendingWork) + { + await factory.SwitchToMainThreadAsync(this.TimeoutToken).GetAwaiter().YieldAndNotify(workQueued); + } + else + { + workQueued.Set(); + await finishWork.WaitAsync(this.TimeoutToken); + } + })); + + waitingFactory.Run(async delegate + { + await TaskScheduler.Default.SwitchTo(alwaysYield: true); + try + { + JoinableTask child = await childStarted; + await workQueued.WaitAsync(this.TimeoutToken); + await waitingFactory.WaitedOnce.WaitAsync(this.TimeoutToken); + + for (int i = 0; i < 100; i++) + { + collection.Join().Dispose(); + } + +#if NET + const int Iterations = 1000; + long allocatedBefore = GC.GetAllocatedBytesForCurrentThread(); + for (int i = 0; i < Iterations; i++) + { + collection.Join().Dispose(); + } + + long allocatedBytes = GC.GetAllocatedBytesForCurrentThread() - allocatedBefore; + this.Logger.WriteLine("Pending work: {0}; allocated bytes per join: {1}", hasPendingWork, allocatedBytes / Iterations); + + // Only dependency-tracking and event-pulse allocations remain after warm-up. + long maximumBytesPerJoin = hasPendingWork ? 240 : 144; + Assert.InRange(allocatedBytes, 0, maximumBytesPerJoin * Iterations); +#endif + + finishWork.Set(); + using (collection.Join()) + { + resumeMainThread.Set(); + await child.JoinAsync(this.TimeoutToken); + } + } + finally + { + finishWork.Set(); + resumeMainThread.Set(); + } + }); + } + [Fact] public void AddTwiceRemoveOnceRemovesWhenNotRefCounting() { @@ -210,4 +279,26 @@ public void JoinTillEmptyWorksWithRefCounting() Task waiter = collection.JoinTillEmptyAsync(); Assert.True(waiter.Wait(UnexpectedTimeout)); } + + private sealed class PausedWaitJoinableTaskFactory : JoinableTaskFactory + { + private readonly ManualResetEventSlim resumeMainThread; + private readonly CancellationToken cancellationToken; + + internal PausedWaitJoinableTaskFactory(JoinableTaskContext context, ManualResetEventSlim resumeMainThread, CancellationToken cancellationToken) + : base(context) + { + this.resumeMainThread = resumeMainThread; + this.cancellationToken = cancellationToken; + } + + internal AsyncManualResetEvent WaitedOnce { get; } = new AsyncManualResetEvent(); + + protected override void WaitSynchronously(Task task) + { + this.WaitedOnce.Set(); + this.resumeMainThread.Wait(this.cancellationToken); + base.WaitSynchronously(task); + } + } } diff --git a/test/Microsoft.VisualStudio.Threading.Tests/Microsoft.VisualStudio.Threading.Tests.csproj b/test/Microsoft.VisualStudio.Threading.Tests/Microsoft.VisualStudio.Threading.Tests.csproj index c89dfa97e..2e33b52cf 100644 --- a/test/Microsoft.VisualStudio.Threading.Tests/Microsoft.VisualStudio.Threading.Tests.csproj +++ b/test/Microsoft.VisualStudio.Threading.Tests/Microsoft.VisualStudio.Threading.Tests.csproj @@ -27,6 +27,9 @@ RarelyRemoveItemSet`1.cs + + SmallDependencyMap`1.cs + WeakKeyDictionary`2.cs diff --git a/test/Microsoft.VisualStudio.Threading.Tests/SmallDependencyMapTests.cs b/test/Microsoft.VisualStudio.Threading.Tests/SmallDependencyMapTests.cs new file mode 100644 index 000000000..b50ffb321 --- /dev/null +++ b/test/Microsoft.VisualStudio.Threading.Tests/SmallDependencyMapTests.cs @@ -0,0 +1,274 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; + +public class SmallDependencyMapTests +{ + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + [InlineData(8)] + [InlineData(32)] + public void AddUpdateRemoveAndEnumerate(int count) + { + var map = new SmallDependencyMap(); + object[] keys = CreateKeys(count); + Assert.Empty(map.Keys); + for (int i = 0; i < keys.Length; i++) + { + map[keys[i]] = i; + } + + Assert.Equal(keys.Length, map.Count); + Assert.Equal(keys, map.Keys); + for (int i = 0; i < keys.Length; i++) + { + Assert.True(map.ContainsKey(keys[i])); + Assert.True(map.TryGetValue(keys[i], out int value)); + Assert.Equal(i, value); + map[keys[i]] = i + 10; + } + + var entries = new List>(); + foreach (KeyValuePair entry in map) + { + entries.Add(entry); + } + + Assert.Equal(keys.Length, entries.Count); + for (int i = 0; i < entries.Count; i++) + { + Assert.Same(keys[i], entries[i].Key); + Assert.Equal(i + 10, entries[i].Value); + Assert.True(map.Remove(keys[i])); + Assert.False(map.Remove(keys[i])); + Assert.False(map.ContainsKey(keys[i])); + Assert.False(map.TryGetValue(keys[i], out int value)); + Assert.Equal(0, value); + } + + Assert.Empty(map.Keys); + Assert.Throws(() => map[new object()]); + } + + [Fact] + public void ReusesRemovedSlotsInDictionaryOrder() + { + var map = new SmallDependencyMap(); + var expected = new Dictionary(capacity: 2); + object[] keys = CreateKeys(6); + for (int i = 0; i < 3; i++) + { + map[keys[i]] = expected[keys[i]] = i; + } + + Assert.True(map.Remove(keys[0])); + Assert.True(map.Remove(keys[2])); + expected.Remove(keys[0]); + expected.Remove(keys[2]); + for (int i = 3; i < keys.Length; i++) + { + map[keys[i]] = expected[keys[i]] = i; + Assert.Equal(expected.Keys, map.Keys); + } + } + + [Fact] + public void MatchesDictionaryForMixedOperations() + { + var map = new SmallDependencyMap(); + var expected = new Dictionary(capacity: 2); + object[] keys = CreateKeys(32); + var random = new Random(1); + for (int operation = 0; operation < 1000; operation++) + { + object key = keys[random.Next(keys.Length)]; + if (random.Next(3) == 0) + { + Assert.Equal(expected.Remove(key), map.Remove(key)); + } + else + { + map[key] = expected[key] = operation; + } + + Assert.Equal(expected.Count, map.Count); + Assert.Equal(expected.Keys, map.Keys); + foreach (object candidate in keys) + { + Assert.Equal(expected.TryGetValue(candidate, out int expectedValue), map.TryGetValue(candidate, out int value)); + Assert.Equal(expectedValue, value); + } + } + } + + [Fact] + public void UsesDefaultKeyEquality() + { + var map = new SmallDependencyMap(); + string first = new string('a', 1); + string equal = new string('a', 1); + Assert.NotSame(first, equal); + map[first] = 1; + map[equal] = 2; + Assert.Single(map.Keys); + Assert.Equal(2, map[first]); + Assert.True(map.Remove(equal)); + Assert.Empty(map.Keys); + } + + [Fact] + public void RejectsNullKeys() + { + var map = new SmallDependencyMap(); + Assert.Throws(() => map[null!] = 1); + Assert.Throws(() => map.TryGetValue(null!, out _)); + Assert.Throws(() => map.Remove(null!)); + } + + [Theory] + [InlineData(1)] + [InlineData(3)] + public void EnumerationDetectsMutationIncludingPromotion(int count) + { + var map = new SmallDependencyMap(); + foreach (object key in CreateKeys(count)) + { + map[key] = 1; + } + + SmallDependencyMap.Enumerator entries = map.GetEnumerator(); + using IEnumerator keys = map.Keys.GetEnumerator(); + Assert.True(entries.MoveNext()); + Assert.True(keys.MoveNext()); + map[new object()] = 2; + Assert.Throws(() => entries.MoveNext()); + Assert.Throws(() => keys.MoveNext()); + } + + [Fact] + public void NonGenericKeyEnumeratorValidatesCurrentAndResets() + { + var map = new SmallDependencyMap(); + var key = new object(); + map[key] = 1; + IEnumerator enumerator = ((IEnumerable)map.Keys).GetEnumerator(); + Assert.Throws(() => enumerator.Current); + Assert.True(enumerator.MoveNext()); + Assert.Same(key, enumerator.Current); + Assert.False(enumerator.MoveNext()); + Assert.Throws(() => enumerator.Current); + enumerator.Reset(); + Assert.True(enumerator.MoveNext()); + Assert.Same(key, enumerator.Current); + } + +#if NET + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + [InlineData(7)] + [InlineData(8)] + [InlineData(17)] + [InlineData(100)] + public void AllocatesNoMoreThanDictionaryAtGrowthBoundaries(int count) + { + object[] keys = CreateKeys(count); + const int Iterations = 100; + FillMap(keys); + FillDictionary(keys); + long allocatedBefore = GC.GetAllocatedBytesForCurrentThread(); + for (int i = 0; i < Iterations; i++) + { + FillMap(keys); + } + + long mapBytes = GC.GetAllocatedBytesForCurrentThread() - allocatedBefore; + allocatedBefore = GC.GetAllocatedBytesForCurrentThread(); + for (int i = 0; i < Iterations; i++) + { + FillDictionary(keys); + } + + long dictionaryBytes = GC.GetAllocatedBytesForCurrentThread() - allocatedBefore; + Assert.InRange(mapBytes, 0, dictionaryBytes); + } + + [Theory] + [InlineData(1)] + [InlineData(3)] + [InlineData(4)] + [InlineData(32)] + public void EnumerationAllocatesNoMoreThanDictionary(int count) + { + object[] keys = CreateKeys(count); + SmallDependencyMap map = FillMap(keys); + Dictionary dictionary = FillDictionary(keys); + const int Iterations = 100; + long allocatedBefore = GC.GetAllocatedBytesForCurrentThread(); + for (int i = 0; i < Iterations; i++) + { + foreach (KeyValuePair entry in map) + { + GC.KeepAlive(entry.Key); + } + } + + Assert.Equal(0, GC.GetAllocatedBytesForCurrentThread() - allocatedBefore); + allocatedBefore = GC.GetAllocatedBytesForCurrentThread(); + for (int i = 0; i < Iterations; i++) + { + foreach (object key in map.Keys) + { + GC.KeepAlive(key); + } + } + + long mapBytes = GC.GetAllocatedBytesForCurrentThread() - allocatedBefore; + allocatedBefore = GC.GetAllocatedBytesForCurrentThread(); + for (int i = 0; i < Iterations; i++) + { + foreach (object key in (IReadOnlyCollection)dictionary.Keys) + { + GC.KeepAlive(key); + } + } + + long dictionaryBytes = GC.GetAllocatedBytesForCurrentThread() - allocatedBefore; + Assert.InRange(mapBytes, 0, dictionaryBytes); + } +#endif + + private static object[] CreateKeys(int count) => Enumerable.Range(0, count).Select(_ => new object()).ToArray(); + + private static SmallDependencyMap FillMap(object[] keys) + { + var map = new SmallDependencyMap(); + for (int i = 0; i < keys.Length; i++) + { + map[keys[i]] = i; + } + + return map; + } + + private static Dictionary FillDictionary(object[] keys) + { + var dictionary = new Dictionary(capacity: 2); + for (int i = 0; i < keys.Length; i++) + { + dictionary[keys[i]] = i; + } + + return dictionary; + } +}