From 8b5b05977603c81392441d02289614705172cdb3 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 12 Dec 2020 21:52:09 +0100 Subject: [PATCH 01/50] Add PriorityQueue to System.Collections.Generic (#43957) This commit adds a new data structure, priority queue. Fixes #43957 --- .../ref/System.Collections.cs | 44 +++++ .../src/System.Collections.csproj | 1 + .../Collections/Generic/PriorityQueue.cs | 177 ++++++++++++++++++ .../PriorityQueue.Generic.Tests.cs | 17 ++ .../PriorityQueue/PriorityQueue.Generic.cs | 21 +++ .../PriorityQueue/PriorityQueue.Tests.cs | 17 ++ .../tests/System.Collections.Tests.csproj | 3 + 7 files changed, 280 insertions(+) create mode 100644 src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs create mode 100644 src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs create mode 100644 src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.cs create mode 100644 src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs diff --git a/src/libraries/System.Collections/ref/System.Collections.cs b/src/libraries/System.Collections/ref/System.Collections.cs index 405eea4de0a4a3..33b57dfe256d6c 100644 --- a/src/libraries/System.Collections/ref/System.Collections.cs +++ b/src/libraries/System.Collections/ref/System.Collections.cs @@ -378,6 +378,50 @@ public void Dispose() { } void System.Collections.IEnumerator.Reset() { } } } + + public partial class PriorityQueue + { + public PriorityQueue() { } + public PriorityQueue(int initialCapacity) { } + public PriorityQueue(System.Collections.Generic.IComparer? comparer) { } + public PriorityQueue(int initialCapacity, System.Collections.Generic.IComparer? comparer) { } + public PriorityQueue(System.Collections.Generic.IEnumerable<(TElement Element, TPriority Priority)> values) { } + public PriorityQueue(System.Collections.Generic.IEnumerable<(TElement Element, TPriority Priority)> values, System.Collections.Generic.IComparer? comparer) { } + public int Count { get { throw null; } } + public System.Collections.Generic.IComparer Comparer { get { throw null; } } + public void Enqueue(TElement element, TPriority priority) { } + public TElement Peek() { throw null; } + public TElement Dequeue() { throw null; } + public bool TryDequeue([System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TElement element, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TPriority priority) { throw null; } + public bool TryPeek([System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TElement element, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TPriority priority) { throw null; } + public TElement EnqueueDequeue(TElement element, TPriority priority) { throw null; } + public void EnqueueRange(System.Collections.Generic.IEnumerable<(TElement Element, TPriority Priority)> values) { } + public void EnqueueRange(System.Collections.Generic.IEnumerable values, TPriority priority) { } + public void Clear() { } + public void EnsureCapacity(int capacity) { } + public void TrimExcess() { } + public System.Collections.Generic.PriorityQueue.UnorderedItemsCollection UnorderedItems { get; } + public partial class UnorderedItemsCollection : System.Collections.Generic.IReadOnlyCollection<(TElement Element, TPriority Priority)>, System.Collections.ICollection + { + public int Count { get { throw null; } } + bool System.Collections.ICollection.IsSynchronized { get { throw null; } } + object System.Collections.ICollection.SyncRoot { get { throw null; } } + public void CopyTo(Array array, int index) { } + public partial struct Enumerator : IEnumerator<(TElement TElement, TPriority Priority)>, IEnumerator + { + (TElement TElement, TPriority Priority) IEnumerator<(TElement TElement, TPriority Priority)>.Current { get { throw null; } } + object IEnumerator.Current { get { throw null; } } + + void IDisposable.Dispose() { } + bool IEnumerator.MoveNext() { throw null; } + void IEnumerator.Reset() { } + } + public System.Collections.Generic.PriorityQueue.UnorderedItemsCollection.Enumerator GetEnumerator() { throw null; } + System.Collections.Generic.IEnumerator<(TElement Element, TPriority Priority)> System.Collections.Generic.IEnumerable<(TElement Element, TPriority Priority)>.GetEnumerator() { throw null; } + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; } + } + } + public partial class Queue : System.Collections.Generic.IEnumerable, System.Collections.Generic.IReadOnlyCollection, System.Collections.ICollection, System.Collections.IEnumerable { public Queue() { } diff --git a/src/libraries/System.Collections/src/System.Collections.csproj b/src/libraries/System.Collections/src/System.Collections.csproj index 9f8e4b8bb6295d..c8fd9f78b68ec0 100644 --- a/src/libraries/System.Collections/src/System.Collections.csproj +++ b/src/libraries/System.Collections/src/System.Collections.csproj @@ -13,6 +13,7 @@ + diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs new file mode 100644 index 00000000000000..3e30aa4b94e881 --- /dev/null +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -0,0 +1,177 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics.CodeAnalysis; + +namespace System.Collections.Generic +{ + [System.Runtime.CompilerServices.TypeForwardedFrom("System, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")] + public class PriorityQueue + { + /// + /// Creates an empty PriorityQueue instance. + /// + public PriorityQueue() + { + throw new NotImplementedException(); + } + + /// + /// Creates a PriorityQueue instance with specified initial capacity in its backing array. + /// + public PriorityQueue(int initialCapacity) + { + throw new NotImplementedException(); + } + + /// + /// Creates a PriorityQueue instance with specified priority comparer. + /// + public PriorityQueue(IComparer? comparer) + { + throw new NotImplementedException(); + } + + public PriorityQueue(int initialCapacity, IComparer? comparer) + { + throw new NotImplementedException(); + } + + /// + /// Creates a PriorityQueue populated with the specified values and priorities. + /// + public PriorityQueue(IEnumerable<(TElement Element, TPriority Priority)> values) + { + throw new NotImplementedException(); + } + + public PriorityQueue(IEnumerable<(TElement Element, TPriority Priority)> values, IComparer? comparer) + { + throw new NotImplementedException(); + } + + /// + /// Gets the current element count in the queue. + /// + public int Count => throw new NotImplementedException(); + + /// + /// Gets the priority comparer of the queue. + /// + public IComparer Comparer => throw new NotImplementedException(); + + /// + /// Enqueues the element with specified priority. + /// + public void Enqueue(TElement element, TPriority priority) + { + throw new NotImplementedException(); + } + + /// + /// Gets the element with minimal priority, if it exists. + /// + /// The queue is empty. + public TElement Peek() + { + throw new NotImplementedException(); + } + + /// + /// Dequeues the element with minimal priority, if it exists. + /// + /// The queue is empty. + public TElement Dequeue() + { + throw new NotImplementedException(); + } + + /// + /// Try-variants of Dequeue and Peek methods. + /// + public bool TryDequeue([MaybeNullWhen(false)] out TElement element, [MaybeNullWhen(false)] out TPriority priority) + { + throw new NotImplementedException(); + } + public bool TryPeek([MaybeNullWhen(false)] out TElement element, [MaybeNullWhen(false)] out TPriority priority) + { + throw new NotImplementedException(); + } + + /// + /// Combined enqueue/dequeue operation, generally more efficient than sequential Enqueue/Dequeue calls. + /// + public TElement EnqueueDequeue(TElement element, TPriority priority) + { + throw new NotImplementedException(); + } + + /// + /// Enqueues a sequence of element/priority pairs to the queue. + /// + public void EnqueueRange(IEnumerable<(TElement Element, TPriority Priority)> values) + { + throw new NotImplementedException(); + } + + /// + /// Enqueues a sequence of elements with provided priority. + /// + public void EnqueueRange(IEnumerable values, TPriority priority) + { + throw new NotImplementedException(); + } + + /// + /// Removes all objects from the PriorityQueue. + /// + public void Clear() + { + throw new NotImplementedException(); + } + + /// + /// Ensures that the PriorityQueue can hold the specified capacity and resizes its underlying buffer if necessary. + /// + public void EnsureCapacity(int capacity) + { + throw new NotImplementedException(); + } + + /// + /// Sets capacity to the actual number of elements in the queue, if that is less than 90 percent of current capacity. + /// + public void TrimExcess() + { + throw new NotImplementedException(); + } + + /// + /// Gets a collection that enumerates the elements of the queue. + /// + public UnorderedItemsCollection UnorderedItems { get; } + + public class UnorderedItemsCollection : IReadOnlyCollection<(TElement Element, TPriority Priority)>, ICollection + { + public int Count => throw new NotImplementedException(); + object ICollection.SyncRoot => throw new NotImplementedException(); + bool ICollection.IsSynchronized => throw new NotImplementedException(); + + public void CopyTo(Array array, int index) => throw new NotImplementedException(); + + public struct Enumerator : IEnumerator<(TElement TElement, TPriority Priority)>, IEnumerator + { + (TElement TElement, TPriority Priority) IEnumerator<(TElement TElement, TPriority Priority)>.Current => throw new NotImplementedException(); + object IEnumerator.Current => throw new NotImplementedException(); + + void IDisposable.Dispose() => throw new NotImplementedException(); + bool IEnumerator.MoveNext() => throw new NotImplementedException(); + void IEnumerator.Reset() => throw new NotImplementedException(); + } + + public Enumerator GetEnumerator() => throw new NotImplementedException(); + IEnumerator<(TElement Element, TPriority Priority)> IEnumerable<(TElement Element, TPriority Priority)>.GetEnumerator() => throw new NotImplementedException(); + IEnumerator IEnumerable.GetEnumerator() => throw new NotImplementedException(); + } + } +} diff --git a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs new file mode 100644 index 00000000000000..31ab44deb2d134 --- /dev/null +++ b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using Xunit; + +namespace System.Collections.Tests +{ + public abstract class PriorityQueue_Generic_Tests : TestBase<(TElement, TPriority)> + { + [Fact] + public void ConstructorThrows() + { + Assert.Throws(() => new PriorityQueue()); + } + } +} diff --git a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.cs b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.cs new file mode 100644 index 00000000000000..3f5307e81538ca --- /dev/null +++ b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.cs @@ -0,0 +1,21 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Collections.Tests +{ + public class PriorityQueue_Generic_Tests_string_string : PriorityQueue_Generic_Tests + { + protected override (string, string) CreateT(int seed) + { + throw new NotImplementedException(); + } + } + + public class PriorityQueue_Generic_Tests_int_int : PriorityQueue_Generic_Tests + { + protected override (int, int) CreateT(int seed) + { + throw new NotImplementedException(); + } + } +} diff --git a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs new file mode 100644 index 00000000000000..3bafdbc160fc48 --- /dev/null +++ b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using Xunit; + +namespace System.Collections.Tests +{ + public class PriorityQueue_NonGeneric_Tests : TestBase + { + [Fact] + public void ConstructorThrows() + { + Assert.Throws(() => new PriorityQueue()); + } + } +} diff --git a/src/libraries/System.Collections/tests/System.Collections.Tests.csproj b/src/libraries/System.Collections/tests/System.Collections.Tests.csproj index 412f68fc8425f9..eb646000406169 100644 --- a/src/libraries/System.Collections/tests/System.Collections.Tests.csproj +++ b/src/libraries/System.Collections/tests/System.Collections.Tests.csproj @@ -95,6 +95,9 @@ + + + From 9658a1be85763cf4db7b5d94024a90b66698bc63 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Tue, 29 Dec 2020 13:41:30 +0100 Subject: [PATCH 02/50] (draft step, to squash) Modify API reference In this commit, I modified the API reference using [these guidelines](https://github.com/dotnet/runtime/blob/4d784693ebc5f91c7eede32170046355ef3969b2/docs/coding-guidelines/updating-ref-source.md), following the advice from @danmosemsft. The automatically generated code (with `dotnet msbuild /t:GenerateReferenceAssemblySource`) didn't build out of the box and I tweaked it manually. --- .../ref/System.Collections.cs | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/src/libraries/System.Collections/ref/System.Collections.cs b/src/libraries/System.Collections/ref/System.Collections.cs index 33b57dfe256d6c..6a6f927635cb52 100644 --- a/src/libraries/System.Collections/ref/System.Collections.cs +++ b/src/libraries/System.Collections/ref/System.Collections.cs @@ -382,43 +382,43 @@ void System.Collections.IEnumerator.Reset() { } public partial class PriorityQueue { public PriorityQueue() { } - public PriorityQueue(int initialCapacity) { } public PriorityQueue(System.Collections.Generic.IComparer? comparer) { } + public PriorityQueue(System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)> values) { } + public PriorityQueue(System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)> values, System.Collections.Generic.IComparer? comparer) { } + public PriorityQueue(int initialCapacity) { } public PriorityQueue(int initialCapacity, System.Collections.Generic.IComparer? comparer) { } - public PriorityQueue(System.Collections.Generic.IEnumerable<(TElement Element, TPriority Priority)> values) { } - public PriorityQueue(System.Collections.Generic.IEnumerable<(TElement Element, TPriority Priority)> values, System.Collections.Generic.IComparer? comparer) { } - public int Count { get { throw null; } } public System.Collections.Generic.IComparer Comparer { get { throw null; } } - public void Enqueue(TElement element, TPriority priority) { } - public TElement Peek() { throw null; } + public int Count { get { throw null; } } + public System.Collections.Generic.PriorityQueue.UnorderedItemsCollection UnorderedItems { get { throw null; } } + public void Clear() { } public TElement Dequeue() { throw null; } - public bool TryDequeue([System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TElement element, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TPriority priority) { throw null; } - public bool TryPeek([System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TElement element, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TPriority priority) { throw null; } + public void Enqueue(TElement element, TPriority priority) { } public TElement EnqueueDequeue(TElement element, TPriority priority) { throw null; } - public void EnqueueRange(System.Collections.Generic.IEnumerable<(TElement Element, TPriority Priority)> values) { } + public void EnqueueRange(System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)> values) { } public void EnqueueRange(System.Collections.Generic.IEnumerable values, TPriority priority) { } - public void Clear() { } public void EnsureCapacity(int capacity) { } + public TElement Peek() { throw null; } public void TrimExcess() { } - public System.Collections.Generic.PriorityQueue.UnorderedItemsCollection UnorderedItems { get; } - public partial class UnorderedItemsCollection : System.Collections.Generic.IReadOnlyCollection<(TElement Element, TPriority Priority)>, System.Collections.ICollection + public bool TryDequeue([System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TElement element, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TPriority priority) { throw null; } + public bool TryPeek([System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TElement element, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TPriority priority) { throw null; } + public partial class UnorderedItemsCollection : System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)>, System.Collections.Generic.IReadOnlyCollection<(TElement element, TPriority priority)>, System.Collections.ICollection, System.Collections.IEnumerable { + public UnorderedItemsCollection() { } public int Count { get { throw null; } } bool System.Collections.ICollection.IsSynchronized { get { throw null; } } object System.Collections.ICollection.SyncRoot { get { throw null; } } - public void CopyTo(Array array, int index) { } - public partial struct Enumerator : IEnumerator<(TElement TElement, TPriority Priority)>, IEnumerator - { - (TElement TElement, TPriority Priority) IEnumerator<(TElement TElement, TPriority Priority)>.Current { get { throw null; } } - object IEnumerator.Current { get { throw null; } } - - void IDisposable.Dispose() { } - bool IEnumerator.MoveNext() { throw null; } - void IEnumerator.Reset() { } - } + public void CopyTo(System.Array array, int index) { } public System.Collections.Generic.PriorityQueue.UnorderedItemsCollection.Enumerator GetEnumerator() { throw null; } - System.Collections.Generic.IEnumerator<(TElement Element, TPriority Priority)> System.Collections.Generic.IEnumerable<(TElement Element, TPriority Priority)>.GetEnumerator() { throw null; } + System.Collections.Generic.IEnumerator<(TElement element, TPriority priority)> System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)>.GetEnumerator() { throw null; } System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; } + public partial struct Enumerator : System.Collections.Generic.IEnumerator<(TElement element, TPriority priority)>, System.Collections.IEnumerator, System.IDisposable + { + (TElement element, TPriority priority) IEnumerator<(TElement element, TPriority priority)>.Current { get { throw null; } } + object System.Collections.IEnumerator.Current { get { throw null; } } + bool System.Collections.IEnumerator.MoveNext() { throw null; } + void System.Collections.IEnumerator.Reset() { } + void System.IDisposable.Dispose() { } + } } } From dda9697979d7fe9c88eb7435d70a199a4a168e9b Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Tue, 29 Dec 2020 16:08:21 +0100 Subject: [PATCH 03/50] (draft step, to squash) Add tests for PriorityQueue Added generic tests for and . Removed non-generic tests. --- .../PriorityQueue.Generic.Tests.cs | 278 +++++++++++++++++- .../PriorityQueue/PriorityQueue.Generic.cs | 19 +- .../PriorityQueue/PriorityQueue.Tests.cs | 17 -- .../tests/System.Collections.Tests.csproj | 1 - 4 files changed, 293 insertions(+), 22 deletions(-) delete mode 100644 src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs diff --git a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs index 31ab44deb2d134..d26f69a9992b77 100644 --- a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs +++ b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs @@ -2,16 +2,290 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Linq; using Xunit; namespace System.Collections.Tests { public abstract class PriorityQueue_Generic_Tests : TestBase<(TElement, TPriority)> { + #region Helper methods + + protected IEnumerable<(TElement, TPriority)> GenericIEnumerableFactory(int count) + { + int seed = count * 34; + for (int i = 0; i < count; i++) + { + yield return CreateT(seed++); + } + } + + protected PriorityQueue GenericPriorityQueueFactory( + int initialCapacity, int countOfItemsToGenerate, out List<(TElement element, TPriority priority)> generatedItems) + { + generatedItems = this.GenericIEnumerableFactory(countOfItemsToGenerate).ToList(); + + var queue = new PriorityQueue(initialCapacity); + foreach (var (element, priority) in generatedItems) + { + queue.Enqueue(element, priority); + } + + return queue; + } + + #endregion + + #region Constructors + + [Fact] + public void PriorityQueue_Generic_Constructor() + { + var queue = new PriorityQueue(); + + Assert.Equal(expected: 0, queue.Count); + Assert.Empty(queue.UnorderedItems); + Assert.Equal(queue.Comparer, Comparer.Default); + } + + [Theory] + [MemberData(nameof(ValidCollectionSizes))] + public void PriorityQueue_Generic_Constructor_int(int initialCapacity) + { + var queue = new PriorityQueue(initialCapacity); + + Assert.Empty(queue.UnorderedItems); + } + + [Fact] + public void PriorityQueue_Generic_Constructor_int_Negative_ThrowsArgumentOutOfRangeException() + { + AssertExtensions.Throws("initialCapacity", () => new PriorityQueue(-1)); + AssertExtensions.Throws("initialCapacity", () => new PriorityQueue(int.MinValue)); + } + + [Fact] + public void PriorityQueue_Generic_Constructor_IComparer() + { + IComparer comparer = Comparer.Default; + var queue = new PriorityQueue(comparer); + + Assert.Equal(comparer, queue.Comparer); + } + + [Fact] + public void PriorityQueue_Generic_Constructor_IComparer_Null() + { + var queue = new PriorityQueue((IComparer)null); + Assert.Equal(Comparer.Default, queue.Comparer); + } + + [Theory] + [MemberData(nameof(ValidCollectionSizes))] + public void PriorityQueue_Generic_Constructor_int_IComparer(int initialCapacity) + { + IComparer comparer = Comparer.Default; + var queue = new PriorityQueue(initialCapacity); + + Assert.Empty(queue.UnorderedItems); + Assert.Equal(comparer, queue.Comparer); + } + + [Theory] + [MemberData(nameof(ValidCollectionSizes))] + public void PriorityQueue_Generic_Constructor_IEnumerable(int count) + { + HashSet<(TElement, TPriority)> itemsToEnqueue = this.GenericIEnumerableFactory(count).ToHashSet(); + PriorityQueue queue = new PriorityQueue(itemsToEnqueue); + Assert.True(itemsToEnqueue.SetEquals(queue.UnorderedItems)); + } + + #endregion + + #region Enqueue, Dequeue, Peek + + [Theory] + [MemberData(nameof(ValidCollectionSizes))] + public void PriorityQueue_Generic_Enqueue(int count) + { + PriorityQueue queue = GenericPriorityQueueFactory(count, count, out var generatedItems); + HashSet<(TElement, TPriority)> expectedItems = generatedItems.ToHashSet(); + + Assert.Equal(count, queue.Count); + var actualItems = queue.UnorderedItems.ToArray(); + foreach (var (element, priority) in actualItems) + { + Assert.True(expectedItems.Contains((element, priority))); + } + } + + [Fact] + public void PriorityQueue_Generic_Dequeue_EmptyCollection() + { + var queue = new PriorityQueue(); + + Assert.False(queue.TryDequeue(out _, out _)); + Assert.Throws(() => queue.Dequeue()); + } + [Fact] - public void ConstructorThrows() + public void PriorityQueue_Generic_Peek_EmptyCollection() + { + var queue = new PriorityQueue(); + + Assert.False(queue.TryPeek(out _, out _)); + Assert.Throws(() => queue.Peek()); + } + + [Theory] + [MemberData(nameof(ValidPositiveCollectionSizes))] + public void PriorityQueue_Generic_Peek_PositiveCount(int count) + { + IReadOnlyCollection<(TElement, TPriority)> itemsToEnqueue = this.GenericIEnumerableFactory(count).ToArray(); + (TElement element, TPriority priority) expectedPeek = itemsToEnqueue.First(); + PriorityQueue queue = new PriorityQueue(); + + foreach (var (element, priority) in itemsToEnqueue) + { + if (queue.Comparer.Compare(priority, expectedPeek.priority) < 0) + { + expectedPeek = (element, priority); + } + + queue.Enqueue(element, priority); + + var actualPeekElement = queue.Peek(); + var actualTryPeekSuccess = queue.TryPeek(out TElement actualTryPeekElement, out TPriority actualTryPeekPriority); + + Assert.Equal(expectedPeek.element, actualPeekElement); + Assert.True(actualTryPeekSuccess); + Assert.Equal(expectedPeek.element, actualTryPeekElement); + Assert.Equal(expectedPeek.priority, actualTryPeekPriority); + } + } + + [Theory] + [InlineData(0, 5)] + [InlineData(1, 1)] + [InlineData(3, 100)] + public void PriorityQueue_Generic_PeekAndDequeue(int initialCapacity, int count) { - Assert.Throws(() => new PriorityQueue()); + PriorityQueue queue = this.GenericPriorityQueueFactory(initialCapacity, count, out var generatedItems); + + var expectedPeekPriorities = generatedItems + .Select(x => x.priority) + .OrderBy(x => x, queue.Comparer) + .ToArray(); + + for (var i = 0; i < count; ++i) + { + var expectedPeekPriority = expectedPeekPriorities[i]; + + var actualTryPeekSuccess = queue.TryPeek(out TElement actualTryPeekElement, out TPriority actualTryPeekPriority); + var actualTryDequeueSuccess = queue.TryDequeue(out TElement actualTryDequeueElement, out TPriority actualTryDequeuePriority); + + Assert.True(actualTryPeekSuccess); + Assert.True(actualTryDequeueSuccess); + Assert.Equal(expectedPeekPriority, actualTryPeekPriority); + Assert.Equal(expectedPeekPriority, actualTryDequeuePriority); + } + + Assert.Equal(expected: 0, queue.Count); + Assert.False(queue.TryPeek(out _, out _)); + Assert.False(queue.TryDequeue(out _, out _)); } + + [Theory] + [MemberData(nameof(ValidCollectionSizes))] + public void PriorityQueue_Generic_EnqueueRange_IEnumerable(int count) + { + HashSet<(TElement, TPriority)> itemsToEnqueue = this.GenericIEnumerableFactory(count).ToHashSet(); + PriorityQueue queue = new PriorityQueue(); + + queue.EnqueueRange(itemsToEnqueue); + + Assert.True(itemsToEnqueue.SetEquals(queue.UnorderedItems)); + } + + #endregion + + #region Clear + + [Theory] + [MemberData(nameof(ValidCollectionSizes))] + public void PriorityQueue_Generic_Clear(int count) + { + PriorityQueue queue = this.GenericPriorityQueueFactory(initialCapacity: 0, count, out _); + + Assert.Equal(count, queue.Count); + queue.Clear(); + Assert.Equal(expected: 0, queue.Count); + } + + #endregion + + #region EnsureCapacity, TrimExcess + + [Theory] + [InlineData(0, 0)] + [InlineData(0, 5)] + [InlineData(1, 1)] + [InlineData(3, 100)] + public void PriorityQueue_Generic_TrimExcess_ValidQueueThatHasntBeenRemovedFrom(int initialCapacity, int count) + { + PriorityQueue queue = this.GenericPriorityQueueFactory(initialCapacity, count, out _); + queue.TrimExcess(); + } + + [Theory] + [MemberData(nameof(ValidCollectionSizes))] + public void PriorityQueue_Generic_TrimExcess_Repeatedly(int count) + { + PriorityQueue queue = this.GenericPriorityQueueFactory(initialCapacity: 0, count, out _); + + Assert.Equal(count, queue.Count); + queue.TrimExcess(); + queue.TrimExcess(); + queue.TrimExcess(); + Assert.Equal(count, queue.Count); + } + + [Theory] + [MemberData(nameof(ValidPositiveCollectionSizes))] + public void PriorityQueue_Generic_EnsureCapacityAndTrimExcess(int count) + { + IReadOnlyCollection<(TElement, TPriority)> itemsToEnqueue = this.GenericIEnumerableFactory(count).ToArray(); + PriorityQueue queue = new PriorityQueue(); + int expectedCount = 0; + Random random = new Random(Seed: 34); + int getNextEnsureCapacity() => random.Next(0, count * 2); + void trimAndEnsureCapacity() + { + queue.TrimExcess(); + queue.EnsureCapacity(getNextEnsureCapacity()); + queue.TrimExcess(); + }; + + foreach (var (element, priority) in itemsToEnqueue) + { + trimAndEnsureCapacity(); + queue.Enqueue(element, priority); + expectedCount++; + Assert.Equal(expectedCount, queue.Count); + } + + while (expectedCount > 0) + { + queue.Dequeue(); + trimAndEnsureCapacity(); + expectedCount--; + Assert.Equal(expectedCount, queue.Count); + } + + trimAndEnsureCapacity(); + Assert.Equal(0, queue.Count); + } + + #endregion } } diff --git a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.cs b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.cs index 3f5307e81538ca..624649bdbaa6fb 100644 --- a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.cs +++ b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.cs @@ -7,7 +7,18 @@ public class PriorityQueue_Generic_Tests_string_string : PriorityQueue_Generic_T { protected override (string, string) CreateT(int seed) { - throw new NotImplementedException(); + var element = this.CreateString(seed); + var priority = this.CreateString(seed); + return (element, priority); + } + + protected string CreateString(int seed) + { + int stringLength = seed % 10 + 5; + Random rand = new Random(seed); + byte[] bytes = new byte[stringLength]; + rand.NextBytes(bytes); + return Convert.ToBase64String(bytes); } } @@ -15,7 +26,11 @@ public class PriorityQueue_Generic_Tests_int_int : PriorityQueue_Generic_Tests new Random(seed).Next(); } } diff --git a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs deleted file mode 100644 index 3bafdbc160fc48..00000000000000 --- a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs +++ /dev/null @@ -1,17 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Collections.Generic; -using Xunit; - -namespace System.Collections.Tests -{ - public class PriorityQueue_NonGeneric_Tests : TestBase - { - [Fact] - public void ConstructorThrows() - { - Assert.Throws(() => new PriorityQueue()); - } - } -} diff --git a/src/libraries/System.Collections/tests/System.Collections.Tests.csproj b/src/libraries/System.Collections/tests/System.Collections.Tests.csproj index eb646000406169..46688df9c7cc85 100644 --- a/src/libraries/System.Collections/tests/System.Collections.Tests.csproj +++ b/src/libraries/System.Collections/tests/System.Collections.Tests.csproj @@ -97,7 +97,6 @@ - From bf8b2e7d53ceb5e62ec664a8bf364aa65af8267d Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Wed, 30 Dec 2020 00:39:06 +0100 Subject: [PATCH 04/50] (draft step, to squash) Add initial implementation This commit adds the core of the heap implementation for the priority queue. It doesn't implement the method `EnqueueDequeue`, as I'm not convinced by it. It also doesn't implement the method `CopyTo(Array array, int index)`, leaving this one for later. --- .../ref/System.Collections.cs | 10 +- .../Collections/Generic/PriorityQueue.cs | 440 +++++++++++++++--- 2 files changed, 393 insertions(+), 57 deletions(-) diff --git a/src/libraries/System.Collections/ref/System.Collections.cs b/src/libraries/System.Collections/ref/System.Collections.cs index 6a6f927635cb52..1c4d20f42479e6 100644 --- a/src/libraries/System.Collections/ref/System.Collections.cs +++ b/src/libraries/System.Collections/ref/System.Collections.cs @@ -383,8 +383,8 @@ public partial class PriorityQueue { public PriorityQueue() { } public PriorityQueue(System.Collections.Generic.IComparer? comparer) { } - public PriorityQueue(System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)> values) { } - public PriorityQueue(System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)> values, System.Collections.Generic.IComparer? comparer) { } + public PriorityQueue(System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)> items) { } + public PriorityQueue(System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)> items, System.Collections.Generic.IComparer? comparer) { } public PriorityQueue(int initialCapacity) { } public PriorityQueue(int initialCapacity, System.Collections.Generic.IComparer? comparer) { } public System.Collections.Generic.IComparer Comparer { get { throw null; } } @@ -394,8 +394,8 @@ public void Clear() { } public TElement Dequeue() { throw null; } public void Enqueue(TElement element, TPriority priority) { } public TElement EnqueueDequeue(TElement element, TPriority priority) { throw null; } - public void EnqueueRange(System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)> values) { } - public void EnqueueRange(System.Collections.Generic.IEnumerable values, TPriority priority) { } + public void EnqueueRange(System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)> items) { } + public void EnqueueRange(System.Collections.Generic.IEnumerable elements, TPriority priority) { } public void EnsureCapacity(int capacity) { } public TElement Peek() { throw null; } public void TrimExcess() { } @@ -403,7 +403,7 @@ public void TrimExcess() { } public bool TryPeek([System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TElement element, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TPriority priority) { throw null; } public partial class UnorderedItemsCollection : System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)>, System.Collections.Generic.IReadOnlyCollection<(TElement element, TPriority priority)>, System.Collections.ICollection, System.Collections.IEnumerable { - public UnorderedItemsCollection() { } + public UnorderedItemsCollection(IReadOnlyCollection<(TElement element, TPriority priority)> items) { } public int Count { get { throw null; } } bool System.Collections.ICollection.IsSynchronized { get { throw null; } } object System.Collections.ICollection.SyncRoot { get { throw null; } } diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 3e30aa4b94e881..42531dbad0bc51 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -2,104 +2,235 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; namespace System.Collections.Generic { - [System.Runtime.CompilerServices.TypeForwardedFrom("System, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")] + /// + /// Represents a data structure in which each element additionally has a priority + /// associated with it. + /// + /// The type of the element. + /// The type of the priority. public class PriorityQueue { /// - /// Creates an empty PriorityQueue instance. + /// Represents an implicit heap-ordered complete d-ary tree, stored as an array. + /// + private readonly List<(TElement element, TPriority priority)> nodes; + + private const int RootIndex = 0; + + /// + /// Specifies the arity of the d-ary heap, which here is quaternary. + /// + private const int Arity = 4; + + /// + /// Creates an empty priority queue. /// public PriorityQueue() { - throw new NotImplementedException(); + this.nodes = new List<(TElement, TPriority)>(); + this.UnorderedItems = new UnorderedItemsCollection(this.nodes); + this.Comparer = Comparer.Default; } /// - /// Creates a PriorityQueue instance with specified initial capacity in its backing array. + /// Creates an empty priority queue with the specified initial capacity for its underlying array. /// public PriorityQueue(int initialCapacity) { - throw new NotImplementedException(); + if (initialCapacity < 0) + { + throw new ArgumentOutOfRangeException(nameof(initialCapacity)); + } + + this.nodes = new List<(TElement, TPriority)>(initialCapacity); + this.UnorderedItems = new UnorderedItemsCollection(this.nodes); + this.Comparer = Comparer.Default; } /// - /// Creates a PriorityQueue instance with specified priority comparer. + /// Creates an empty priority queue with the specified priority comparer. /// public PriorityQueue(IComparer? comparer) { - throw new NotImplementedException(); + this.nodes = new List<(TElement, TPriority)>(); + this.UnorderedItems = new UnorderedItemsCollection(this.nodes); + this.Comparer = comparer ?? Comparer.Default; } + /// + /// Creates an empty priority queue with the specified priority comparer and + /// the specified initial capacity for its underlying array. + /// public PriorityQueue(int initialCapacity, IComparer? comparer) { - throw new NotImplementedException(); + if (initialCapacity < 0) + { + throw new ArgumentOutOfRangeException(nameof(initialCapacity)); + } + + this.nodes = new List<(TElement, TPriority)>(initialCapacity); + this.UnorderedItems = new UnorderedItemsCollection(this.nodes); + this.Comparer = comparer ?? Comparer.Default; } /// - /// Creates a PriorityQueue populated with the specified values and priorities. + /// Creates a priority queue populated with the specified elements and priorities. /// - public PriorityQueue(IEnumerable<(TElement Element, TPriority Priority)> values) + public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items) { - throw new NotImplementedException(); + if (items is null) + { + throw new ArgumentNullException(nameof(items)); + } + + this.nodes = new List<(TElement, TPriority)>(items); + this.UnorderedItems = new UnorderedItemsCollection(this.nodes); + this.Comparer = Comparer.Default; + + if (this.nodes.Count > 1) + { + this.Heapify(); + } } - public PriorityQueue(IEnumerable<(TElement Element, TPriority Priority)> values, IComparer? comparer) + /// + /// Creates a priority queue populated with the specified elements and priorities, + /// and with the specified priority comparer. + /// + public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items, IComparer? comparer) { - throw new NotImplementedException(); + if (items is null) + { + throw new ArgumentNullException(nameof(items)); + } + + this.nodes = new List<(TElement, TPriority)>(items); + this.UnorderedItems = new UnorderedItemsCollection(this.nodes); + this.Comparer = comparer ?? Comparer.Default; + + if (this.nodes.Count > 1) + { + this.Heapify(); + } } /// - /// Gets the current element count in the queue. + /// Gets the current amount of items in the priority queue. /// - public int Count => throw new NotImplementedException(); + public int Count => this.nodes.Count; /// - /// Gets the priority comparer of the queue. + /// Gets the priority comparer of the priority queue. /// - public IComparer Comparer => throw new NotImplementedException(); + public IComparer Comparer { get; } /// - /// Enqueues the element with specified priority. + /// Enqueues the specified element and associates it with the specified priority. /// public void Enqueue(TElement element, TPriority priority) { - throw new NotImplementedException(); + if (element is null) + { + throw new ArgumentNullException(nameof(element)); + } + + if (priority is null) + { + throw new ArgumentNullException(nameof(priority)); + } + + // Add the node at the end + var node = (element, priority); + this.nodes.Add(node); + + // Restore the heap order + var lastNodeIndex = this.GetLastNodeIndex(); + this.MoveUp(node, lastNodeIndex); } /// - /// Gets the element with minimal priority, if it exists. + /// Gets the element associated with the minimal priority. /// /// The queue is empty. public TElement Peek() { - throw new NotImplementedException(); + if (this.TryPeek(out var element, out var priority)) + { + return element; + } + else + { + throw new InvalidOperationException( + "The priority queue is empty, cannot get the element with minimal priority."); + } } /// - /// Dequeues the element with minimal priority, if it exists. + /// Dequeues the element associated with the minimal priority. /// /// The queue is empty. public TElement Dequeue() { - throw new NotImplementedException(); + if (this.TryDequeue(out var element, out var priority)) + { + return element; + } + else + { + throw new InvalidOperationException( + "The priority queue is empty, cannot dequeue an element."); + } } /// - /// Try-variants of Dequeue and Peek methods. + /// Dequeues the element associated with the minimal priority /// + /// + /// if the priority queue is non-empty; otherwise. + /// public bool TryDequeue([MaybeNullWhen(false)] out TElement element, [MaybeNullWhen(false)] out TPriority priority) { - throw new NotImplementedException(); + if (this.nodes.Count == 0) + { + element = default; + priority = default; + return false; + } + else + { + (element, priority) = this.nodes[RootIndex]; + this.Remove(RootIndex); + return true; + } } + + /// + /// Gets the element associated with the minimal priority. + /// + /// + /// if the priority queue is non-empty; otherwise. + /// public bool TryPeek([MaybeNullWhen(false)] out TElement element, [MaybeNullWhen(false)] out TPriority priority) { - throw new NotImplementedException(); + if (this.nodes.Count == 0) + { + element = default; + priority = default; + return false; + } + else + { + (element, priority) = this.nodes[RootIndex]; + return true; + } } /// - /// Combined enqueue/dequeue operation, generally more efficient than sequential Enqueue/Dequeue calls. + /// Combined enqueue/dequeue operation, generally more efficient than sequential Enqueue/Dequeue calls. /// public TElement EnqueueDequeue(TElement element, TPriority priority) { @@ -107,71 +238,276 @@ public TElement EnqueueDequeue(TElement element, TPriority priority) } /// - /// Enqueues a sequence of element/priority pairs to the queue. + /// Enqueues a collection of element/priority pairs. /// - public void EnqueueRange(IEnumerable<(TElement Element, TPriority Priority)> values) + public void EnqueueRange(IEnumerable<(TElement Element, TPriority Priority)> items) { - throw new NotImplementedException(); + if (items is null) + { + throw new ArgumentNullException(nameof(items)); + } + + foreach (var (element, priority) in items) + { + this.Enqueue(element, priority); + } } /// - /// Enqueues a sequence of elements with provided priority. + /// Enqueues a collection of elements, each associated with the specified priority. /// - public void EnqueueRange(IEnumerable values, TPriority priority) + public void EnqueueRange(IEnumerable elements, TPriority priority) { - throw new NotImplementedException(); + if (elements is null) + { + throw new ArgumentNullException(nameof(elements)); + } + + foreach (var element in elements) + { + this.Enqueue(element, priority); + } } /// - /// Removes all objects from the PriorityQueue. + /// Removes all items from the priority queue. /// public void Clear() { - throw new NotImplementedException(); + this.nodes.Clear(); } /// - /// Ensures that the PriorityQueue can hold the specified capacity and resizes its underlying buffer if necessary. + /// Ensures that the priority queue has the specified capacity + /// and resizes its underlying array if necessary. /// public void EnsureCapacity(int capacity) { - throw new NotImplementedException(); + if (capacity < 0) + { + throw new ArgumentOutOfRangeException(nameof(capacity)); + } + + if (capacity <= this.nodes.Count) + { + return; + } + + this.nodes.Capacity = capacity; } /// - /// Sets capacity to the actual number of elements in the queue, if that is less than 90 percent of current capacity. + /// Sets the capacity to the actual number of items in the priority queue, + /// if that is less than 90 percent of current capacity. /// public void TrimExcess() { - throw new NotImplementedException(); + this.nodes.TrimExcess(); + } + + /// + /// Removes the node at the specified index. + /// + private void Remove(int indexOfNodeToRemove) + { + // The idea is to replace the specified node by the very last + // node and shorten the array by one. + + var lastNodeIndex = this.GetLastNodeIndex(); + var lastNode = this.nodes[lastNodeIndex]; + this.nodes.RemoveAt(lastNodeIndex); + + // In case we wanted to remove the node that was the last one, + // we are done. + + if (indexOfNodeToRemove == lastNodeIndex) + { + return; + } + + // Our last node was erased from the array and needs to be + // inserted again. Of course, we will overwrite the node we + // wanted to remove. After that operation, we will need + // to restore the heap property (in general). + + var nodeToRemove = this.nodes[indexOfNodeToRemove]; + + var relation = this.Comparer.Compare(lastNode.priority, nodeToRemove.priority); + this.PutAt(lastNode, indexOfNodeToRemove); + + if (relation < 0) + { + this.MoveUp(lastNode, indexOfNodeToRemove); + } + else + { + this.MoveDown(lastNode, indexOfNodeToRemove); + } + } + + /// + /// Puts a node at the specified index. + /// + private void PutAt((TElement element, TPriority priority) node, int index) + { + this.nodes[index] = node; } /// - /// Gets a collection that enumerates the elements of the queue. + /// Gets the index of the last node in the heap. + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private int GetLastNodeIndex() => this.nodes.Count - 1; + + /// + /// Gets the index of an element's parent. + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private int GetParentIndex(int index) => (index - 1) / Arity; + + /// + /// Gets the index of the first child of an element. + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private int GetFirstChildIndex(int index) => Arity * index + 1; + + /// + /// Converts an unordered list into a heap. + /// + private void Heapify() + { + // Leaves of the tree are in fact 1-element heaps, for which there + // is no need to correct them. The heap property needs to be restored + // only for higher nodes, starting from the first node that has children. + // It is the parent of the very last element in the array. + + var lastNodeIndex = this.GetLastNodeIndex(); + var lastParentWithChildren = this.GetParentIndex(lastNodeIndex); + + for (var index = lastParentWithChildren; index >= 0; --index) + { + this.MoveDown(this.nodes[index], index); + } + } + + /// + /// Moves a node up in the tree to restore heap order. + /// + private void MoveUp((TElement element, TPriority priority) node, int nodeIndex) + { + // Instead of swapping items all the way to the root, we will perform + // a similar optimization as in the insertion sort. + + while (nodeIndex > 0) + { + var parentIndex = this.GetParentIndex(nodeIndex); + var parent = this.nodes[parentIndex]; + + if (this.Comparer.Compare(node.priority, parent.priority) < 0) + { + this.PutAt(parent, nodeIndex); + nodeIndex = parentIndex; + } + else + { + break; + } + } + + this.PutAt(node, nodeIndex); + } + + /// + /// Moves a node down in the tree to restore heap order. + /// + private void MoveDown((TElement element, TPriority priority) node, int nodeIndex) + { + // The node to move down will not actually be swapped every time. + // Rather, values on the affected path will be moved up, thus leaving a free spot + // for this value to drop in. Similar optimization as in the insertion sort. + + int i; + while ((i = this.GetFirstChildIndex(nodeIndex)) < this.nodes.Count) + { + // Check if the current node (pointed by 'nodeIndex') should really be extracted + // first, or maybe one of its children should be extracted earlier. + var topChild = this.nodes[i]; + var childrenIndexesLimit = Math.Min(i + Arity, this.nodes.Count); + int topChildIndex = i; + + while (++i < childrenIndexesLimit) + { + var child = this.nodes[i]; + if (this.Comparer.Compare(child.priority, topChild.priority) < 0) + { + topChild = child; + topChildIndex = i; + } + } + + // In case no child needs to be extracted earlier than the current node, + // there is nothing more to do - the right spot was found. + if (this.Comparer.Compare(node.priority, topChild.priority) <= 0) + { + break; + } + + // Move the top child up by one node and now investigate the + // node that was considered to be the top child (recursive). + this.PutAt(topChild, nodeIndex); + nodeIndex = topChildIndex; + } + + this.PutAt(node, nodeIndex); + } + + /// + /// Gets a collection that enumerates the elements of the queue. /// public UnorderedItemsCollection UnorderedItems { get; } - public class UnorderedItemsCollection : IReadOnlyCollection<(TElement Element, TPriority Priority)>, ICollection + public partial class UnorderedItemsCollection : IReadOnlyCollection<(TElement Element, TPriority Priority)>, ICollection { - public int Count => throw new NotImplementedException(); - object ICollection.SyncRoot => throw new NotImplementedException(); - bool ICollection.IsSynchronized => throw new NotImplementedException(); + private readonly IReadOnlyCollection<(TElement Element, TPriority Priority)> items; + + public UnorderedItemsCollection(IReadOnlyCollection<(TElement element, TPriority priority)> items) + { + this.items = items; + } + + public int Count => this.items.Count; + object ICollection.SyncRoot => this; + bool ICollection.IsSynchronized => false; public void CopyTo(Array array, int index) => throw new NotImplementedException(); public struct Enumerator : IEnumerator<(TElement TElement, TPriority Priority)>, IEnumerator { - (TElement TElement, TPriority Priority) IEnumerator<(TElement TElement, TPriority Priority)>.Current => throw new NotImplementedException(); - object IEnumerator.Current => throw new NotImplementedException(); + private readonly IEnumerator<(TElement TElement, TPriority Priority)> enumerator; + + internal Enumerator(System.Collections.Generic.IEnumerator<(TElement TElement, TPriority Priority)> enumerator) + { + this.enumerator = enumerator; + } - void IDisposable.Dispose() => throw new NotImplementedException(); - bool IEnumerator.MoveNext() => throw new NotImplementedException(); - void IEnumerator.Reset() => throw new NotImplementedException(); + (TElement TElement, TPriority Priority) IEnumerator<(TElement TElement, TPriority Priority)>.Current + => this.enumerator.Current; + + object IEnumerator.Current => this.enumerator.Current; + + void IDisposable.Dispose() => this.enumerator.Dispose(); + bool IEnumerator.MoveNext() => this.enumerator.MoveNext(); + void IEnumerator.Reset() => this.enumerator.Reset(); } - public Enumerator GetEnumerator() => throw new NotImplementedException(); - IEnumerator<(TElement Element, TPriority Priority)> IEnumerable<(TElement Element, TPriority Priority)>.GetEnumerator() => throw new NotImplementedException(); - IEnumerator IEnumerable.GetEnumerator() => throw new NotImplementedException(); + public Enumerator GetEnumerator() + => new Enumerator(this.items.GetEnumerator()); + + IEnumerator<(TElement Element, TPriority Priority)> IEnumerable<(TElement Element, TPriority Priority)>.GetEnumerator() + => new Enumerator(this.items.GetEnumerator()); + + IEnumerator IEnumerable.GetEnumerator() + => new Enumerator(this.items.GetEnumerator()); } } } From 55f4e305e1de4cd1a2555f666bfc582068cf7a18 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Wed, 30 Dec 2020 00:52:26 +0100 Subject: [PATCH 05/50] (draft step, to squash) Rename parameters --- .../System/Collections/Generic/PriorityQueue.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 42531dbad0bc51..9245b9622ce637 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -348,6 +348,7 @@ private void Remove(int indexOfNodeToRemove) /// /// Puts a node at the specified index. /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] private void PutAt((TElement element, TPriority priority) node, int index) { this.nodes[index] = node; @@ -466,9 +467,9 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex /// public UnorderedItemsCollection UnorderedItems { get; } - public partial class UnorderedItemsCollection : IReadOnlyCollection<(TElement Element, TPriority Priority)>, ICollection + public partial class UnorderedItemsCollection : IReadOnlyCollection<(TElement element, TPriority priority)>, ICollection { - private readonly IReadOnlyCollection<(TElement Element, TPriority Priority)> items; + private readonly IReadOnlyCollection<(TElement element, TPriority priority)> items; public UnorderedItemsCollection(IReadOnlyCollection<(TElement element, TPriority priority)> items) { @@ -481,16 +482,16 @@ public UnorderedItemsCollection(IReadOnlyCollection<(TElement element, TPriority public void CopyTo(Array array, int index) => throw new NotImplementedException(); - public struct Enumerator : IEnumerator<(TElement TElement, TPriority Priority)>, IEnumerator + public struct Enumerator : IEnumerator<(TElement element, TPriority priority)>, IEnumerator { - private readonly IEnumerator<(TElement TElement, TPriority Priority)> enumerator; + private readonly IEnumerator<(TElement element, TPriority priority)> enumerator; - internal Enumerator(System.Collections.Generic.IEnumerator<(TElement TElement, TPriority Priority)> enumerator) + internal Enumerator(System.Collections.Generic.IEnumerator<(TElement element, TPriority priority)> enumerator) { this.enumerator = enumerator; } - (TElement TElement, TPriority Priority) IEnumerator<(TElement TElement, TPriority Priority)>.Current + (TElement element, TPriority priority) IEnumerator<(TElement element, TPriority priority)>.Current => this.enumerator.Current; object IEnumerator.Current => this.enumerator.Current; @@ -503,7 +504,7 @@ internal Enumerator(System.Collections.Generic.IEnumerator<(TElement TElement, T public Enumerator GetEnumerator() => new Enumerator(this.items.GetEnumerator()); - IEnumerator<(TElement Element, TPriority Priority)> IEnumerable<(TElement Element, TPriority Priority)>.GetEnumerator() + IEnumerator<(TElement element, TPriority priority)> IEnumerable<(TElement element, TPriority priority)>.GetEnumerator() => new Enumerator(this.items.GetEnumerator()); IEnumerator IEnumerable.GetEnumerator() From ad531feca1a48fd764d55410844bef6d3d5b9629 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Fri, 8 Jan 2021 21:10:15 +0100 Subject: [PATCH 06/50] (draft step, to squash) Replace `this.nodes` with `_nodes` --- .../Collections/Generic/PriorityQueue.cs | 72 +++++++++---------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 9245b9622ce637..18156bd5d6ab32 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -17,7 +17,7 @@ public class PriorityQueue /// /// Represents an implicit heap-ordered complete d-ary tree, stored as an array. /// - private readonly List<(TElement element, TPriority priority)> nodes; + private readonly List<(TElement element, TPriority priority)> _nodes; private const int RootIndex = 0; @@ -31,8 +31,8 @@ public class PriorityQueue /// public PriorityQueue() { - this.nodes = new List<(TElement, TPriority)>(); - this.UnorderedItems = new UnorderedItemsCollection(this.nodes); + _nodes = new List<(TElement, TPriority)>(); + this.UnorderedItems = new UnorderedItemsCollection(_nodes); this.Comparer = Comparer.Default; } @@ -46,8 +46,8 @@ public PriorityQueue(int initialCapacity) throw new ArgumentOutOfRangeException(nameof(initialCapacity)); } - this.nodes = new List<(TElement, TPriority)>(initialCapacity); - this.UnorderedItems = new UnorderedItemsCollection(this.nodes); + _nodes = new List<(TElement, TPriority)>(initialCapacity); + this.UnorderedItems = new UnorderedItemsCollection(_nodes); this.Comparer = Comparer.Default; } @@ -56,8 +56,8 @@ public PriorityQueue(int initialCapacity) /// public PriorityQueue(IComparer? comparer) { - this.nodes = new List<(TElement, TPriority)>(); - this.UnorderedItems = new UnorderedItemsCollection(this.nodes); + _nodes = new List<(TElement, TPriority)>(); + this.UnorderedItems = new UnorderedItemsCollection(_nodes); this.Comparer = comparer ?? Comparer.Default; } @@ -72,8 +72,8 @@ public PriorityQueue(int initialCapacity, IComparer? comparer) throw new ArgumentOutOfRangeException(nameof(initialCapacity)); } - this.nodes = new List<(TElement, TPriority)>(initialCapacity); - this.UnorderedItems = new UnorderedItemsCollection(this.nodes); + _nodes = new List<(TElement, TPriority)>(initialCapacity); + this.UnorderedItems = new UnorderedItemsCollection(_nodes); this.Comparer = comparer ?? Comparer.Default; } @@ -87,11 +87,11 @@ public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items) throw new ArgumentNullException(nameof(items)); } - this.nodes = new List<(TElement, TPriority)>(items); - this.UnorderedItems = new UnorderedItemsCollection(this.nodes); + _nodes = new List<(TElement, TPriority)>(items); + this.UnorderedItems = new UnorderedItemsCollection(_nodes); this.Comparer = Comparer.Default; - if (this.nodes.Count > 1) + if (_nodes.Count > 1) { this.Heapify(); } @@ -108,11 +108,11 @@ public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items, throw new ArgumentNullException(nameof(items)); } - this.nodes = new List<(TElement, TPriority)>(items); - this.UnorderedItems = new UnorderedItemsCollection(this.nodes); + _nodes = new List<(TElement, TPriority)>(items); + this.UnorderedItems = new UnorderedItemsCollection(_nodes); this.Comparer = comparer ?? Comparer.Default; - if (this.nodes.Count > 1) + if (_nodes.Count > 1) { this.Heapify(); } @@ -121,7 +121,7 @@ public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items, /// /// Gets the current amount of items in the priority queue. /// - public int Count => this.nodes.Count; + public int Count => _nodes.Count; /// /// Gets the priority comparer of the priority queue. @@ -145,7 +145,7 @@ public void Enqueue(TElement element, TPriority priority) // Add the node at the end var node = (element, priority); - this.nodes.Add(node); + _nodes.Add(node); // Restore the heap order var lastNodeIndex = this.GetLastNodeIndex(); @@ -194,7 +194,7 @@ public TElement Dequeue() /// public bool TryDequeue([MaybeNullWhen(false)] out TElement element, [MaybeNullWhen(false)] out TPriority priority) { - if (this.nodes.Count == 0) + if (_nodes.Count == 0) { element = default; priority = default; @@ -202,7 +202,7 @@ public bool TryDequeue([MaybeNullWhen(false)] out TElement element, [MaybeNullWh } else { - (element, priority) = this.nodes[RootIndex]; + (element, priority) = _nodes[RootIndex]; this.Remove(RootIndex); return true; } @@ -216,7 +216,7 @@ public bool TryDequeue([MaybeNullWhen(false)] out TElement element, [MaybeNullWh /// public bool TryPeek([MaybeNullWhen(false)] out TElement element, [MaybeNullWhen(false)] out TPriority priority) { - if (this.nodes.Count == 0) + if (_nodes.Count == 0) { element = default; priority = default; @@ -224,7 +224,7 @@ public bool TryPeek([MaybeNullWhen(false)] out TElement element, [MaybeNullWhen( } else { - (element, priority) = this.nodes[RootIndex]; + (element, priority) = _nodes[RootIndex]; return true; } } @@ -274,7 +274,7 @@ public void EnqueueRange(IEnumerable elements, TPriority priority) /// public void Clear() { - this.nodes.Clear(); + _nodes.Clear(); } /// @@ -288,12 +288,12 @@ public void EnsureCapacity(int capacity) throw new ArgumentOutOfRangeException(nameof(capacity)); } - if (capacity <= this.nodes.Count) + if (capacity <= _nodes.Count) { return; } - this.nodes.Capacity = capacity; + _nodes.Capacity = capacity; } /// @@ -302,7 +302,7 @@ public void EnsureCapacity(int capacity) /// public void TrimExcess() { - this.nodes.TrimExcess(); + _nodes.TrimExcess(); } /// @@ -314,8 +314,8 @@ private void Remove(int indexOfNodeToRemove) // node and shorten the array by one. var lastNodeIndex = this.GetLastNodeIndex(); - var lastNode = this.nodes[lastNodeIndex]; - this.nodes.RemoveAt(lastNodeIndex); + var lastNode = _nodes[lastNodeIndex]; + _nodes.RemoveAt(lastNodeIndex); // In case we wanted to remove the node that was the last one, // we are done. @@ -330,7 +330,7 @@ private void Remove(int indexOfNodeToRemove) // wanted to remove. After that operation, we will need // to restore the heap property (in general). - var nodeToRemove = this.nodes[indexOfNodeToRemove]; + var nodeToRemove = _nodes[indexOfNodeToRemove]; var relation = this.Comparer.Compare(lastNode.priority, nodeToRemove.priority); this.PutAt(lastNode, indexOfNodeToRemove); @@ -351,14 +351,14 @@ private void Remove(int indexOfNodeToRemove) [MethodImpl(MethodImplOptions.AggressiveInlining)] private void PutAt((TElement element, TPriority priority) node, int index) { - this.nodes[index] = node; + _nodes[index] = node; } /// /// Gets the index of the last node in the heap. /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - private int GetLastNodeIndex() => this.nodes.Count - 1; + private int GetLastNodeIndex() => _nodes.Count - 1; /// /// Gets the index of an element's parent. @@ -387,7 +387,7 @@ private void Heapify() for (var index = lastParentWithChildren; index >= 0; --index) { - this.MoveDown(this.nodes[index], index); + this.MoveDown(_nodes[index], index); } } @@ -402,7 +402,7 @@ private void MoveUp((TElement element, TPriority priority) node, int nodeIndex) while (nodeIndex > 0) { var parentIndex = this.GetParentIndex(nodeIndex); - var parent = this.nodes[parentIndex]; + var parent = _nodes[parentIndex]; if (this.Comparer.Compare(node.priority, parent.priority) < 0) { @@ -428,17 +428,17 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex // for this value to drop in. Similar optimization as in the insertion sort. int i; - while ((i = this.GetFirstChildIndex(nodeIndex)) < this.nodes.Count) + while ((i = this.GetFirstChildIndex(nodeIndex)) < _nodes.Count) { // Check if the current node (pointed by 'nodeIndex') should really be extracted // first, or maybe one of its children should be extracted earlier. - var topChild = this.nodes[i]; - var childrenIndexesLimit = Math.Min(i + Arity, this.nodes.Count); + var topChild = _nodes[i]; + var childrenIndexesLimit = Math.Min(i + Arity, _nodes.Count); int topChildIndex = i; while (++i < childrenIndexesLimit) { - var child = this.nodes[i]; + var child = _nodes[i]; if (this.Comparer.Compare(child.priority, topChild.priority) < 0) { topChild = child; From 28ca1e397ffd20158f5cbdc8e0ba7048da304478 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Fri, 8 Jan 2021 22:23:26 +0100 Subject: [PATCH 07/50] (draft step, to squash) Use an array and handle sizing ourselves --- .../ref/System.Collections.cs | 7 +- .../Collections/Generic/PriorityQueue.cs | 232 ++++++++++++++---- 2 files changed, 193 insertions(+), 46 deletions(-) diff --git a/src/libraries/System.Collections/ref/System.Collections.cs b/src/libraries/System.Collections/ref/System.Collections.cs index 1c4d20f42479e6..3861fef7a7b056 100644 --- a/src/libraries/System.Collections/ref/System.Collections.cs +++ b/src/libraries/System.Collections/ref/System.Collections.cs @@ -403,7 +403,7 @@ public void TrimExcess() { } public bool TryPeek([System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TElement element, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TPriority priority) { throw null; } public partial class UnorderedItemsCollection : System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)>, System.Collections.Generic.IReadOnlyCollection<(TElement element, TPriority priority)>, System.Collections.ICollection, System.Collections.IEnumerable { - public UnorderedItemsCollection(IReadOnlyCollection<(TElement element, TPriority priority)> items) { } + public UnorderedItemsCollection(PriorityQueue queue) { } public int Count { get { throw null; } } bool System.Collections.ICollection.IsSynchronized { get { throw null; } } object System.Collections.ICollection.SyncRoot { get { throw null; } } @@ -414,10 +414,11 @@ public void CopyTo(System.Array array, int index) { } public partial struct Enumerator : System.Collections.Generic.IEnumerator<(TElement element, TPriority priority)>, System.Collections.IEnumerator, System.IDisposable { (TElement element, TPriority priority) IEnumerator<(TElement element, TPriority priority)>.Current { get { throw null; } } + public void Dispose() { } + public bool MoveNext() { throw null; } + public (TElement element, TPriority priority) Current { get { throw null; } } object System.Collections.IEnumerator.Current { get { throw null; } } - bool System.Collections.IEnumerator.MoveNext() { throw null; } void System.Collections.IEnumerator.Reset() { } - void System.IDisposable.Dispose() { } } } } diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 18156bd5d6ab32..6e404a00a0885e 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; @@ -17,7 +18,20 @@ public class PriorityQueue /// /// Represents an implicit heap-ordered complete d-ary tree, stored as an array. /// - private readonly List<(TElement element, TPriority priority)> _nodes; + private (TElement element, TPriority priority)[] _nodes; + + /// + /// The number of nodes in the heap. + /// + private int _size; + + /// + /// Used to keep enumerator in sync with the collection. + /// + private int _version; + + private const int MinimumGrow = 4; + private const int GrowFactor = 200; // double each time private const int RootIndex = 0; @@ -31,8 +45,8 @@ public class PriorityQueue /// public PriorityQueue() { - _nodes = new List<(TElement, TPriority)>(); - this.UnorderedItems = new UnorderedItemsCollection(_nodes); + _nodes = Array.Empty<(TElement, TPriority)>(); + this.UnorderedItems = new UnorderedItemsCollection(this); this.Comparer = Comparer.Default; } @@ -46,8 +60,8 @@ public PriorityQueue(int initialCapacity) throw new ArgumentOutOfRangeException(nameof(initialCapacity)); } - _nodes = new List<(TElement, TPriority)>(initialCapacity); - this.UnorderedItems = new UnorderedItemsCollection(_nodes); + _nodes = new (TElement, TPriority)[initialCapacity]; + this.UnorderedItems = new UnorderedItemsCollection(this); this.Comparer = Comparer.Default; } @@ -56,8 +70,8 @@ public PriorityQueue(int initialCapacity) /// public PriorityQueue(IComparer? comparer) { - _nodes = new List<(TElement, TPriority)>(); - this.UnorderedItems = new UnorderedItemsCollection(_nodes); + _nodes = Array.Empty<(TElement, TPriority)>(); + this.UnorderedItems = new UnorderedItemsCollection(this); this.Comparer = comparer ?? Comparer.Default; } @@ -72,8 +86,8 @@ public PriorityQueue(int initialCapacity, IComparer? comparer) throw new ArgumentOutOfRangeException(nameof(initialCapacity)); } - _nodes = new List<(TElement, TPriority)>(initialCapacity); - this.UnorderedItems = new UnorderedItemsCollection(_nodes); + _nodes = new (TElement, TPriority)[initialCapacity]; + this.UnorderedItems = new UnorderedItemsCollection(this); this.Comparer = comparer ?? Comparer.Default; } @@ -87,11 +101,11 @@ public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items) throw new ArgumentNullException(nameof(items)); } - _nodes = new List<(TElement, TPriority)>(items); - this.UnorderedItems = new UnorderedItemsCollection(_nodes); + _nodes = EnumerableHelpers.ToArray(items, out _size); + this.UnorderedItems = new UnorderedItemsCollection(this); this.Comparer = Comparer.Default; - if (_nodes.Count > 1) + if (_size > 1) { this.Heapify(); } @@ -108,11 +122,11 @@ public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items, throw new ArgumentNullException(nameof(items)); } - _nodes = new List<(TElement, TPriority)>(items); - this.UnorderedItems = new UnorderedItemsCollection(_nodes); + _nodes = EnumerableHelpers.ToArray(items, out _size); + this.UnorderedItems = new UnorderedItemsCollection(this); this.Comparer = comparer ?? Comparer.Default; - if (_nodes.Count > 1) + if (_size > 1) { this.Heapify(); } @@ -121,7 +135,7 @@ public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items, /// /// Gets the current amount of items in the priority queue. /// - public int Count => _nodes.Count; + public int Count => _size; /// /// Gets the priority comparer of the priority queue. @@ -143,9 +157,13 @@ public void Enqueue(TElement element, TPriority priority) throw new ArgumentNullException(nameof(priority)); } + this.EnsureEnoughCapacityBeforeAddingNode(); + // Add the node at the end var node = (element, priority); - _nodes.Add(node); + _nodes[_size] = node; + _size++; + _version++; // Restore the heap order var lastNodeIndex = this.GetLastNodeIndex(); @@ -194,7 +212,7 @@ public TElement Dequeue() /// public bool TryDequeue([MaybeNullWhen(false)] out TElement element, [MaybeNullWhen(false)] out TPriority priority) { - if (_nodes.Count == 0) + if (_size == 0) { element = default; priority = default; @@ -216,7 +234,7 @@ public bool TryDequeue([MaybeNullWhen(false)] out TElement element, [MaybeNullWh /// public bool TryPeek([MaybeNullWhen(false)] out TElement element, [MaybeNullWhen(false)] out TPriority priority) { - if (_nodes.Count == 0) + if (_size == 0) { element = default; priority = default; @@ -274,7 +292,13 @@ public void EnqueueRange(IEnumerable elements, TPriority priority) /// public void Clear() { - _nodes.Clear(); + if (RuntimeHelpers.IsReferenceOrContainsReferences<(TElement, TPriority)>()) + { + // Clear the elements so that the gc can reclaim the references + Array.Clear(_nodes, 0, _size); + } + _size = 0; + _version++; } /// @@ -288,12 +312,12 @@ public void EnsureCapacity(int capacity) throw new ArgumentOutOfRangeException(nameof(capacity)); } - if (capacity <= _nodes.Count) + if (capacity <= _size) { return; } - _nodes.Capacity = capacity; + this.SetCapacity(capacity); } /// @@ -302,7 +326,40 @@ public void EnsureCapacity(int capacity) /// public void TrimExcess() { - _nodes.TrimExcess(); + int threshold = (int)(((double)_nodes.Length) * 0.9); + if (_size < threshold) + { + Array.Resize(ref _nodes, _size); + _version++; + } + } + + private void EnsureEnoughCapacityBeforeAddingNode() + { + if (_size == _nodes.Length) + { + int newCapacity = (int)((long)_nodes.Length * (long)GrowFactor / 100); + if (newCapacity < _nodes.Length + MinimumGrow) + { + newCapacity = _nodes.Length + MinimumGrow; + } + this.SetCapacity(newCapacity); + } + } + + /// + /// Grows or shrinks the array holding nodes. Capacity must be >= _size. + /// + private void SetCapacity(int capacity) + { + var newArray = new (TElement, TPriority)[capacity]; + + if (_size > 0) + { + Array.Copy(_nodes, 0, newArray, 0, _size); + } + + _nodes = newArray; } /// @@ -315,7 +372,8 @@ private void Remove(int indexOfNodeToRemove) var lastNodeIndex = this.GetLastNodeIndex(); var lastNode = _nodes[lastNodeIndex]; - _nodes.RemoveAt(lastNodeIndex); + _size--; + _version++; // In case we wanted to remove the node that was the last one, // we are done. @@ -358,7 +416,7 @@ private void PutAt((TElement element, TPriority priority) node, int index) /// Gets the index of the last node in the heap. /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - private int GetLastNodeIndex() => _nodes.Count - 1; + private int GetLastNodeIndex() => _size - 1; /// /// Gets the index of an element's parent. @@ -428,12 +486,12 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex // for this value to drop in. Similar optimization as in the insertion sort. int i; - while ((i = this.GetFirstChildIndex(nodeIndex)) < _nodes.Count) + while ((i = this.GetFirstChildIndex(nodeIndex)) < _size) { // Check if the current node (pointed by 'nodeIndex') should really be extracted // first, or maybe one of its children should be extracted earlier. var topChild = _nodes[i]; - var childrenIndexesLimit = Math.Min(i + Arity, _nodes.Count); + var childrenIndexesLimit = Math.Min(i + Arity, _size); int topChildIndex = i; while (++i < childrenIndexesLimit) @@ -469,14 +527,14 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex public partial class UnorderedItemsCollection : IReadOnlyCollection<(TElement element, TPriority priority)>, ICollection { - private readonly IReadOnlyCollection<(TElement element, TPriority priority)> items; + private readonly PriorityQueue _queue; - public UnorderedItemsCollection(IReadOnlyCollection<(TElement element, TPriority priority)> items) + public UnorderedItemsCollection(PriorityQueue queue) { - this.items = items; + _queue = queue; } - public int Count => this.items.Count; + public int Count => _queue._size; object ICollection.SyncRoot => this; bool ICollection.IsSynchronized => false; @@ -484,31 +542,119 @@ public UnorderedItemsCollection(IReadOnlyCollection<(TElement element, TPriority public struct Enumerator : IEnumerator<(TElement element, TPriority priority)>, IEnumerator { - private readonly IEnumerator<(TElement element, TPriority priority)> enumerator; + private readonly PriorityQueue _queue; + private readonly int _version; + + private int _index; + private (TElement element, TPriority priority)? _currentElement; + + private const int FirstCallToEnumerator = -2; + private const int EndOfEnumeration = -1; + + internal Enumerator(PriorityQueue queue) + { + _queue = queue; + _version = queue._version; + _index = FirstCallToEnumerator; + _currentElement = default; + } - internal Enumerator(System.Collections.Generic.IEnumerator<(TElement element, TPriority priority)> enumerator) + public void Dispose() { - this.enumerator = enumerator; + _index = EndOfEnumeration; } - (TElement element, TPriority priority) IEnumerator<(TElement element, TPriority priority)>.Current - => this.enumerator.Current; + public bool MoveNext() + { + bool advancedEnumerator; - object IEnumerator.Current => this.enumerator.Current; + if (_version != _queue._version) + { + throw new InvalidOperationException(SR.InvalidOperation_EnumFailedVersion); + } + + if (_index == FirstCallToEnumerator) + { + _index = 0; + advancedEnumerator = (_queue._size > 0); + + if (advancedEnumerator) + { + _currentElement = _queue._nodes[_index]; + } + else + { + _index = EndOfEnumeration; + } + + return advancedEnumerator; + } + + if (_index == EndOfEnumeration) + { + return false; + } + + _index++; + advancedEnumerator = (_index < _queue._size); + + if (advancedEnumerator) + { + _currentElement = _queue._nodes[_index]; + } + else + { + _currentElement = default; + } + + return advancedEnumerator; + } - void IDisposable.Dispose() => this.enumerator.Dispose(); - bool IEnumerator.MoveNext() => this.enumerator.MoveNext(); - void IEnumerator.Reset() => this.enumerator.Reset(); + public (TElement element, TPriority priority) Current + { + get + { + if (_index < 0) + { + ThrowEnumerationNotStartedOrEnded(); + } + return _currentElement!.Value; + } + } + + private void ThrowEnumerationNotStartedOrEnded() + { + Debug.Assert(_index == FirstCallToEnumerator || _index == EndOfEnumeration); + + string message = _index == FirstCallToEnumerator + ? SR.InvalidOperation_EnumNotStarted + : SR.InvalidOperation_EnumEnded; + + throw new InvalidOperationException(message); + } + + object IEnumerator.Current => this.Current; + + void IEnumerator.Reset() + { + if (_version != _queue._version) + { + throw new InvalidOperationException(SR.InvalidOperation_EnumFailedVersion); + } + + _index = FirstCallToEnumerator; + _currentElement = default; + } } public Enumerator GetEnumerator() - => new Enumerator(this.items.GetEnumerator()); + => new Enumerator(this._queue); IEnumerator<(TElement element, TPriority priority)> IEnumerable<(TElement element, TPriority priority)>.GetEnumerator() - => new Enumerator(this.items.GetEnumerator()); + => new Enumerator(this._queue); IEnumerator IEnumerable.GetEnumerator() - => new Enumerator(this.items.GetEnumerator()); + => new Enumerator(this._queue); } } } From b043b5abb3215d19fe32002a3be3e3c817effa55 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Fri, 8 Jan 2021 22:27:00 +0100 Subject: [PATCH 08/50] (draft step, to squash) Create UnorderedItemsCollection lazily --- .../Collections/Generic/PriorityQueue.cs | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 6e404a00a0885e..736c410c2fc1c5 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -20,6 +20,8 @@ public class PriorityQueue /// private (TElement element, TPriority priority)[] _nodes; + private Lazy _unorderedItems; + /// /// The number of nodes in the heap. /// @@ -46,7 +48,7 @@ public class PriorityQueue public PriorityQueue() { _nodes = Array.Empty<(TElement, TPriority)>(); - this.UnorderedItems = new UnorderedItemsCollection(this); + _unorderedItems = new Lazy(() => new UnorderedItemsCollection(this)); this.Comparer = Comparer.Default; } @@ -61,7 +63,7 @@ public PriorityQueue(int initialCapacity) } _nodes = new (TElement, TPriority)[initialCapacity]; - this.UnorderedItems = new UnorderedItemsCollection(this); + _unorderedItems = new Lazy(() => new UnorderedItemsCollection(this)); this.Comparer = Comparer.Default; } @@ -71,7 +73,7 @@ public PriorityQueue(int initialCapacity) public PriorityQueue(IComparer? comparer) { _nodes = Array.Empty<(TElement, TPriority)>(); - this.UnorderedItems = new UnorderedItemsCollection(this); + _unorderedItems = new Lazy(() => new UnorderedItemsCollection(this)); this.Comparer = comparer ?? Comparer.Default; } @@ -87,7 +89,7 @@ public PriorityQueue(int initialCapacity, IComparer? comparer) } _nodes = new (TElement, TPriority)[initialCapacity]; - this.UnorderedItems = new UnorderedItemsCollection(this); + _unorderedItems = new Lazy(() => new UnorderedItemsCollection(this)); this.Comparer = comparer ?? Comparer.Default; } @@ -102,7 +104,7 @@ public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items) } _nodes = EnumerableHelpers.ToArray(items, out _size); - this.UnorderedItems = new UnorderedItemsCollection(this); + _unorderedItems = new Lazy(() => new UnorderedItemsCollection(this)); this.Comparer = Comparer.Default; if (_size > 1) @@ -123,7 +125,7 @@ public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items, } _nodes = EnumerableHelpers.ToArray(items, out _size); - this.UnorderedItems = new UnorderedItemsCollection(this); + _unorderedItems = new Lazy(() => new UnorderedItemsCollection(this)); this.Comparer = comparer ?? Comparer.Default; if (_size > 1) @@ -142,6 +144,11 @@ public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items, /// public IComparer Comparer { get; } + /// + /// Gets a collection that enumerates the elements of the queue. + /// + public UnorderedItemsCollection UnorderedItems => _unorderedItems.Value; + /// /// Enqueues the specified element and associates it with the specified priority. /// @@ -520,11 +527,6 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex this.PutAt(node, nodeIndex); } - /// - /// Gets a collection that enumerates the elements of the queue. - /// - public UnorderedItemsCollection UnorderedItems { get; } - public partial class UnorderedItemsCollection : IReadOnlyCollection<(TElement element, TPriority priority)>, ICollection { private readonly PriorityQueue _queue; From f60e250b551bc96b4f41d2908474b76be015b984 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Fri, 8 Jan 2021 22:30:33 +0100 Subject: [PATCH 09/50] (draft step, to squash) Deduplicate constructors --- .../Collections/Generic/PriorityQueue.cs | 31 +++---------------- 1 file changed, 4 insertions(+), 27 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 736c410c2fc1c5..c6dff5e3f04b16 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -46,35 +46,24 @@ public class PriorityQueue /// Creates an empty priority queue. /// public PriorityQueue() + : this(initialCapacity: 0, comparer: null) { - _nodes = Array.Empty<(TElement, TPriority)>(); - _unorderedItems = new Lazy(() => new UnorderedItemsCollection(this)); - this.Comparer = Comparer.Default; } /// /// Creates an empty priority queue with the specified initial capacity for its underlying array. /// public PriorityQueue(int initialCapacity) + : this(initialCapacity, comparer: null) { - if (initialCapacity < 0) - { - throw new ArgumentOutOfRangeException(nameof(initialCapacity)); - } - - _nodes = new (TElement, TPriority)[initialCapacity]; - _unorderedItems = new Lazy(() => new UnorderedItemsCollection(this)); - this.Comparer = Comparer.Default; } /// /// Creates an empty priority queue with the specified priority comparer. /// public PriorityQueue(IComparer? comparer) + : this(initialCapacity: 0, comparer) { - _nodes = Array.Empty<(TElement, TPriority)>(); - _unorderedItems = new Lazy(() => new UnorderedItemsCollection(this)); - this.Comparer = comparer ?? Comparer.Default; } /// @@ -97,20 +86,8 @@ public PriorityQueue(int initialCapacity, IComparer? comparer) /// Creates a priority queue populated with the specified elements and priorities. /// public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items) + : this(items, comparer: null) { - if (items is null) - { - throw new ArgumentNullException(nameof(items)); - } - - _nodes = EnumerableHelpers.ToArray(items, out _size); - _unorderedItems = new Lazy(() => new UnorderedItemsCollection(this)); - this.Comparer = Comparer.Default; - - if (_size > 1) - { - this.Heapify(); - } } /// From 6e0180176c426c83988d836e1f0da1cd4e4241e9 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Fri, 8 Jan 2021 22:35:58 +0100 Subject: [PATCH 10/50] (draft step, to squash) Replace excessive `var` with explicit types --- .../Collections/Generic/PriorityQueue.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index c6dff5e3f04b16..fc3fdb563991b1 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -150,7 +150,7 @@ public void Enqueue(TElement element, TPriority priority) _version++; // Restore the heap order - var lastNodeIndex = this.GetLastNodeIndex(); + int lastNodeIndex = this.GetLastNodeIndex(); this.MoveUp(node, lastNodeIndex); } @@ -160,7 +160,7 @@ public void Enqueue(TElement element, TPriority priority) /// The queue is empty. public TElement Peek() { - if (this.TryPeek(out var element, out var priority)) + if (this.TryPeek(out TElement? element, out TPriority? priority)) { return element; } @@ -177,7 +177,7 @@ public TElement Peek() /// The queue is empty. public TElement Dequeue() { - if (this.TryDequeue(out var element, out var priority)) + if (this.TryDequeue(out TElement? element, out TPriority? priority)) { return element; } @@ -354,7 +354,7 @@ private void Remove(int indexOfNodeToRemove) // The idea is to replace the specified node by the very last // node and shorten the array by one. - var lastNodeIndex = this.GetLastNodeIndex(); + int lastNodeIndex = this.GetLastNodeIndex(); var lastNode = _nodes[lastNodeIndex]; _size--; _version++; @@ -374,7 +374,7 @@ private void Remove(int indexOfNodeToRemove) var nodeToRemove = _nodes[indexOfNodeToRemove]; - var relation = this.Comparer.Compare(lastNode.priority, nodeToRemove.priority); + int relation = this.Comparer.Compare(lastNode.priority, nodeToRemove.priority); this.PutAt(lastNode, indexOfNodeToRemove); if (relation < 0) @@ -424,10 +424,10 @@ private void Heapify() // only for higher nodes, starting from the first node that has children. // It is the parent of the very last element in the array. - var lastNodeIndex = this.GetLastNodeIndex(); - var lastParentWithChildren = this.GetParentIndex(lastNodeIndex); + int lastNodeIndex = this.GetLastNodeIndex(); + int lastParentWithChildren = this.GetParentIndex(lastNodeIndex); - for (var index = lastParentWithChildren; index >= 0; --index) + for (int index = lastParentWithChildren; index >= 0; --index) { this.MoveDown(_nodes[index], index); } @@ -443,7 +443,7 @@ private void MoveUp((TElement element, TPriority priority) node, int nodeIndex) while (nodeIndex > 0) { - var parentIndex = this.GetParentIndex(nodeIndex); + int parentIndex = this.GetParentIndex(nodeIndex); var parent = _nodes[parentIndex]; if (this.Comparer.Compare(node.priority, parent.priority) < 0) @@ -475,7 +475,7 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex // Check if the current node (pointed by 'nodeIndex') should really be extracted // first, or maybe one of its children should be extracted earlier. var topChild = _nodes[i]; - var childrenIndexesLimit = Math.Min(i + Arity, _size); + int childrenIndexesLimit = Math.Min(i + Arity, _size); int topChildIndex = i; while (++i < childrenIndexesLimit) From e119ecc87884044ddb3f15b0c6f2beaf5ebb6dfc Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Fri, 8 Jan 2021 23:13:34 +0100 Subject: [PATCH 11/50] (draft step, to squash) Remove `this.` in front of methods --- .../Collections/Generic/PriorityQueue.cs | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index fc3fdb563991b1..528d97c29eb3e2 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -107,7 +107,7 @@ public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items, if (_size > 1) { - this.Heapify(); + Heapify(); } } @@ -141,7 +141,7 @@ public void Enqueue(TElement element, TPriority priority) throw new ArgumentNullException(nameof(priority)); } - this.EnsureEnoughCapacityBeforeAddingNode(); + EnsureEnoughCapacityBeforeAddingNode(); // Add the node at the end var node = (element, priority); @@ -150,8 +150,8 @@ public void Enqueue(TElement element, TPriority priority) _version++; // Restore the heap order - int lastNodeIndex = this.GetLastNodeIndex(); - this.MoveUp(node, lastNodeIndex); + int lastNodeIndex = GetLastNodeIndex(); + MoveUp(node, lastNodeIndex); } /// @@ -160,7 +160,7 @@ public void Enqueue(TElement element, TPriority priority) /// The queue is empty. public TElement Peek() { - if (this.TryPeek(out TElement? element, out TPriority? priority)) + if (TryPeek(out TElement? element, out TPriority? priority)) { return element; } @@ -177,7 +177,7 @@ public TElement Peek() /// The queue is empty. public TElement Dequeue() { - if (this.TryDequeue(out TElement? element, out TPriority? priority)) + if (TryDequeue(out TElement? element, out TPriority? priority)) { return element; } @@ -205,7 +205,7 @@ public bool TryDequeue([MaybeNullWhen(false)] out TElement element, [MaybeNullWh else { (element, priority) = _nodes[RootIndex]; - this.Remove(RootIndex); + Remove(RootIndex); return true; } } @@ -251,7 +251,7 @@ public void EnqueueRange(IEnumerable<(TElement Element, TPriority Priority)> ite foreach (var (element, priority) in items) { - this.Enqueue(element, priority); + Enqueue(element, priority); } } @@ -267,7 +267,7 @@ public void EnqueueRange(IEnumerable elements, TPriority priority) foreach (var element in elements) { - this.Enqueue(element, priority); + Enqueue(element, priority); } } @@ -301,7 +301,7 @@ public void EnsureCapacity(int capacity) return; } - this.SetCapacity(capacity); + SetCapacity(capacity); } /// @@ -327,7 +327,7 @@ private void EnsureEnoughCapacityBeforeAddingNode() { newCapacity = _nodes.Length + MinimumGrow; } - this.SetCapacity(newCapacity); + SetCapacity(newCapacity); } } @@ -354,7 +354,7 @@ private void Remove(int indexOfNodeToRemove) // The idea is to replace the specified node by the very last // node and shorten the array by one. - int lastNodeIndex = this.GetLastNodeIndex(); + int lastNodeIndex = GetLastNodeIndex(); var lastNode = _nodes[lastNodeIndex]; _size--; _version++; @@ -375,15 +375,15 @@ private void Remove(int indexOfNodeToRemove) var nodeToRemove = _nodes[indexOfNodeToRemove]; int relation = this.Comparer.Compare(lastNode.priority, nodeToRemove.priority); - this.PutAt(lastNode, indexOfNodeToRemove); + PutAt(lastNode, indexOfNodeToRemove); if (relation < 0) { - this.MoveUp(lastNode, indexOfNodeToRemove); + MoveUp(lastNode, indexOfNodeToRemove); } else { - this.MoveDown(lastNode, indexOfNodeToRemove); + MoveDown(lastNode, indexOfNodeToRemove); } } @@ -424,12 +424,12 @@ private void Heapify() // only for higher nodes, starting from the first node that has children. // It is the parent of the very last element in the array. - int lastNodeIndex = this.GetLastNodeIndex(); - int lastParentWithChildren = this.GetParentIndex(lastNodeIndex); + int lastNodeIndex = GetLastNodeIndex(); + int lastParentWithChildren = GetParentIndex(lastNodeIndex); for (int index = lastParentWithChildren; index >= 0; --index) { - this.MoveDown(_nodes[index], index); + MoveDown(_nodes[index], index); } } @@ -443,12 +443,12 @@ private void MoveUp((TElement element, TPriority priority) node, int nodeIndex) while (nodeIndex > 0) { - int parentIndex = this.GetParentIndex(nodeIndex); + int parentIndex = GetParentIndex(nodeIndex); var parent = _nodes[parentIndex]; if (this.Comparer.Compare(node.priority, parent.priority) < 0) { - this.PutAt(parent, nodeIndex); + PutAt(parent, nodeIndex); nodeIndex = parentIndex; } else @@ -457,7 +457,7 @@ private void MoveUp((TElement element, TPriority priority) node, int nodeIndex) } } - this.PutAt(node, nodeIndex); + PutAt(node, nodeIndex); } /// @@ -470,7 +470,7 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex // for this value to drop in. Similar optimization as in the insertion sort. int i; - while ((i = this.GetFirstChildIndex(nodeIndex)) < _size) + while ((i = GetFirstChildIndex(nodeIndex)) < _size) { // Check if the current node (pointed by 'nodeIndex') should really be extracted // first, or maybe one of its children should be extracted earlier. @@ -497,11 +497,11 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex // Move the top child up by one node and now investigate the // node that was considered to be the top child (recursive). - this.PutAt(topChild, nodeIndex); + PutAt(topChild, nodeIndex); nodeIndex = topChildIndex; } - this.PutAt(node, nodeIndex); + PutAt(node, nodeIndex); } public partial class UnorderedItemsCollection : IReadOnlyCollection<(TElement element, TPriority priority)>, ICollection @@ -627,13 +627,13 @@ void IEnumerator.Reset() } public Enumerator GetEnumerator() - => new Enumerator(this._queue); + => new Enumerator(_queue); IEnumerator<(TElement element, TPriority priority)> IEnumerable<(TElement element, TPriority priority)>.GetEnumerator() - => new Enumerator(this._queue); + => new Enumerator(_queue); IEnumerator IEnumerable.GetEnumerator() - => new Enumerator(this._queue); + => new Enumerator(_queue); } } } From d76b0cf4751ac0f63020c6287c232e40fc431137 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Fri, 8 Jan 2021 23:21:44 +0100 Subject: [PATCH 12/50] (draft step, to squash) Improve out-of-range-argument exceptions --- .../src/System/Collections/Generic/PriorityQueue.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 528d97c29eb3e2..1cfbfadd51d3b6 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -74,7 +74,8 @@ public PriorityQueue(int initialCapacity, IComparer? comparer) { if (initialCapacity < 0) { - throw new ArgumentOutOfRangeException(nameof(initialCapacity)); + throw new ArgumentOutOfRangeException( + nameof(initialCapacity), initialCapacity, SR.ArgumentOutOfRange_NeedNonNegNum); } _nodes = new (TElement, TPriority)[initialCapacity]; @@ -293,7 +294,8 @@ public void EnsureCapacity(int capacity) { if (capacity < 0) { - throw new ArgumentOutOfRangeException(nameof(capacity)); + throw new ArgumentOutOfRangeException( + nameof(capacity), capacity, SR.ArgumentOutOfRange_NeedNonNegNum); } if (capacity <= _size) From 4e7e13852db4e138f00eba039c4360df09ed7617 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 9 Jan 2021 00:24:24 +0100 Subject: [PATCH 13/50] (draft step, to squash) Use error messages from .resx --- .../src/System/Collections/Generic/PriorityQueue.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 1cfbfadd51d3b6..a3b94241ea502f 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -167,8 +167,7 @@ public TElement Peek() } else { - throw new InvalidOperationException( - "The priority queue is empty, cannot get the element with minimal priority."); + throw new InvalidOperationException(SR.InvalidOperation_EmptyQueue); } } @@ -184,8 +183,7 @@ public TElement Dequeue() } else { - throw new InvalidOperationException( - "The priority queue is empty, cannot dequeue an element."); + throw new InvalidOperationException(SR.InvalidOperation_EmptyQueue); } } From 4c2fd529f63e3f60aeeaf3421a435f635a8eb2fb Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 9 Jan 2021 00:26:44 +0100 Subject: [PATCH 14/50] (draft step, to squash) Use positive case first in try methods --- .../Collections/Generic/PriorityQueue.cs | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index a3b94241ea502f..9e5710ceb34317 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -195,18 +195,16 @@ public TElement Dequeue() /// public bool TryDequeue([MaybeNullWhen(false)] out TElement element, [MaybeNullWhen(false)] out TPriority priority) { - if (_size == 0) - { - element = default; - priority = default; - return false; - } - else + if (_size != 0) { (element, priority) = _nodes[RootIndex]; Remove(RootIndex); return true; } + + element = default; + priority = default; + return false; } /// @@ -217,17 +215,15 @@ public bool TryDequeue([MaybeNullWhen(false)] out TElement element, [MaybeNullWh /// public bool TryPeek([MaybeNullWhen(false)] out TElement element, [MaybeNullWhen(false)] out TPriority priority) { - if (_size == 0) - { - element = default; - priority = default; - return false; - } - else + if (_size != 0) { (element, priority) = _nodes[RootIndex]; return true; } + + element = default; + priority = default; + return false; } /// From 321b4b0683243e26b38f35473b76c800206a98ea Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 9 Jan 2021 00:31:32 +0100 Subject: [PATCH 15/50] (draft step, to squash) Implement UnorderedItemsCollection.CopyTo --- .../Collections/Generic/PriorityQueue.cs | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 9e5710ceb34317..8fb2b97dda114e 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -513,7 +513,42 @@ public UnorderedItemsCollection(PriorityQueue queue) object ICollection.SyncRoot => this; bool ICollection.IsSynchronized => false; - public void CopyTo(Array array, int index) => throw new NotImplementedException(); + public void CopyTo(Array array, int arrayIndex) + { + if (array == null) + { + throw new ArgumentNullException(nameof(array)); + } + + if (array.Rank != 1) + { + throw new ArgumentException(SR.Arg_RankMultiDimNotSupported, nameof(array)); + } + + if (array.GetLowerBound(0) != 0) + { + throw new ArgumentException(SR.Arg_NonZeroLowerBound, nameof(array)); + } + + if (arrayIndex < 0 || arrayIndex > array.Length) + { + throw new ArgumentOutOfRangeException(nameof(arrayIndex), arrayIndex, SR.ArgumentOutOfRange_Index); + } + + if (array.Length - arrayIndex < _queue._size) + { + throw new ArgumentException(SR.Argument_InvalidOffLen); + } + + try + { + Array.Copy(_queue._nodes, 0, array, arrayIndex, _queue._size); + } + catch (ArrayTypeMismatchException) + { + throw new ArgumentException(SR.Argument_InvalidArrayType, nameof(array)); + } + } public struct Enumerator : IEnumerator<(TElement element, TPriority priority)>, IEnumerator { From 72c24dab53405855e743658b531b19103f63a3fb Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 9 Jan 2021 00:36:00 +0100 Subject: [PATCH 16/50] (draft step, to squash) Optimize expressions involving Arity --- .../src/System/Collections/Generic/PriorityQueue.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 8fb2b97dda114e..e937bbbd201172 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -40,7 +40,7 @@ public class PriorityQueue /// /// Specifies the arity of the d-ary heap, which here is quaternary. /// - private const int Arity = 4; + private const uint Arity = 4; /// /// Creates an empty priority queue. @@ -402,13 +402,13 @@ private void PutAt((TElement element, TPriority priority) node, int index) /// Gets the index of an element's parent. /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - private int GetParentIndex(int index) => (index - 1) / Arity; + private int GetParentIndex(int index) => (int)((index - 1) / Arity); /// /// Gets the index of the first child of an element. /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - private int GetFirstChildIndex(int index) => Arity * index + 1; + private int GetFirstChildIndex(int index) => (int)(Arity * index + 1); /// /// Converts an unordered list into a heap. @@ -471,7 +471,7 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex // Check if the current node (pointed by 'nodeIndex') should really be extracted // first, or maybe one of its children should be extracted earlier. var topChild = _nodes[i]; - int childrenIndexesLimit = Math.Min(i + Arity, _size); + int childrenIndexesLimit = (int)Math.Min(i + Arity, _size); int topChildIndex = i; while (++i < childrenIndexesLimit) From e647b2ba1c2c9ca97f8ced8e68885054b11d51e0 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 9 Jan 2021 00:38:48 +0100 Subject: [PATCH 17/50] (draft step, to squash) Adjust implementation to be consistent with reference --- .../src/System/Collections/Generic/PriorityQueue.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index e937bbbd201172..b29b221244d329 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -513,7 +513,7 @@ public UnorderedItemsCollection(PriorityQueue queue) object ICollection.SyncRoot => this; bool ICollection.IsSynchronized => false; - public void CopyTo(Array array, int arrayIndex) + public void CopyTo(Array array, int index) { if (array == null) { @@ -530,19 +530,19 @@ public void CopyTo(Array array, int arrayIndex) throw new ArgumentException(SR.Arg_NonZeroLowerBound, nameof(array)); } - if (arrayIndex < 0 || arrayIndex > array.Length) + if (index < 0 || index > array.Length) { - throw new ArgumentOutOfRangeException(nameof(arrayIndex), arrayIndex, SR.ArgumentOutOfRange_Index); + throw new ArgumentOutOfRangeException(nameof(index), index, SR.ArgumentOutOfRange_Index); } - if (array.Length - arrayIndex < _queue._size) + if (array.Length - index < _queue._size) { throw new ArgumentException(SR.Argument_InvalidOffLen); } try { - Array.Copy(_queue._nodes, 0, array, arrayIndex, _queue._size); + Array.Copy(_queue._nodes, 0, array, index, _queue._size); } catch (ArrayTypeMismatchException) { From 4d87259253778899dda3b499220456f8c6227f4b Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 9 Jan 2021 01:17:55 +0100 Subject: [PATCH 18/50] (draft step, to squash) Implement method `EnqueueDequeue` --- .../Collections/Generic/PriorityQueue.cs | 27 ++++++++++- .../PriorityQueue/PriorityQueue.Tests.cs | 48 +++++++++++++++++++ .../tests/System.Collections.Tests.csproj | 1 + 3 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index b29b221244d329..7f03dd5f92e0e2 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -231,7 +231,32 @@ public bool TryPeek([MaybeNullWhen(false)] out TElement element, [MaybeNullWhen( /// public TElement EnqueueDequeue(TElement element, TPriority priority) { - throw new NotImplementedException(); + if (element is null) + { + throw new ArgumentNullException(nameof(element)); + } + + if (priority is null) + { + throw new ArgumentNullException(nameof(priority)); + } + + var root = _nodes[RootIndex]; + + if (this.Comparer.Compare(priority, root.priority) < 0) + { + return element; + } + else + { + var newRoot = (element, priority); + _nodes[RootIndex] = newRoot; + + MoveDown(newRoot, RootIndex); + _version++; + + return root.element; + } } /// diff --git a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs new file mode 100644 index 00000000000000..c604408be08631 --- /dev/null +++ b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs @@ -0,0 +1,48 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using Xunit; + +namespace System.Collections.Tests +{ + public class PriorityQueue_NonGeneric_Tests : TestBase + { + protected PriorityQueue SmallPriorityQueueFactory(out HashSet<(string, int)> items) + { + items = new HashSet<(string, int)> + { + ("one", 1), + ("two", 2), + ("three", 3) + }; + var queue = new PriorityQueue(items); + + return queue; + } + + [Fact] + public void PriorityQueue_Generic_EnqueueDequeue_SmallerThanMin() + { + PriorityQueue queue = SmallPriorityQueueFactory(out HashSet<(string, int)> enqueuedItems); + + string actualElement = queue.EnqueueDequeue("zero", 0); + + Assert.Equal("zero", actualElement); + Assert.True(enqueuedItems.SetEquals(queue.UnorderedItems)); + } + + [Fact] + public void PriorityQueue_Generic_EnqueueDequeue_LargerThanMin() + { + PriorityQueue queue = SmallPriorityQueueFactory(out HashSet<(string, int)> enqueuedItems); + + string actualElement = queue.EnqueueDequeue("four", 4); + + Assert.Equal("one", actualElement); + Assert.Equal("two", queue.Dequeue()); + Assert.Equal("three", queue.Dequeue()); + Assert.Equal("four", queue.Dequeue()); + } + } +} diff --git a/src/libraries/System.Collections/tests/System.Collections.Tests.csproj b/src/libraries/System.Collections/tests/System.Collections.Tests.csproj index 46688df9c7cc85..eb646000406169 100644 --- a/src/libraries/System.Collections/tests/System.Collections.Tests.csproj +++ b/src/libraries/System.Collections/tests/System.Collections.Tests.csproj @@ -97,6 +97,7 @@ + From cd5c0b21111116d5a1e609b8998684250b848fbb Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sun, 17 Jan 2021 19:55:57 +0100 Subject: [PATCH 19/50] (draft step, to squash) Make EnsureCapacity return int --- .../System.Collections/ref/System.Collections.cs | 2 +- .../src/System/Collections/Generic/PriorityQueue.cs | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Collections/ref/System.Collections.cs b/src/libraries/System.Collections/ref/System.Collections.cs index 3861fef7a7b056..c10c961a1dbb91 100644 --- a/src/libraries/System.Collections/ref/System.Collections.cs +++ b/src/libraries/System.Collections/ref/System.Collections.cs @@ -396,7 +396,7 @@ public void Enqueue(TElement element, TPriority priority) { } public TElement EnqueueDequeue(TElement element, TPriority priority) { throw null; } public void EnqueueRange(System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)> items) { } public void EnqueueRange(System.Collections.Generic.IEnumerable elements, TPriority priority) { } - public void EnsureCapacity(int capacity) { } + public int EnsureCapacity(int capacity) { throw null; } public TElement Peek() { throw null; } public void TrimExcess() { } public bool TryDequeue([System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TElement element, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TPriority priority) { throw null; } diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 7f03dd5f92e0e2..d4513ccad417a5 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -309,7 +309,7 @@ public void Clear() /// Ensures that the priority queue has the specified capacity /// and resizes its underlying array if necessary. /// - public void EnsureCapacity(int capacity) + public int EnsureCapacity(int capacity) { if (capacity < 0) { @@ -317,12 +317,14 @@ public void EnsureCapacity(int capacity) nameof(capacity), capacity, SR.ArgumentOutOfRange_NeedNonNegNum); } - if (capacity <= _size) + int currentCapacity = _nodes.Length; + if (currentCapacity >= capacity) { - return; + return currentCapacity; } SetCapacity(capacity); + return capacity; } /// From ea2b155d5253951186384c0053b4e6b9ebf8170e Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sun, 17 Jan 2021 20:04:36 +0100 Subject: [PATCH 20/50] (draft step, to squash) Simplify lazy initialization of _unorderedItems --- .../src/System/Collections/Generic/PriorityQueue.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index d4513ccad417a5..e685623ed4e385 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -20,7 +20,7 @@ public class PriorityQueue /// private (TElement element, TPriority priority)[] _nodes; - private Lazy _unorderedItems; + private UnorderedItemsCollection? _unorderedItems; /// /// The number of nodes in the heap. @@ -79,7 +79,6 @@ public PriorityQueue(int initialCapacity, IComparer? comparer) } _nodes = new (TElement, TPriority)[initialCapacity]; - _unorderedItems = new Lazy(() => new UnorderedItemsCollection(this)); this.Comparer = comparer ?? Comparer.Default; } @@ -103,7 +102,6 @@ public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items, } _nodes = EnumerableHelpers.ToArray(items, out _size); - _unorderedItems = new Lazy(() => new UnorderedItemsCollection(this)); this.Comparer = comparer ?? Comparer.Default; if (_size > 1) @@ -125,7 +123,7 @@ public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items, /// /// Gets a collection that enumerates the elements of the queue. /// - public UnorderedItemsCollection UnorderedItems => _unorderedItems.Value; + public UnorderedItemsCollection UnorderedItems => _unorderedItems ??= new UnorderedItemsCollection(this); /// /// Enqueues the specified element and associates it with the specified priority. From a4c0d4f2291b5d61a1b1a426d4673d6b61afad70 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sun, 17 Jan 2021 20:13:14 +0100 Subject: [PATCH 21/50] (draft step, to squash) Use `out _` discard for unused properties --- .../src/System/Collections/Generic/PriorityQueue.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index e685623ed4e385..1bc21728159937 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -159,7 +159,7 @@ public void Enqueue(TElement element, TPriority priority) /// The queue is empty. public TElement Peek() { - if (TryPeek(out TElement? element, out TPriority? priority)) + if (TryPeek(out TElement? element, out _)) { return element; } @@ -175,7 +175,7 @@ public TElement Peek() /// The queue is empty. public TElement Dequeue() { - if (TryDequeue(out TElement? element, out TPriority? priority)) + if (TryDequeue(out TElement? element, out _)) { return element; } From 2618b29e48e4845eca2be37d3fe63ea0c19391b4 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sun, 17 Jan 2021 20:16:29 +0100 Subject: [PATCH 22/50] (draft step, to squash) Relax null checks on elements and priorities --- .../Collections/Generic/PriorityQueue.cs | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 1bc21728159937..d7732426a05b2b 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -130,16 +130,6 @@ public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items, /// public void Enqueue(TElement element, TPriority priority) { - if (element is null) - { - throw new ArgumentNullException(nameof(element)); - } - - if (priority is null) - { - throw new ArgumentNullException(nameof(priority)); - } - EnsureEnoughCapacityBeforeAddingNode(); // Add the node at the end @@ -229,16 +219,6 @@ public bool TryPeek([MaybeNullWhen(false)] out TElement element, [MaybeNullWhen( /// public TElement EnqueueDequeue(TElement element, TPriority priority) { - if (element is null) - { - throw new ArgumentNullException(nameof(element)); - } - - if (priority is null) - { - throw new ArgumentNullException(nameof(priority)); - } - var root = _nodes[RootIndex]; if (this.Comparer.Compare(priority, root.priority) < 0) From 25f01258917410ead1f1e14d0bf91dc1a8e68683 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sun, 17 Jan 2021 20:47:56 +0100 Subject: [PATCH 23/50] (draft step, to squash) Simplify method SetCapacity --- .../src/System/Collections/Generic/PriorityQueue.cs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index d7732426a05b2b..b2bb561a342231 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -337,14 +337,7 @@ private void EnsureEnoughCapacityBeforeAddingNode() /// private void SetCapacity(int capacity) { - var newArray = new (TElement, TPriority)[capacity]; - - if (_size > 0) - { - Array.Copy(_nodes, 0, newArray, 0, _size); - } - - _nodes = newArray; + Array.Resize(ref _nodes, capacity); } /// From 7d187c98cae439c934f68f09910735cc13f959f3 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sun, 17 Jan 2021 20:49:31 +0100 Subject: [PATCH 24/50] (draft step, to squash) Remove MethodImplOptions.AggressiveInlining attributed --- .../src/System/Collections/Generic/PriorityQueue.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index b2bb561a342231..c3a80265cba147 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -384,7 +384,6 @@ private void Remove(int indexOfNodeToRemove) /// /// Puts a node at the specified index. /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] private void PutAt((TElement element, TPriority priority) node, int index) { _nodes[index] = node; @@ -393,19 +392,16 @@ private void PutAt((TElement element, TPriority priority) node, int index) /// /// Gets the index of the last node in the heap. /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] private int GetLastNodeIndex() => _size - 1; /// /// Gets the index of an element's parent. /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] private int GetParentIndex(int index) => (int)((index - 1) / Arity); /// /// Gets the index of the first child of an element. /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] private int GetFirstChildIndex(int index) => (int)(Arity * index + 1); /// From c8c2da400920f1f55709ef751ecf0ae88f745584 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sun, 17 Jan 2021 20:54:25 +0100 Subject: [PATCH 25/50] (draft step, to squash) Use Array.Empty if the initial capacity is zero --- .../src/System/Collections/Generic/PriorityQueue.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index c3a80265cba147..cf3f49b4bacc5d 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -78,7 +78,10 @@ public PriorityQueue(int initialCapacity, IComparer? comparer) nameof(initialCapacity), initialCapacity, SR.ArgumentOutOfRange_NeedNonNegNum); } - _nodes = new (TElement, TPriority)[initialCapacity]; + _nodes = (initialCapacity == 0) + ? Array.Empty<(TElement, TPriority)>() + : new (TElement, TPriority)[initialCapacity]; + this.Comparer = comparer ?? Comparer.Default; } From 0ba9699dbc9f0b904756bc07f7b58b34f66668cc Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sun, 17 Jan 2021 21:00:25 +0100 Subject: [PATCH 26/50] (draft step, to squash) Simplify UnorderedItemsCollection.Enumerator declaration --- .../src/System/Collections/Generic/PriorityQueue.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index cf3f49b4bacc5d..dcfbb9a89ddc1f 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -547,7 +547,7 @@ public void CopyTo(Array array, int index) } } - public struct Enumerator : IEnumerator<(TElement element, TPriority priority)>, IEnumerator + public struct Enumerator : IEnumerator<(TElement element, TPriority priority)> { private readonly PriorityQueue _queue; private readonly int _version; From 418593735d0b169c011b2568fa41ee2e2b1f8b34 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sun, 17 Jan 2021 21:06:19 +0100 Subject: [PATCH 27/50] (draft step, to squash) Simplify GetEnumerator methods --- .../src/System/Collections/Generic/PriorityQueue.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index dcfbb9a89ddc1f..7cc71cea0488ad 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -658,10 +658,10 @@ public Enumerator GetEnumerator() => new Enumerator(_queue); IEnumerator<(TElement element, TPriority priority)> IEnumerable<(TElement element, TPriority priority)>.GetEnumerator() - => new Enumerator(_queue); + => GetEnumerator(); IEnumerator IEnumerable.GetEnumerator() - => new Enumerator(_queue); + => GetEnumerator(); } } } From af21790470b686480285ed26a819fd8b4b16c338 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 23 Jan 2021 12:27:19 +0100 Subject: [PATCH 28/50] (draft step, to squash) Optimize EnqueueRange methods --- .../Collections/Generic/PriorityQueue.cs | 50 ++++++++++++++++--- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 7cc71cea0488ad..254b811eff4db9 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -250,9 +250,21 @@ public void EnqueueRange(IEnumerable<(TElement Element, TPriority Priority)> ite throw new ArgumentNullException(nameof(items)); } - foreach (var (element, priority) in items) + if (_size == 0) { - Enqueue(element, priority); + _nodes = EnumerableHelpers.ToArray(items, out _size); + + if (_size > 1) + { + Heapify(); + } + } + else + { + foreach (var (element, priority) in items) + { + Enqueue(element, priority); + } } } @@ -266,9 +278,35 @@ public void EnqueueRange(IEnumerable elements, TPriority priority) throw new ArgumentNullException(nameof(elements)); } - foreach (var element in elements) + if (_size == 0) { - Enqueue(element, priority); + using (var eumerator = elements.GetEnumerator()) + { + if (eumerator.MoveNext()) + { + _nodes = new (TElement, TPriority)[MinimumGrow]; + _nodes[0] = (eumerator.Current, priority); + _size = 1; + + while (eumerator.MoveNext()) + { + EnsureEnoughCapacityBeforeAddingNode(); + _nodes[_size++] = (eumerator.Current, priority); + } + } + } + + if (_size > 1) + { + Heapify(); + } + } + else + { + foreach (var element in elements) + { + Enqueue(element, priority); + } } } @@ -317,8 +355,7 @@ public void TrimExcess() int threshold = (int)(((double)_nodes.Length) * 0.9); if (_size < threshold) { - Array.Resize(ref _nodes, _size); - _version++; + SetCapacity(_size); } } @@ -341,6 +378,7 @@ private void EnsureEnoughCapacityBeforeAddingNode() private void SetCapacity(int capacity) { Array.Resize(ref _nodes, capacity); + _version++; } /// From 58a903271d8192952465eb7f73a4c31dbe129498 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 23 Jan 2021 12:30:19 +0100 Subject: [PATCH 29/50] (draft step, to squash) Capitalize members of (TElement, TPriority)[] --- .../System/Collections/Generic/PriorityQueue.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 254b811eff4db9..5e0926ddd93a37 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -18,7 +18,7 @@ public class PriorityQueue /// /// Represents an implicit heap-ordered complete d-ary tree, stored as an array. /// - private (TElement element, TPriority priority)[] _nodes; + private (TElement Element, TPriority Priority)[] _nodes; private UnorderedItemsCollection? _unorderedItems; @@ -224,7 +224,7 @@ public TElement EnqueueDequeue(TElement element, TPriority priority) { var root = _nodes[RootIndex]; - if (this.Comparer.Compare(priority, root.priority) < 0) + if (this.Comparer.Compare(priority, root.Priority) < 0) { return element; } @@ -236,7 +236,7 @@ public TElement EnqueueDequeue(TElement element, TPriority priority) MoveDown(newRoot, RootIndex); _version++; - return root.element; + return root.Element; } } @@ -409,7 +409,7 @@ private void Remove(int indexOfNodeToRemove) var nodeToRemove = _nodes[indexOfNodeToRemove]; - int relation = this.Comparer.Compare(lastNode.priority, nodeToRemove.priority); + int relation = this.Comparer.Compare(lastNode.Priority, nodeToRemove.Priority); PutAt(lastNode, indexOfNodeToRemove); if (relation < 0) @@ -477,7 +477,7 @@ private void MoveUp((TElement element, TPriority priority) node, int nodeIndex) int parentIndex = GetParentIndex(nodeIndex); var parent = _nodes[parentIndex]; - if (this.Comparer.Compare(node.priority, parent.priority) < 0) + if (this.Comparer.Compare(node.priority, parent.Priority) < 0) { PutAt(parent, nodeIndex); nodeIndex = parentIndex; @@ -512,7 +512,7 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex while (++i < childrenIndexesLimit) { var child = _nodes[i]; - if (this.Comparer.Compare(child.priority, topChild.priority) < 0) + if (this.Comparer.Compare(child.Priority, topChild.Priority) < 0) { topChild = child; topChildIndex = i; @@ -521,7 +521,7 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex // In case no child needs to be extracted earlier than the current node, // there is nothing more to do - the right spot was found. - if (this.Comparer.Compare(node.priority, topChild.priority) <= 0) + if (this.Comparer.Compare(node.priority, topChild.Priority) <= 0) { break; } From f8b4564ca269c0b1b75e779dd987a8bee10e3fb0 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 23 Jan 2021 12:37:20 +0100 Subject: [PATCH 30/50] (draft step, to squash) Improve resize constants --- .../src/System/Collections/Generic/PriorityQueue.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 5e0926ddd93a37..e4951f5267553a 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -32,8 +32,7 @@ public class PriorityQueue /// private int _version; - private const int MinimumGrow = 4; - private const int GrowFactor = 200; // double each time + private const int MinimumElementsToGrowBy = 4; private const int RootIndex = 0; @@ -284,7 +283,7 @@ public void EnqueueRange(IEnumerable elements, TPriority priority) { if (eumerator.MoveNext()) { - _nodes = new (TElement, TPriority)[MinimumGrow]; + _nodes = new (TElement, TPriority)[MinimumElementsToGrowBy]; _nodes[0] = (eumerator.Current, priority); _size = 1; @@ -363,10 +362,11 @@ private void EnsureEnoughCapacityBeforeAddingNode() { if (_size == _nodes.Length) { - int newCapacity = (int)((long)_nodes.Length * (long)GrowFactor / 100); - if (newCapacity < _nodes.Length + MinimumGrow) + const int GrowthFactor = 2; + int newCapacity = _nodes.Length * GrowthFactor; + if (newCapacity < _nodes.Length + MinimumElementsToGrowBy) { - newCapacity = _nodes.Length + MinimumGrow; + newCapacity = _nodes.Length + MinimumElementsToGrowBy; } SetCapacity(newCapacity); } From 5851006dc867e7fba99c789e02b4d11948792fa7 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 23 Jan 2021 12:39:46 +0100 Subject: [PATCH 31/50] (draft step, to squash) Remove redundant `.this` --- .../System/Collections/Generic/PriorityQueue.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index e4951f5267553a..6962420332362d 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -81,7 +81,7 @@ public PriorityQueue(int initialCapacity, IComparer? comparer) ? Array.Empty<(TElement, TPriority)>() : new (TElement, TPriority)[initialCapacity]; - this.Comparer = comparer ?? Comparer.Default; + Comparer = comparer ?? Comparer.Default; } /// @@ -104,7 +104,7 @@ public PriorityQueue(IEnumerable<(TElement element, TPriority priority)> items, } _nodes = EnumerableHelpers.ToArray(items, out _size); - this.Comparer = comparer ?? Comparer.Default; + Comparer = comparer ?? Comparer.Default; if (_size > 1) { @@ -223,7 +223,7 @@ public TElement EnqueueDequeue(TElement element, TPriority priority) { var root = _nodes[RootIndex]; - if (this.Comparer.Compare(priority, root.Priority) < 0) + if (Comparer.Compare(priority, root.Priority) < 0) { return element; } @@ -409,7 +409,7 @@ private void Remove(int indexOfNodeToRemove) var nodeToRemove = _nodes[indexOfNodeToRemove]; - int relation = this.Comparer.Compare(lastNode.Priority, nodeToRemove.Priority); + int relation = Comparer.Compare(lastNode.Priority, nodeToRemove.Priority); PutAt(lastNode, indexOfNodeToRemove); if (relation < 0) @@ -477,7 +477,7 @@ private void MoveUp((TElement element, TPriority priority) node, int nodeIndex) int parentIndex = GetParentIndex(nodeIndex); var parent = _nodes[parentIndex]; - if (this.Comparer.Compare(node.priority, parent.Priority) < 0) + if (Comparer.Compare(node.priority, parent.Priority) < 0) { PutAt(parent, nodeIndex); nodeIndex = parentIndex; @@ -512,7 +512,7 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex while (++i < childrenIndexesLimit) { var child = _nodes[i]; - if (this.Comparer.Compare(child.Priority, topChild.Priority) < 0) + if (Comparer.Compare(child.Priority, topChild.Priority) < 0) { topChild = child; topChildIndex = i; @@ -521,7 +521,7 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex // In case no child needs to be extracted earlier than the current node, // there is nothing more to do - the right spot was found. - if (this.Comparer.Compare(node.priority, topChild.Priority) <= 0) + if (Comparer.Compare(node.priority, topChild.Priority) <= 0) { break; } @@ -678,7 +678,7 @@ private void ThrowEnumerationNotStartedOrEnded() throw new InvalidOperationException(message); } - object IEnumerator.Current => this.Current; + object IEnumerator.Current => Current; void IEnumerator.Reset() { From ae630c2834e15ec7ebe1e5175278132ed9db84d3 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 23 Jan 2021 12:42:11 +0100 Subject: [PATCH 32/50] (draft step, to squash) Optimize EnqueueDequeue --- .../src/System/Collections/Generic/PriorityQueue.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 6962420332362d..45eae82a4de854 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -223,7 +223,7 @@ public TElement EnqueueDequeue(TElement element, TPriority priority) { var root = _nodes[RootIndex]; - if (Comparer.Compare(priority, root.Priority) < 0) + if (Comparer.Compare(priority, root.Priority) <= 0) { return element; } From fef750282d28d2597ae112502cc5eb0426dad4e0 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 23 Jan 2021 12:50:02 +0100 Subject: [PATCH 33/50] (draft step, to squash) Reduce indentation --- .../Collections/Generic/PriorityQueue.cs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 45eae82a4de854..a987cd04a9f25d 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -360,16 +360,18 @@ public void TrimExcess() private void EnsureEnoughCapacityBeforeAddingNode() { - if (_size == _nodes.Length) + if (_size < _nodes.Length) { - const int GrowthFactor = 2; - int newCapacity = _nodes.Length * GrowthFactor; - if (newCapacity < _nodes.Length + MinimumElementsToGrowBy) - { - newCapacity = _nodes.Length + MinimumElementsToGrowBy; - } - SetCapacity(newCapacity); + return; + } + + const int GrowthFactor = 2; + int newCapacity = _nodes.Length * GrowthFactor; + if (newCapacity < _nodes.Length + MinimumElementsToGrowBy) + { + newCapacity = _nodes.Length + MinimumElementsToGrowBy; } + SetCapacity(newCapacity); } /// From 18301b22833758f008e3a0ae6d31f3d5c7317fba Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 23 Jan 2021 13:03:46 +0100 Subject: [PATCH 34/50] (draft step, to squash) Simplify math expressions --- .../src/System/Collections/Generic/PriorityQueue.cs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index a987cd04a9f25d..fb88323d73dd2b 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -39,7 +39,12 @@ public class PriorityQueue /// /// Specifies the arity of the d-ary heap, which here is quaternary. /// - private const uint Arity = 4; + private const int Arity = 4; + + /// + /// The binary logarithm of . + /// + private const int ArityLog2 = 2; /// /// Creates an empty priority queue. @@ -351,7 +356,7 @@ public int EnsureCapacity(int capacity) /// public void TrimExcess() { - int threshold = (int)(((double)_nodes.Length) * 0.9); + int threshold = (int)(_nodes.Length * 0.9); if (_size < threshold) { SetCapacity(_size); @@ -440,7 +445,7 @@ private void PutAt((TElement element, TPriority priority) node, int index) /// /// Gets the index of an element's parent. /// - private int GetParentIndex(int index) => (int)((index - 1) / Arity); + private int GetParentIndex(int index) => (index - 1) >> ArityLog2; /// /// Gets the index of the first child of an element. @@ -508,7 +513,7 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex // Check if the current node (pointed by 'nodeIndex') should really be extracted // first, or maybe one of its children should be extracted earlier. var topChild = _nodes[i]; - int childrenIndexesLimit = (int)Math.Min(i + Arity, _size); + int childrenIndexesLimit = Math.Min(i + Arity, _size); int topChildIndex = i; while (++i < childrenIndexesLimit) From 2bde65bc92a959bea8b7fdef0ab8c70a71471b85 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 23 Jan 2021 13:08:19 +0100 Subject: [PATCH 35/50] (draft step, to squash) Remove the PutAt helper method --- .../Collections/Generic/PriorityQueue.cs | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index fb88323d73dd2b..05d2899c0bec21 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -417,7 +417,7 @@ private void Remove(int indexOfNodeToRemove) var nodeToRemove = _nodes[indexOfNodeToRemove]; int relation = Comparer.Compare(lastNode.Priority, nodeToRemove.Priority); - PutAt(lastNode, indexOfNodeToRemove); + _nodes[indexOfNodeToRemove] = lastNode; if (relation < 0) { @@ -429,14 +429,6 @@ private void Remove(int indexOfNodeToRemove) } } - /// - /// Puts a node at the specified index. - /// - private void PutAt((TElement element, TPriority priority) node, int index) - { - _nodes[index] = node; - } - /// /// Gets the index of the last node in the heap. /// @@ -486,7 +478,7 @@ private void MoveUp((TElement element, TPriority priority) node, int nodeIndex) if (Comparer.Compare(node.priority, parent.Priority) < 0) { - PutAt(parent, nodeIndex); + _nodes[nodeIndex] = parent; nodeIndex = parentIndex; } else @@ -495,7 +487,7 @@ private void MoveUp((TElement element, TPriority priority) node, int nodeIndex) } } - PutAt(node, nodeIndex); + _nodes[nodeIndex] = node; } /// @@ -535,11 +527,11 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex // Move the top child up by one node and now investigate the // node that was considered to be the top child (recursive). - PutAt(topChild, nodeIndex); + _nodes[nodeIndex] = topChild; nodeIndex = topChildIndex; } - PutAt(node, nodeIndex); + _nodes[nodeIndex] = node; } public partial class UnorderedItemsCollection : IReadOnlyCollection<(TElement element, TPriority priority)>, ICollection From 230a0a5338b8049a3f00c6c90efa309ee6e04939 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 23 Jan 2021 13:31:56 +0100 Subject: [PATCH 36/50] (draft step, to squash) Make the UnorderedItemsCollection constructor internal --- src/libraries/System.Collections/ref/System.Collections.cs | 2 +- .../src/System/Collections/Generic/PriorityQueue.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Collections/ref/System.Collections.cs b/src/libraries/System.Collections/ref/System.Collections.cs index c10c961a1dbb91..5b3dbc8c991fbe 100644 --- a/src/libraries/System.Collections/ref/System.Collections.cs +++ b/src/libraries/System.Collections/ref/System.Collections.cs @@ -403,7 +403,7 @@ public void TrimExcess() { } public bool TryPeek([System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TElement element, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TPriority priority) { throw null; } public partial class UnorderedItemsCollection : System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)>, System.Collections.Generic.IReadOnlyCollection<(TElement element, TPriority priority)>, System.Collections.ICollection, System.Collections.IEnumerable { - public UnorderedItemsCollection(PriorityQueue queue) { } + internal UnorderedItemsCollection(PriorityQueue queue) { } public int Count { get { throw null; } } bool System.Collections.ICollection.IsSynchronized { get { throw null; } } object System.Collections.ICollection.SyncRoot { get { throw null; } } diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 05d2899c0bec21..25eb33b8cd3ec9 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -538,7 +538,7 @@ public partial class UnorderedItemsCollection : IReadOnlyCollection<(TElement el { private readonly PriorityQueue _queue; - public UnorderedItemsCollection(PriorityQueue queue) + internal UnorderedItemsCollection(PriorityQueue queue) { _queue = queue; } From abfb09b229606f0a5cc8fa8d9ac26a67be41f5e8 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 23 Jan 2021 13:37:39 +0100 Subject: [PATCH 37/50] (draft step, to squash) Clear last node slot on removal --- .../src/System/Collections/Generic/PriorityQueue.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 25eb33b8cd3ec9..d71704b9f05979 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -400,6 +400,7 @@ private void Remove(int indexOfNodeToRemove) var lastNode = _nodes[lastNodeIndex]; _size--; _version++; + _nodes[lastNodeIndex] = default; // In case we wanted to remove the node that was the last one, // we are done. From f700cbaf9a0ba852d57bd974d795fd4eb233a08e Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 23 Jan 2021 14:03:27 +0100 Subject: [PATCH 38/50] (draft step, to squash) Improve next growth capacity computation --- .../Collections/Generic/PriorityQueue.cs | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index d71704b9f05979..5f03e5cf3e21e5 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -346,6 +346,12 @@ public int EnsureCapacity(int capacity) return currentCapacity; } + int capacityForNextGrowth = ComputeCapacityForNextGrowth(); + if (capacityForNextGrowth > capacity) + { + capacity = capacityForNextGrowth; + } + SetCapacity(capacity); return capacity; } @@ -370,13 +376,30 @@ private void EnsureEnoughCapacityBeforeAddingNode() return; } + int newCapacity = ComputeCapacityForNextGrowth(); + SetCapacity(newCapacity); + } + + private int ComputeCapacityForNextGrowth() + { const int GrowthFactor = 2; + const int MaxArrayLength = 0X7FEFFFFF; + int newCapacity = _nodes.Length * GrowthFactor; if (newCapacity < _nodes.Length + MinimumElementsToGrowBy) { newCapacity = _nodes.Length + MinimumElementsToGrowBy; } - SetCapacity(newCapacity); + + // Allow the structure to grow to maximum possible capacity (~2G elements) before encountering overflow. + // Note that this check works even when _nodes.Length overflowed thanks to the (uint) cast. + + if ((uint)newCapacity > MaxArrayLength) + { + newCapacity = MaxArrayLength; + } + + return newCapacity; } /// From e790b42f8fa41cc3a91f8fa6de06f007b13af992 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 23 Jan 2021 14:12:09 +0100 Subject: [PATCH 39/50] (draft step, to squash) Optimize Enqueue method --- .../src/System/Collections/Generic/PriorityQueue.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 5f03e5cf3e21e5..edd7a7980c79e7 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -139,9 +139,10 @@ public void Enqueue(TElement element, TPriority priority) { EnsureEnoughCapacityBeforeAddingNode(); - // Add the node at the end + // Virtually add the node at the end of the underlying array. + // Note that the node being enqueued does not need to be physically placed + // there at this point, as such an assignment would be redundant. var node = (element, priority); - _nodes[_size] = node; _size++; _version++; From 07ec07ce05f351e4dc6b3260113e2c169f22109a Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 23 Jan 2021 14:16:37 +0100 Subject: [PATCH 40/50] (draft step, to squash) Make UnorderedItemsCollection.CopyTo implemented explicitly on ICollection --- src/libraries/System.Collections/ref/System.Collections.cs | 2 +- .../src/System/Collections/Generic/PriorityQueue.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Collections/ref/System.Collections.cs b/src/libraries/System.Collections/ref/System.Collections.cs index 5b3dbc8c991fbe..8f77ab2074c391 100644 --- a/src/libraries/System.Collections/ref/System.Collections.cs +++ b/src/libraries/System.Collections/ref/System.Collections.cs @@ -407,7 +407,7 @@ internal UnorderedItemsCollection(PriorityQueue queue) { } public int Count { get { throw null; } } bool System.Collections.ICollection.IsSynchronized { get { throw null; } } object System.Collections.ICollection.SyncRoot { get { throw null; } } - public void CopyTo(System.Array array, int index) { } + void ICollection.CopyTo(System.Array array, int index) { } public System.Collections.Generic.PriorityQueue.UnorderedItemsCollection.Enumerator GetEnumerator() { throw null; } System.Collections.Generic.IEnumerator<(TElement element, TPriority priority)> System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)>.GetEnumerator() { throw null; } System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; } diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index edd7a7980c79e7..15755bf5ac7965 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -572,7 +572,7 @@ internal UnorderedItemsCollection(PriorityQueue queue) object ICollection.SyncRoot => this; bool ICollection.IsSynchronized => false; - public void CopyTo(Array array, int index) + void ICollection.CopyTo(Array array, int index) { if (array == null) { From ecf4c584bbf082e0bb90dbfe4ebdd45fd8a125ea Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 23 Jan 2021 14:30:25 +0100 Subject: [PATCH 41/50] (draft step, to squash) Improve priority queue tests --- .../PriorityQueue.Generic.Tests.cs | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs index d26f69a9992b77..972c101e3dc3bf 100644 --- a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs +++ b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs @@ -13,7 +13,8 @@ public abstract class PriorityQueue_Generic_Tests : TestBas protected IEnumerable<(TElement, TPriority)> GenericIEnumerableFactory(int count) { - int seed = count * 34; + const int MagicValue = 34; + int seed = count * MagicValue; for (int i = 0; i < count; i++) { yield return CreateT(seed++); @@ -113,10 +114,7 @@ public void PriorityQueue_Generic_Enqueue(int count) Assert.Equal(count, queue.Count); var actualItems = queue.UnorderedItems.ToArray(); - foreach (var (element, priority) in actualItems) - { - Assert.True(expectedItems.Contains((element, priority))); - } + Assert.True(expectedItems.SetEquals(actualItems)); } [Fact] @@ -287,5 +285,23 @@ void trimAndEnsureCapacity() } #endregion + + #region Enumeration ordering + + [Theory] + [MemberData(nameof(ValidPositiveCollectionSizes))] + public void PriorityQueue_EnumerationIsConsistent(int count) + { + PriorityQueue queue = this.GenericPriorityQueueFactory(initialCapacity: 0, count, out _); + + (TElement, TPriority)[] firstEnumeration = queue.UnorderedItems.ToArray(); + (TElement, TPriority)[] secondEnumeration = queue.UnorderedItems.ToArray(); + + Assert.Equal(firstEnumeration.Length, count); + Assert.True(firstEnumeration.SequenceEqual(secondEnumeration)); + + } + + #endregion } } From a877864e8cba9c5ce519aaf62286716f47fb81dd Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sun, 24 Jan 2021 22:32:32 +0100 Subject: [PATCH 42/50] (draft step, to squash) Drop redundant casting --- .../src/System/Collections/Generic/PriorityQueue.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 15755bf5ac7965..2bb3fc2144347a 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -467,7 +467,7 @@ private void Remove(int indexOfNodeToRemove) /// /// Gets the index of the first child of an element. /// - private int GetFirstChildIndex(int index) => (int)(Arity * index + 1); + private int GetFirstChildIndex(int index) => Arity * index + 1; /// /// Converts an unordered list into a heap. From 26d4ce776017c8487613cba86d8182175b536c4d Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Thu, 28 Jan 2021 22:29:18 +0100 Subject: [PATCH 43/50] (draft step, to squash) Cosmetic improvements --- .../Collections/Generic/PriorityQueue.cs | 56 +++++++++---------- 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 2bb3fc2144347a..ce272ddc1dec2f 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -44,7 +44,7 @@ public class PriorityQueue /// /// The binary logarithm of . /// - private const int ArityLog2 = 2; + private const int Log2Arity = 2; /// /// Creates an empty priority queue. @@ -142,7 +142,7 @@ public void Enqueue(TElement element, TPriority priority) // Virtually add the node at the end of the underlying array. // Note that the node being enqueued does not need to be physically placed // there at this point, as such an assignment would be redundant. - var node = (element, priority); + (TElement Element, TPriority Priority) node = (element, priority); _size++; _version++; @@ -157,14 +157,12 @@ public void Enqueue(TElement element, TPriority priority) /// The queue is empty. public TElement Peek() { - if (TryPeek(out TElement? element, out _)) - { - return element; - } - else + if (_size == 0) { throw new InvalidOperationException(SR.InvalidOperation_EmptyQueue); } + + return _nodes[RootIndex].Element; } /// @@ -173,14 +171,14 @@ public TElement Peek() /// The queue is empty. public TElement Dequeue() { - if (TryDequeue(out TElement? element, out _)) - { - return element; - } - else + if (_size == 0) { throw new InvalidOperationException(SR.InvalidOperation_EmptyQueue); } + + TElement element = _nodes[RootIndex].Element; + Remove(RootIndex); + return element; } /// @@ -227,7 +225,7 @@ public bool TryPeek([MaybeNullWhen(false)] out TElement element, [MaybeNullWhen( /// public TElement EnqueueDequeue(TElement element, TPriority priority) { - var root = _nodes[RootIndex]; + (TElement Element, TPriority Priority) root = _nodes[RootIndex]; if (Comparer.Compare(priority, root.Priority) <= 0) { @@ -235,7 +233,7 @@ public TElement EnqueueDequeue(TElement element, TPriority priority) } else { - var newRoot = (element, priority); + (TElement Element, TPriority Priority) newRoot = (element, priority); _nodes[RootIndex] = newRoot; MoveDown(newRoot, RootIndex); @@ -266,7 +264,7 @@ public void EnqueueRange(IEnumerable<(TElement Element, TPriority Priority)> ite } else { - foreach (var (element, priority) in items) + foreach ((TElement element, TPriority priority) in items) { Enqueue(element, priority); } @@ -285,18 +283,18 @@ public void EnqueueRange(IEnumerable elements, TPriority priority) if (_size == 0) { - using (var eumerator = elements.GetEnumerator()) + using (IEnumerator enumerator = elements.GetEnumerator()) { - if (eumerator.MoveNext()) + if (enumerator.MoveNext()) { _nodes = new (TElement, TPriority)[MinimumElementsToGrowBy]; - _nodes[0] = (eumerator.Current, priority); + _nodes[0] = (enumerator.Current, priority); _size = 1; - while (eumerator.MoveNext()) + while (enumerator.MoveNext()) { EnsureEnoughCapacityBeforeAddingNode(); - _nodes[_size++] = (eumerator.Current, priority); + _nodes[_size++] = (enumerator.Current, priority); } } } @@ -308,7 +306,7 @@ public void EnqueueRange(IEnumerable elements, TPriority priority) } else { - foreach (var element in elements) + foreach (TElement element in elements) { Enqueue(element, priority); } @@ -421,10 +419,10 @@ private void Remove(int indexOfNodeToRemove) // node and shorten the array by one. int lastNodeIndex = GetLastNodeIndex(); - var lastNode = _nodes[lastNodeIndex]; + (TElement Element, TPriority Priority) lastNode = _nodes[lastNodeIndex]; + _nodes[lastNodeIndex] = default; _size--; _version++; - _nodes[lastNodeIndex] = default; // In case we wanted to remove the node that was the last one, // we are done. @@ -439,7 +437,7 @@ private void Remove(int indexOfNodeToRemove) // wanted to remove. After that operation, we will need // to restore the heap property (in general). - var nodeToRemove = _nodes[indexOfNodeToRemove]; + (TElement Element, TPriority Priority) nodeToRemove = _nodes[indexOfNodeToRemove]; int relation = Comparer.Compare(lastNode.Priority, nodeToRemove.Priority); _nodes[indexOfNodeToRemove] = lastNode; @@ -462,7 +460,7 @@ private void Remove(int indexOfNodeToRemove) /// /// Gets the index of an element's parent. /// - private int GetParentIndex(int index) => (index - 1) >> ArityLog2; + private int GetParentIndex(int index) => (index - 1) >> Log2Arity; /// /// Gets the index of the first child of an element. @@ -499,7 +497,7 @@ private void MoveUp((TElement element, TPriority priority) node, int nodeIndex) while (nodeIndex > 0) { int parentIndex = GetParentIndex(nodeIndex); - var parent = _nodes[parentIndex]; + (TElement Element, TPriority Priority) parent = _nodes[parentIndex]; if (Comparer.Compare(node.priority, parent.Priority) < 0) { @@ -529,13 +527,13 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex { // Check if the current node (pointed by 'nodeIndex') should really be extracted // first, or maybe one of its children should be extracted earlier. - var topChild = _nodes[i]; + (TElement Element, TPriority Priority) topChild = _nodes[i]; int childrenIndexesLimit = Math.Min(i + Arity, _size); int topChildIndex = i; while (++i < childrenIndexesLimit) { - var child = _nodes[i]; + (TElement Element, TPriority Priority) child = _nodes[i]; if (Comparer.Compare(child.Priority, topChild.Priority) < 0) { topChild = child; @@ -574,7 +572,7 @@ internal UnorderedItemsCollection(PriorityQueue queue) void ICollection.CopyTo(Array array, int index) { - if (array == null) + if (array is null) { throw new ArgumentNullException(nameof(array)); } From a3c114a0f0ddd7f3041870d3df2366944e37b7f5 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 13 Feb 2021 12:58:45 +0100 Subject: [PATCH 44/50] (draft step, to squash) Change signature of UnorderedItemsCollection --- src/libraries/System.Collections/ref/System.Collections.cs | 2 +- .../src/System/Collections/Generic/PriorityQueue.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Collections/ref/System.Collections.cs b/src/libraries/System.Collections/ref/System.Collections.cs index 8f77ab2074c391..5409b03b22c5ad 100644 --- a/src/libraries/System.Collections/ref/System.Collections.cs +++ b/src/libraries/System.Collections/ref/System.Collections.cs @@ -401,7 +401,7 @@ public void EnqueueRange(System.Collections.Generic.IEnumerable elemen public void TrimExcess() { } public bool TryDequeue([System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TElement element, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TPriority priority) { throw null; } public bool TryPeek([System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TElement element, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TPriority priority) { throw null; } - public partial class UnorderedItemsCollection : System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)>, System.Collections.Generic.IReadOnlyCollection<(TElement element, TPriority priority)>, System.Collections.ICollection, System.Collections.IEnumerable + public sealed partial class UnorderedItemsCollection : System.Collections.Generic.IEnumerable<(TElement element, TPriority priority)>, System.Collections.Generic.IReadOnlyCollection<(TElement element, TPriority priority)>, System.Collections.ICollection, System.Collections.IEnumerable { internal UnorderedItemsCollection(PriorityQueue queue) { } public int Count { get { throw null; } } diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index ce272ddc1dec2f..11c2fe693d8e31 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -557,7 +557,7 @@ private void MoveDown((TElement element, TPriority priority) node, int nodeIndex _nodes[nodeIndex] = node; } - public partial class UnorderedItemsCollection : IReadOnlyCollection<(TElement element, TPriority priority)>, ICollection + public sealed class UnorderedItemsCollection : IReadOnlyCollection<(TElement element, TPriority priority)>, ICollection { private readonly PriorityQueue _queue; From 3dbc4f5e00df072a9a6cf5e535eea60137d640f3 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 13 Feb 2021 13:09:33 +0100 Subject: [PATCH 45/50] (draft step, to squash) Add test PriorityQueue_Generic_EnqueueDequeue_EqualToMin --- .../Generic/PriorityQueue/PriorityQueue.Tests.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs index c604408be08631..f4935361c90f13 100644 --- a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs +++ b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs @@ -44,5 +44,16 @@ public void PriorityQueue_Generic_EnqueueDequeue_LargerThanMin() Assert.Equal("three", queue.Dequeue()); Assert.Equal("four", queue.Dequeue()); } + + [Fact] + public void PriorityQueue_Generic_EnqueueDequeue_EqualToMin() + { + PriorityQueue queue = SmallPriorityQueueFactory(out HashSet<(string, int)> enqueuedItems); + + string actualElement = queue.EnqueueDequeue("one-not-to-enqueue", 1); + + Assert.Equal("one-not-to-enqueue", actualElement); + Assert.True(enqueuedItems.SetEquals(queue.UnorderedItems)); + } } } From 8672c4161dfc3c7710c2b53a1986cff416ed978c Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 13 Feb 2021 13:25:15 +0100 Subject: [PATCH 46/50] (draft step, to squash) Add tests of enqueue null functionality --- .../PriorityQueue/PriorityQueue.Tests.cs | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs index f4935361c90f13..10ef97993b3c97 100644 --- a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs +++ b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Tests.cs @@ -55,5 +55,45 @@ public void PriorityQueue_Generic_EnqueueDequeue_EqualToMin() Assert.Equal("one-not-to-enqueue", actualElement); Assert.True(enqueuedItems.SetEquals(queue.UnorderedItems)); } + + [Fact] + public void PriorityQueue_Generic_Constructor_IEnumerable_Null() + { + (string, int)[] itemsToEnqueue = new(string, int)[] { (null, 0), ("one", 1) } ; + PriorityQueue queue = new PriorityQueue(itemsToEnqueue); + Assert.Null(queue.Dequeue()); + Assert.Equal("one", queue.Dequeue()); + } + + [Fact] + public void PriorityQueue_Generic_Enqueue_Null() + { + PriorityQueue queue = new PriorityQueue(); + + queue.Enqueue(element: null, 1); + queue.Enqueue(element: "zero", 0); + queue.Enqueue(element: "two", 2); + + Assert.Equal("zero", queue.Dequeue()); + Assert.Null(queue.Dequeue()); + Assert.Equal("two", queue.Dequeue()); + } + + [Fact] + public void PriorityQueue_Generic_EnqueueRange_Null() + { + PriorityQueue queue = new PriorityQueue(); + + queue.EnqueueRange(new string[] { null, null, null }, 0); + queue.EnqueueRange(new string[] { "not null" }, 1); + queue.EnqueueRange(new string[] { null, null, null }, 0); + + for (int i = 0; i < 6; ++i) + { + Assert.Null(queue.Dequeue()); + } + + Assert.Equal("not null", queue.Dequeue()); + } } } From 85aeed15249e570edb35b36a3fca1ce1415e3c80 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 13 Feb 2021 13:38:41 +0100 Subject: [PATCH 47/50] (draft step, to squash) Add test PriorityQueue_Generic_EnsureCapacity_Negative --- .../Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs index 972c101e3dc3bf..2f4c28e73d288b 100644 --- a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs +++ b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs @@ -224,6 +224,14 @@ public void PriorityQueue_Generic_Clear(int count) #region EnsureCapacity, TrimExcess + [Fact] + public void PriorityQueue_Generic_EnsureCapacity_Negative() + { + PriorityQueue queue = new PriorityQueue(); + AssertExtensions.Throws("capacity", () => queue.EnsureCapacity(-1)); + AssertExtensions.Throws("capacity", () => queue.EnsureCapacity(int.MinValue)); + } + [Theory] [InlineData(0, 0)] [InlineData(0, 5)] From 938ef63ae3760acafa74573559a8e3bbf9f14c64 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 13 Feb 2021 14:03:59 +0100 Subject: [PATCH 48/50] (draft step, to squash) Check underlying buffer length in tests with reflection --- .../PriorityQueue/PriorityQueue.Generic.Tests.cs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs index 2f4c28e73d288b..82c312c429e6f7 100644 --- a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs +++ b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; +using System.Reflection; using Xunit; namespace System.Collections.Tests @@ -268,8 +269,13 @@ public void PriorityQueue_Generic_EnsureCapacityAndTrimExcess(int count) void trimAndEnsureCapacity() { queue.TrimExcess(); - queue.EnsureCapacity(getNextEnsureCapacity()); + + int capacityAfterEnsureCapacity = queue.EnsureCapacity(getNextEnsureCapacity()); + Assert.Equal(capacityAfterEnsureCapacity, GetUnderlyingBufferCapacity(queue)); + + int capacityAfterTrimExcess = (queue.Count < (int)(capacityAfterEnsureCapacity * 0.9)) ? queue.Count : capacityAfterEnsureCapacity; queue.TrimExcess(); + Assert.Equal(capacityAfterTrimExcess, GetUnderlyingBufferCapacity(queue)); }; foreach (var (element, priority) in itemsToEnqueue) @@ -292,6 +298,13 @@ void trimAndEnsureCapacity() Assert.Equal(0, queue.Count); } + private static int GetUnderlyingBufferCapacity(PriorityQueue queue) + { + FieldInfo nodesType = queue.GetType().GetField("_nodes", BindingFlags.NonPublic | BindingFlags.Instance); + var nodes = ((TElement Element, TPriority Priority)[])nodesType.GetValue(queue); + return nodes.Length; + } + #endregion #region Enumeration ordering From 475183a698717c5528b4441281fc55e7a8ef8b7b Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sat, 13 Feb 2021 14:20:46 +0100 Subject: [PATCH 49/50] (draft step, to squash) Check enumeration invalidation --- .../PriorityQueue.Generic.Tests.cs | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs index 82c312c429e6f7..7100b9fdb9fe50 100644 --- a/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs +++ b/src/libraries/System.Collections/tests/Generic/PriorityQueue/PriorityQueue.Generic.Tests.cs @@ -307,11 +307,11 @@ private static int GetUnderlyingBufferCapacity(PriorityQueue queue = this.GenericPriorityQueueFactory(initialCapacity: 0, count, out _); @@ -320,7 +320,35 @@ public void PriorityQueue_EnumerationIsConsistent(int count) Assert.Equal(firstEnumeration.Length, count); Assert.True(firstEnumeration.SequenceEqual(secondEnumeration)); + } + + [Theory] + [MemberData(nameof(ValidPositiveCollectionSizes))] + public void PriorityQueue_Enumeration_InvalidationOnModifiedCollection(int count) + { + IReadOnlyCollection<(TElement, TPriority)> itemsToEnqueue = this.GenericIEnumerableFactory(count).ToArray(); + PriorityQueue queue = new PriorityQueue(); + queue.EnqueueRange(itemsToEnqueue.Take(count - 1)); + var enumerator = queue.UnorderedItems.GetEnumerator(); + + (TElement element, TPriority priority) = itemsToEnqueue.Last(); + queue.Enqueue(element, priority); + Assert.Throws(() => enumerator.MoveNext()); + } + + [Theory] + [MemberData(nameof(ValidPositiveCollectionSizes))] + public void PriorityQueue_Enumeration_InvalidationOnModifiedCapacity(int count) + { + PriorityQueue queue = this.GenericPriorityQueueFactory(initialCapacity: 0, count, out _); + var enumerator = queue.UnorderedItems.GetEnumerator(); + + int capacityBefore = GetUnderlyingBufferCapacity(queue); + queue.EnsureCapacity(count * 2 + 4); + int capacityAfter = GetUnderlyingBufferCapacity(queue); + Assert.NotEqual(capacityBefore, capacityAfter); + Assert.Throws(() => enumerator.MoveNext()); } #endregion From 4f79c967282e3106b7f37e753b06c493761b3965 Mon Sep 17 00:00:00 2001 From: Patryk Golebiowski Date: Sun, 14 Feb 2021 15:23:18 +0100 Subject: [PATCH 50/50] (draft step, to squash) Simplify code and improve documentation --- .../Collections/Generic/PriorityQueue.cs | 78 ++++++++----------- 1 file changed, 33 insertions(+), 45 deletions(-) diff --git a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs index 11c2fe693d8e31..80fca47e814ace 100644 --- a/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs +++ b/src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs @@ -8,8 +8,8 @@ namespace System.Collections.Generic { /// - /// Represents a data structure in which each element additionally has a priority - /// associated with it. + /// Represents a data structure in which each element has an associated priority + /// that determines the order in which the pair is dequeued. /// /// The type of the element. /// The type of the priority. @@ -28,12 +28,19 @@ public class PriorityQueue private int _size; /// - /// Used to keep enumerator in sync with the collection. + /// Version updated on mutation to help validate enumerators operate on a consistent state. /// private int _version; + /// + /// When the underlying buffer for the heap nodes grows to accomodate more nodes, + /// this is the minimum the capacity will grow by. + /// private const int MinimumElementsToGrowBy = 4; + /// + /// The index at which the heap root is maintained. + /// private const int RootIndex = 0; /// @@ -142,13 +149,12 @@ public void Enqueue(TElement element, TPriority priority) // Virtually add the node at the end of the underlying array. // Note that the node being enqueued does not need to be physically placed // there at this point, as such an assignment would be redundant. - (TElement Element, TPriority Priority) node = (element, priority); _size++; _version++; // Restore the heap order int lastNodeIndex = GetLastNodeIndex(); - MoveUp(node, lastNodeIndex); + MoveUp((element, priority), lastNodeIndex); } /// @@ -296,12 +302,12 @@ public void EnqueueRange(IEnumerable elements, TPriority priority) EnsureEnoughCapacityBeforeAddingNode(); _nodes[_size++] = (enumerator.Current, priority); } - } - } - if (_size > 1) - { - Heapify(); + if (_size > 1) + { + Heapify(); + } + } } } else @@ -335,24 +341,15 @@ public int EnsureCapacity(int capacity) { if (capacity < 0) { - throw new ArgumentOutOfRangeException( - nameof(capacity), capacity, SR.ArgumentOutOfRange_NeedNonNegNum); + throw new ArgumentOutOfRangeException(nameof(capacity), capacity, SR.ArgumentOutOfRange_NeedNonNegNum); } - int currentCapacity = _nodes.Length; - if (currentCapacity >= capacity) + if (_nodes.Length < capacity) { - return currentCapacity; + SetCapacity(Math.Max(capacity, ComputeCapacityForNextGrowth())); } - int capacityForNextGrowth = ComputeCapacityForNextGrowth(); - if (capacityForNextGrowth > capacity) - { - capacity = capacityForNextGrowth; - } - - SetCapacity(capacity); - return capacity; + return _nodes.Length; } /// @@ -370,13 +367,11 @@ public void TrimExcess() private void EnsureEnoughCapacityBeforeAddingNode() { - if (_size < _nodes.Length) + Debug.Assert(_size <= _nodes.Length); + if (_size == _nodes.Length) { - return; + SetCapacity(ComputeCapacityForNextGrowth()); } - - int newCapacity = ComputeCapacityForNextGrowth(); - SetCapacity(newCapacity); } private int ComputeCapacityForNextGrowth() @@ -384,11 +379,7 @@ private int ComputeCapacityForNextGrowth() const int GrowthFactor = 2; const int MaxArrayLength = 0X7FEFFFFF; - int newCapacity = _nodes.Length * GrowthFactor; - if (newCapacity < _nodes.Length + MinimumElementsToGrowBy) - { - newCapacity = _nodes.Length + MinimumElementsToGrowBy; - } + int newCapacity = Math.Max(_nodes.Length * GrowthFactor, _nodes.Length + MinimumElementsToGrowBy); // Allow the structure to grow to maximum possible capacity (~2G elements) before encountering overflow. // Note that this check works even when _nodes.Length overflowed thanks to the (uint) cast. @@ -633,8 +624,6 @@ public void Dispose() public bool MoveNext() { - bool advancedEnumerator; - if (_version != _queue._version) { throw new InvalidOperationException(SR.InvalidOperation_EnumFailedVersion); @@ -642,19 +631,17 @@ public bool MoveNext() if (_index == FirstCallToEnumerator) { - _index = 0; - advancedEnumerator = (_queue._size > 0); - - if (advancedEnumerator) + if (_queue._size > 0) { + _index = 0; _currentElement = _queue._nodes[_index]; + return true; } else { _index = EndOfEnumeration; + return false; } - - return advancedEnumerator; } if (_index == EndOfEnumeration) @@ -662,19 +649,20 @@ public bool MoveNext() return false; } + // advance enumerator _index++; - advancedEnumerator = (_index < _queue._size); - if (advancedEnumerator) + if (_index < _queue._size) { _currentElement = _queue._nodes[_index]; + return true; } else { + _index = EndOfEnumeration; _currentElement = default; + return false; } - - return advancedEnumerator; } public (TElement element, TPriority priority) Current