From 1ce91c00621ea90c63121b4e4790795f6b2f578e Mon Sep 17 00:00:00 2001 From: mlemanczyk Date: Fri, 7 Apr 2023 01:09:25 +0200 Subject: [PATCH] RecyclableArrayList.Remove, RecyclableArrayList.RemoveAt, RecyclableList.Remove & RecyclableList.RemoveAt performance improvements. Support for removal of any item implemented in RecyclableList.Remove & .RemoveAt. --- Recyclable.Collections/RecyclableArrayList.cs | 25 +++-- Recyclable.Collections/RecyclableList.cs | 98 ++++++++++++++----- .../RecyclableListTests.cs | 63 +++++++++--- 3 files changed, 135 insertions(+), 51 deletions(-) diff --git a/Recyclable.Collections/RecyclableArrayList.cs b/Recyclable.Collections/RecyclableArrayList.cs index 1d4bce4..8333cd7 100644 --- a/Recyclable.Collections/RecyclableArrayList.cs +++ b/Recyclable.Collections/RecyclableArrayList.cs @@ -411,6 +411,7 @@ namespace Recyclable.Collections _count++; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool Remove(T itemToRemove) { var itemIdx = IndexOf(itemToRemove); @@ -423,24 +424,28 @@ namespace Recyclable.Collections return false; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void RemoveAt(int index) { - int oldCount = _count; - int oldCountMinus1 = oldCount - 1; - if (index == oldCountMinus1) + if (index == _count - 1) { _count--; - return; } - - if (index >= oldCount) + else if (index > _count - 1) { - ThrowArgumentOutOfRangeException($"Argument \"{index}\" is out of range. The current version supports removing the last element, only "); - return; + ThrowArgumentOutOfRangeException($"Argument \"{nameof(index)}\" = {index} is out of range"); + } + else if (index < --_count) + { + Array.Copy(_memoryBlock, index + 1, _memoryBlock, index, _count - index); } - _memoryBlock.CopyItems(index, oldCountMinus1, ref _memoryBlock); - _count--; + if (NeedsClearing) + { +#pragma warning disable CS8601 // In real use cases we'll never access it + new Span(_memoryBlock, _count, 1)[0] = default; +#pragma warning restore CS8601 + } } public IEnumerator GetEnumerator() => Enumerate(this).GetEnumerator(); diff --git a/Recyclable.Collections/RecyclableList.cs b/Recyclable.Collections/RecyclableList.cs index 217ea6c..a2e4420 100644 --- a/Recyclable.Collections/RecyclableList.cs +++ b/Recyclable.Collections/RecyclableList.cs @@ -49,42 +49,73 @@ namespace Recyclable.Collections public int NextItemBlockIndex => _nextItemBlockIndex; public int NextItemIndex => _nextItemIndex; - private static void RemoveAt(RecyclableList list, long index, ArrayPool blockArrayPool) + [MethodImpl(MethodImplOptions.AggressiveInlining | MethodImplOptions.AggressiveOptimization)] + private void DoRemoveAt(long index) { - long oldCountMinus1 = list._longCount - 1; - if (index != oldCountMinus1) + int blockSize = _blockSize; + int nextItemIndex = _nextItemIndex; + T[][] memoryBlocks = _memoryBlocks; + _longCount--; + if (index < _longCount) { - ThrowArgumentOutOfRangeException(); + int sourceBlockIndex = (int)((index + 1) >> _blockSizePow2Shift); + int sourceItemIndex = (int)((index + 1) & (blockSize - 1)); + + int targetBlockIndex = (int)(index >> _blockSizePow2Shift); + int targetItemIndex = (int)(index & (blockSize - 1)); + + int lastTakenBlockIndex = LastTakenBlockIndex; + while (sourceBlockIndex < lastTakenBlockIndex || (sourceBlockIndex == lastTakenBlockIndex && (sourceItemIndex < nextItemIndex || sourceBlockIndex != _nextItemBlockIndex))) + { + int toCopy = sourceBlockIndex < lastTakenBlockIndex || nextItemIndex == 0 + ? Math.Min(blockSize - sourceItemIndex, blockSize - targetItemIndex) + : Math.Min(nextItemIndex, blockSize - targetItemIndex); + + Array.Copy(memoryBlocks[sourceBlockIndex], sourceItemIndex, memoryBlocks[targetBlockIndex], targetItemIndex, toCopy); + // We didn't have enough room in the target array block. There are still items in the source array block to copy. + if (sourceItemIndex + toCopy < blockSize) + { + sourceItemIndex += toCopy; + targetBlockIndex++; + targetItemIndex = 0; + } + // We copied all the source items in the current array block. But have we filled the target? + else + { + sourceItemIndex = 0; + sourceBlockIndex++; + if (targetItemIndex + toCopy < blockSize) + { + targetItemIndex += toCopy; + } + else + { + targetBlockIndex++; + targetItemIndex = 0; + } + } + } } - list._longCount--; - int blockSize = list._blockSize; - if (list._nextItemIndex > 0) + if (nextItemIndex > 0) { - list._nextItemIndex--; + _nextItemIndex--; } else { - list._nextItemBlockIndex--; - list._nextItemIndex = blockSize; + _nextItemIndex = blockSize - 1; + _nextItemBlockIndex--; } - if (list._capacity - oldCountMinus1 == blockSize) + if (NeedsClearing) { - T[][] memoryBlocks = list._memoryBlocks; - if (blockSize >= RecyclableDefaults.MinPooledArrayLength) - { - blockArrayPool.Return(memoryBlocks[list.ReservedBlockCount - 1], NeedsClearing); - } - - list._capacity -= blockSize; +#pragma warning disable CS8601 // In real use cases we'll never access it + new Span(memoryBlocks[_nextItemBlockIndex], _nextItemIndex, 1)[0] = default; +#pragma warning restore CS8601 } } - private static void ThrowArgumentOutOfRangeException() - { - throw new ArgumentOutOfRangeException("index"); - } + private static void ThrowArgumentOutOfRangeException(in string message) => throw new ArgumentOutOfRangeException("index", message); protected void AddRangeEnumerated(IEnumerable source, int growByCount) { @@ -765,7 +796,7 @@ namespace Recyclable.Collections var itemIndex = LongIndexOf(item); if (itemIndex >= 0) { - RemoveAt(this, itemIndex, _defaultBlockArrayPool); + DoRemoveAt(itemIndex); return true; } @@ -794,8 +825,25 @@ namespace Recyclable.Collections } } - public void RemoveAt(int index) => RemoveAt(this, index, _blockArrayPool); - public void RemoveAt(long index) => RemoveAt(this, index, _blockArrayPool); + public void RemoveAt(int index) + { + if (index > _longCount - 1 || index < 0) + { + ThrowArgumentOutOfRangeException($"Argument \"{nameof(index)}\" = {index} is out of range. Expected value between 0 and {_longCount - 1}"); + } + + DoRemoveAt(index); + } + + public void RemoveAt(long index) + { + if (index > _longCount - 1 || index < 0) + { + ThrowArgumentOutOfRangeException($"Argument \"{nameof(index)}\" = {index} is out of range. Expected value between 0 and {_longCount - 1}"); + } + + DoRemoveAt(index); + } IEnumerator IEnumerable.GetEnumerator() => _memoryBlocks.Enumerate(_blockSize, LongCount).GetEnumerator(); diff --git a/Recyclable.CollectionsTests/RecyclableListTests.cs b/Recyclable.CollectionsTests/RecyclableListTests.cs index e229d6d..3f41035 100644 --- a/Recyclable.CollectionsTests/RecyclableListTests.cs +++ b/Recyclable.CollectionsTests/RecyclableListTests.cs @@ -545,34 +545,65 @@ namespace Recyclable.CollectionsTests [Theory] [MemberData(nameof(RecyclableListTestData.SourceTargetDataVariants), MemberType = typeof(RecyclableListTestData))] - public void RemoveAtShouldRaiseNotSupportedExceptionWhenNotLastItem(string testCase, IEnumerable testData, long itemsCount, int targetBlockSize) + public void RemoveAtFromTheBeginningShouldSucceed(string testCase, IEnumerable testData, long itemsCount, int targetBlockSize) { - if (itemsCount is 0 or 1) - { - return; - } - // Prepare using var list = new RecyclableList(testData, minBlockSize: targetBlockSize); - // Act - _ = Assert.Throws(() => list.RemoveAt(0)); + // Act & Validate + for (var deleted = 1; deleted <= itemsCount; deleted++) + { + // Act + list.RemoveAt(0); + + // Validate + _ = list.Count.Should().Be((int)(itemsCount - deleted)); + _ = list.Should().ContainInConsecutiveOrder(testData.Skip(deleted)); + } + + _ = list.Should().BeEmpty(); + _ = list.Capacity.Should().BeGreaterThanOrEqualTo((int)itemsCount); } [Theory] [MemberData(nameof(RecyclableListTestData.SourceTargetDataVariants), MemberType = typeof(RecyclableListTestData))] - public void RemoveShouldRaiseNotSupportedExceptionWhenNotLastItem(string testCase, IEnumerable testData, long itemsCount, int targetBlockSize) + public void RemoveAtFromTheEndShouldSucceed(string testCase, IEnumerable testData, long itemsCount, int targetBlockSize) { - if (itemsCount is 0 or 1) - { - return; - } - // Prepare using var list = new RecyclableList(testData, minBlockSize: targetBlockSize); - // Act - _ = Assert.Throws(() => list.Remove(testData.First())); + // Act & Validate + for (var deleted = 1; deleted <= itemsCount; deleted++) + { + // Act + list.RemoveAt(list.Count - 1); + + // Validate + _ = list.Count.Should().Be((int)(itemsCount - deleted)); + _ = list.Should().ContainInConsecutiveOrder(testData.Take((int)(itemsCount - deleted))); + } + + _ = list.Should().BeEmpty(); + _ = list.Capacity.Should().BeGreaterThanOrEqualTo((int)itemsCount); + } + + [Theory] + [MemberData(nameof(RecyclableListTestData.SourceTargetDataVariants), MemberType = typeof(RecyclableListTestData))] + public void RemoveFromTheBeginningShouldRemoveTheCorrectItem(string testCase, IEnumerable testData, long itemsCount, int targetBlockSize) + { + // Prepare + using var list = new RecyclableList(testData, minBlockSize: targetBlockSize); + + // Act & Validate + for (var deleted = 1; deleted <= itemsCount; deleted++) + { + // Act + _ = list.Remove(deleted).Should().BeTrue(); + + // Validate + _ = list.Count.Should().Be((int)(itemsCount - deleted)); + _ = list.Should().ContainInConsecutiveOrder(testData.Skip(deleted)); + } } [Theory]