From 9df90acd49ebae7cc7ba57ff1e54c71e93a68ba6 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Mon, 28 Oct 2024 20:46:14 -0700 Subject: [PATCH 01/15] Reduce memory and CPU costs due to SegmentedList usage Currently, the SegmentedList class suffers from two inefficiencies: 1) Upon growth, it doubles the SegmentedArray size. This is necessary for normal List like collections to get constant time amortized growth, but isn't necessary (or desirable) for segmented data structures. 2) Upon growth, it reallocates and copies over the existing pages. Instead, if we only allocate the modified/new pages and the array holding the pages, we can save significant CPU and allocation costs. --- .../Collections/SegmentedArray`1.cs | 33 +++++++++++++++++++ .../Collections/SegmentedList`1.cs | 15 +++++---- .../SegmentedListBenchmarks_InsertRange.cs | 14 +++++++- 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/Dependencies/Collections/SegmentedArray`1.cs b/src/Dependencies/Collections/SegmentedArray`1.cs index ae0eb89b32c9a..1da334fa3ddb3 100644 --- a/src/Dependencies/Collections/SegmentedArray`1.cs +++ b/src/Dependencies/Collections/SegmentedArray`1.cs @@ -94,6 +94,39 @@ public SegmentedArray(int length) } } + public SegmentedArray Resize(int newLength) + { + // For now, only allow growing resizes + if (newLength <= _length) + throw new ArgumentOutOfRangeException(nameof(newLength)); + + var newItems = new T[(newLength + SegmentSize - 1) >> SegmentShift][]; + var lastPageSize = newLength - ((newItems.Length - 1) << SegmentShift); + + // Copy over all old pages + for (var i = 0; i < _items.Length; i++) + newItems[i] = _items[i]; + + // If the previous last page is still the last page, resize it to lastPageSize. + // Otherwise, resize it to SegmentSize. + if (_items.Length > 0) + { + Array.Resize( + ref newItems[_items.Length - 1], + _items.Length == newItems.Length ? lastPageSize : SegmentSize); + } + + // Create all new pages (except the last one which is done separately) + for (var i = _items.Length; i < newItems.Length - 1; i++) + newItems[i] = new T[SegmentSize]; + + // Create a new last page if necessary + if (_items.Length < newItems.Length) + newItems[newItems.Length - 1] = new T[lastPageSize]; + + return new SegmentedArray(newLength, newItems); + } + private SegmentedArray(int length, T[][] items) { _length = length; diff --git a/src/Dependencies/Collections/SegmentedList`1.cs b/src/Dependencies/Collections/SegmentedList`1.cs index 4a5604cbefb39..2d3759ebb6b02 100644 --- a/src/Dependencies/Collections/SegmentedList`1.cs +++ b/src/Dependencies/Collections/SegmentedList`1.cs @@ -135,12 +135,7 @@ public int Capacity { if (value > 0) { - var newItems = new SegmentedArray(value); - if (_size > 0) - { - SegmentedArray.Copy(_items, newItems, _size); - } - _items = newItems; + _items = _items.Resize(value); } else { @@ -503,6 +498,14 @@ internal void Grow(int capacity) // Capacities exceeding Array.MaxLength will be surfaced as OutOfMemoryException by Array.Resize. if (newCapacity < capacity) newCapacity = capacity; + else + { + var segmentSize = SegmentedArrayHelper.GetSegmentSize(); + + // If caller didn't request a large capacity increase, limit the increase to a single page + if (newCapacity > segmentSize) + newCapacity = (((capacity - 1) / segmentSize) + 1) * segmentSize; + } Capacity = newCapacity; } diff --git a/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_InsertRange.cs b/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_InsertRange.cs index 61a202ba2ccd8..4ff71781dfc2c 100644 --- a/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_InsertRange.cs +++ b/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_InsertRange.cs @@ -8,6 +8,7 @@ namespace IdeCoreBenchmarks { + [MemoryDiagnoser] [DisassemblyDiagnoser] public class SegmentedListBenchmarks_InsertRange { @@ -23,7 +24,7 @@ public class SegmentedListBenchmarks_InsertRange private Microsoft.CodeAnalysis.Collections.SegmentedList _segmentedValuesObject = null!; private SegmentedArray _segmentedInsertValuesObject; - [Params(100000)] + [Params(1_000, 10_000, 100_000, 1_000_000)] public int Count { get; set; } [GlobalSetup] @@ -40,6 +41,17 @@ public void GlobalSetup() _segmentedInsertValuesObject = new SegmentedArray(100); } + [Benchmark(Description = "AddToSegmentedList", Baseline = true)] + public void AddList() + { + var array = new Microsoft.CodeAnalysis.Collections.SegmentedList(); + var iterations = Count; + for (var i = 0; i < iterations; i++) + { + array.Add(null); + } + } + [Benchmark(Description = "List", Baseline = true)] public void InsertRangeList() { From 8dd83f25d79b747c24015399b803cc419b8bf255 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 29 Oct 2024 09:12:10 -0700 Subject: [PATCH 02/15] Add comments, braces --- src/Dependencies/Collections/SegmentedArray`1.cs | 13 +++++++++++++ src/Dependencies/Collections/SegmentedList`1.cs | 2 ++ 2 files changed, 15 insertions(+) diff --git a/src/Dependencies/Collections/SegmentedArray`1.cs b/src/Dependencies/Collections/SegmentedArray`1.cs index 1da334fa3ddb3..ed0b407762119 100644 --- a/src/Dependencies/Collections/SegmentedArray`1.cs +++ b/src/Dependencies/Collections/SegmentedArray`1.cs @@ -94,6 +94,19 @@ public SegmentedArray(int length) } } + /// + /// Creates a new SegmentedArray of the specified size. All pages from this + /// SegmentedArray are copied into the new array. Note as this reuses the + /// pages, operations on the returned SegmentedArray and this SegmentedArray + /// will affect each other. + /// + /// The desired length of the returned SegmentedArray. + /// Note that this is currently limited to be larger than the current SegmentedArray's + /// length. + /// + /// The new SegmentedArray + /// Thrown when the desired length is not + /// greater than the current SegmentedArray's length. public SegmentedArray Resize(int newLength) { // For now, only allow growing resizes diff --git a/src/Dependencies/Collections/SegmentedList`1.cs b/src/Dependencies/Collections/SegmentedList`1.cs index 2d3759ebb6b02..bb7a4c03cf127 100644 --- a/src/Dependencies/Collections/SegmentedList`1.cs +++ b/src/Dependencies/Collections/SegmentedList`1.cs @@ -497,7 +497,9 @@ internal void Grow(int capacity) // If the computed capacity is still less than specified, set to the original argument. // Capacities exceeding Array.MaxLength will be surfaced as OutOfMemoryException by Array.Resize. if (newCapacity < capacity) + { newCapacity = capacity; + } else { var segmentSize = SegmentedArrayHelper.GetSegmentSize(); From a3f257868428cfb8fa945642966f0664fd2d72d2 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 29 Oct 2024 14:35:34 -0700 Subject: [PATCH 03/15] Add tests --- .../List/SegmentedList.Generic.Tests.cs | 83 ++++++++++++++++++ .../Collections/SegmentedArrayTests.cs | 86 +++++++++++++++++++ .../Collections/SegmentedArray`1.cs | 2 + 3 files changed, 171 insertions(+) diff --git a/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.cs b/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.cs index 1a05024b7c6ae..d9a3c992f99c8 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.cs @@ -73,5 +73,88 @@ public void CopyTo_ArgumentValidity(int count) Assert.Throws(null, () => list.CopyTo(0, new T[0], 0, count + 1)); Assert.Throws(null, () => list.CopyTo(count, new T[0], 0, 1)); } + + [Theory] + [MemberData(nameof(ValidCollectionSizes))] + public void Capacity_ArgumentValidity(int count) + { + var list = new SegmentedList(count); + for (var i = 0; i < count; i++) + list.Add(CreateT(i)); + + Assert.Throws(() => list.Capacity = count - 1); + } + + [Theory] + [InlineData(0, 0, 1)] + [InlineData(0, 0, 10)] + [InlineData(4, 4, 6)] + [InlineData(4, 4, 10)] + [InlineData(4, 4, 100_000)] + public void Capacity_MatchesSizeRequested(int initialCapacity, int initialSize, int requestedCapacity) + { + var list = new SegmentedList(initialCapacity); + + for (var i = 0; i < initialSize; i++) + list.Add(CreateT(i)); + + list.Capacity = requestedCapacity; + + Assert.Equal(requestedCapacity, list.Capacity); + } + + [Theory] + [InlineData(0, 0, 1, 4)] + [InlineData(0, 0, 10, 10)] + [InlineData(4, 4, 6, 8)] + [InlineData(4, 4, 10, 10)] + public void EnsureCapacity_ResizesAppropriately(int initialCapacity, int initialSize, int requestedCapacity, int expectedCapacity) + { + var list = new SegmentedList(initialCapacity); + + for (var i = 0; i < initialSize; i++) + list.Add(CreateT(i)); + + list.EnsureCapacity(requestedCapacity); + + Assert.Equal(expectedCapacity, list.Capacity); + } + + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(4)] + public void EnsureCapacity_GrowsBySegment(int segmentCount) + { + var elementCount = SegmentedArray.TestAccessor.SegmentSize * segmentCount; + var list = new SegmentedList(elementCount); + + for (var i = 0; i < elementCount; i++) + list.Add(CreateT(i)); + + Assert.Equal(elementCount, list.Capacity); + + list.EnsureCapacity(elementCount + 1); + Assert.Equal(elementCount + SegmentedArray.TestAccessor.SegmentSize, list.Capacity); + } + + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(4)] + public void EnsureCapacity_MatchesSizeWithLargeCapacityRequest(int segmentCount) + { + var elementCount = SegmentedArray.TestAccessor.SegmentSize * segmentCount; + var list = new SegmentedList(elementCount); + + for (var i = 0; i < elementCount; i++) + list.Add(CreateT(i)); + + Assert.Equal(elementCount, list.Capacity); + + var requestedCapacity = 2 * elementCount + 10; + list.EnsureCapacity(requestedCapacity); + Assert.Equal(requestedCapacity, list.Capacity); + } } } diff --git a/src/Compilers/Core/CodeAnalysisTest/Collections/SegmentedArrayTests.cs b/src/Compilers/Core/CodeAnalysisTest/Collections/SegmentedArrayTests.cs index 599103ee1950e..056a7666379a3 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Collections/SegmentedArrayTests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Collections/SegmentedArrayTests.cs @@ -29,6 +29,23 @@ public static IEnumerable TestLengths } } + public static IEnumerable TestLengthsAndSegmentCounts + { + get + { + for (var segmentsToAdd = 1; segmentsToAdd < 4; segmentsToAdd++) + { + yield return new object[] { 1, segmentsToAdd }; + yield return new object[] { 10, segmentsToAdd }; + yield return new object[] { 100, segmentsToAdd }; + yield return new object[] { SegmentedArray.TestAccessor.SegmentSize / 2, segmentsToAdd }; + yield return new object[] { SegmentedArray.TestAccessor.SegmentSize, segmentsToAdd }; + yield return new object[] { SegmentedArray.TestAccessor.SegmentSize * 2, segmentsToAdd }; + yield return new object[] { 100000, segmentsToAdd }; + } + } + } + private static void ResetToSequence(SegmentedArray array) { for (var i = 0; i < array.Length; i++) @@ -241,5 +258,74 @@ static void initialize(int[] array, SegmentedArray segmented) } } } + + [Theory] + [MemberData(nameof(TestLengths))] + public void Resize_ArgumentValidity(int length) + { + var o = new object(); + + var segmented = new SegmentedArray(length); + for (var i = 0; i < length; i++) + segmented[i] = o; + + Assert.Throws(() => segmented.Resize(length - 1)); + Assert.Throws(() => segmented.Resize(length)); + } + + [Theory] + [MemberData(nameof(TestLengthsAndSegmentCounts))] + public void Resize_ReusesSegments( + int length, + int addSegmentCount) + { + var elementCountToAdd = SegmentedArray.TestAccessor.SegmentSize * addSegmentCount; + var o = new object(); + + var oldSegmented = new SegmentedArray(length); + for (var i = 0; i < length; i++) + oldSegmented[i] = o; + + var oldSegments = SegmentedArray.TestAccessor.GetSegments(oldSegmented); + var oldSegmentCount = oldSegments.Length; + + var resizedSegmented = oldSegmented.Resize(length + elementCountToAdd); + var resizedSegments = SegmentedArray.TestAccessor.GetSegments(resizedSegmented); + var resizedSegmentCount = resizedSegments.Length; + + Assert.Equal(oldSegmentCount + addSegmentCount, resizedSegmentCount); + + for (var i = 0; i < oldSegmentCount - 1; i++) + Assert.Same(resizedSegments[i], oldSegments[i]); + + for (var i = oldSegmentCount - 1; i < resizedSegmentCount - 1; i++) + Assert.Equal(resizedSegments[i].Length, SegmentedArray.TestAccessor.SegmentSize); + + Assert.NotSame(resizedSegments[resizedSegmentCount - 1], oldSegments[oldSegmentCount - 1]); + Assert.Equal(resizedSegments[resizedSegmentCount - 1].Length, oldSegments[oldSegmentCount - 1].Length); + } + + [Theory] + [CombinatorialData] + public void Resize_InOnlySingleSegment( + [CombinatorialValues(1, 2, 10, 100)] int length, + [CombinatorialValues(1, 2, 10, 100)] int addItemCount) + { + var o = new object(); + + var oldSegmented = new SegmentedArray(length); + for (var i = 0; i < length; i++) + oldSegmented[i] = o; + + var oldSegments = SegmentedArray.TestAccessor.GetSegments(oldSegmented); + + var resizedSegmented = oldSegmented.Resize(length + addItemCount); + var resizedSegments = SegmentedArray.TestAccessor.GetSegments(resizedSegmented); + + Assert.Equal(1, oldSegments.Length); + Assert.Equal(1, resizedSegments.Length); + Assert.NotSame(resizedSegments[0], oldSegments[0]); + Assert.Equal(resizedSegments[0].Length, oldSegments[0].Length + addItemCount); + } } } diff --git a/src/Dependencies/Collections/SegmentedArray`1.cs b/src/Dependencies/Collections/SegmentedArray`1.cs index ed0b407762119..d9b975ab410d1 100644 --- a/src/Dependencies/Collections/SegmentedArray`1.cs +++ b/src/Dependencies/Collections/SegmentedArray`1.cs @@ -465,6 +465,8 @@ public void Reset() internal static class TestAccessor { public static int SegmentSize => SegmentedArray.SegmentSize; + public static T[][] GetSegments(SegmentedArray array) + => array._items; } } } From ccd5f74a9b31434fcbe1f08be8e5db3bad8fdb8c Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 29 Oct 2024 15:16:14 -0700 Subject: [PATCH 04/15] Use PrivateMarshal.AsSegments --- .../CodeAnalysisTest/Collections/SegmentedArrayTests.cs | 8 ++++---- src/Dependencies/Collections/SegmentedArray`1.cs | 2 -- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Compilers/Core/CodeAnalysisTest/Collections/SegmentedArrayTests.cs b/src/Compilers/Core/CodeAnalysisTest/Collections/SegmentedArrayTests.cs index 056a7666379a3..7b6494b8d63bf 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Collections/SegmentedArrayTests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Collections/SegmentedArrayTests.cs @@ -286,11 +286,11 @@ public void Resize_ReusesSegments( for (var i = 0; i < length; i++) oldSegmented[i] = o; - var oldSegments = SegmentedArray.TestAccessor.GetSegments(oldSegmented); + var oldSegments = SegmentedArray.PrivateMarshal.AsSegments(oldSegmented); var oldSegmentCount = oldSegments.Length; var resizedSegmented = oldSegmented.Resize(length + elementCountToAdd); - var resizedSegments = SegmentedArray.TestAccessor.GetSegments(resizedSegmented); + var resizedSegments = SegmentedArray.PrivateMarshal.AsSegments(resizedSegmented); var resizedSegmentCount = resizedSegments.Length; Assert.Equal(oldSegmentCount + addSegmentCount, resizedSegmentCount); @@ -317,10 +317,10 @@ public void Resize_InOnlySingleSegment( for (var i = 0; i < length; i++) oldSegmented[i] = o; - var oldSegments = SegmentedArray.TestAccessor.GetSegments(oldSegmented); + var oldSegments = SegmentedArray.PrivateMarshal.AsSegments(oldSegmented); var resizedSegmented = oldSegmented.Resize(length + addItemCount); - var resizedSegments = SegmentedArray.TestAccessor.GetSegments(resizedSegmented); + var resizedSegments = SegmentedArray.PrivateMarshal.AsSegments(resizedSegmented); Assert.Equal(1, oldSegments.Length); Assert.Equal(1, resizedSegments.Length); diff --git a/src/Dependencies/Collections/SegmentedArray`1.cs b/src/Dependencies/Collections/SegmentedArray`1.cs index d9b975ab410d1..ed0b407762119 100644 --- a/src/Dependencies/Collections/SegmentedArray`1.cs +++ b/src/Dependencies/Collections/SegmentedArray`1.cs @@ -465,8 +465,6 @@ public void Reset() internal static class TestAccessor { public static int SegmentSize => SegmentedArray.SegmentSize; - public static T[][] GetSegments(SegmentedArray array) - => array._items; } } } From 004dbb89207c95a7cd901b4d1286176520904484 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Wed, 30 Oct 2024 14:00:19 -0700 Subject: [PATCH 05/15] Move specialized optimization logic from SegmentedArray to SegmentedList usage point. --- .../SegmentedList.Generic.Tests.Capacity.cs | 175 ++++++++++++++++++ .../List/SegmentedList.Generic.Tests.cs | 83 --------- .../Collections/SegmentedArrayTests.cs | 86 --------- .../SegmentedArray`1+PrivateMarshal.cs | 14 ++ .../Collections/SegmentedArray`1.cs | 47 ----- .../SegmentedCollectionsMarshal.cs | 18 ++ .../Collections/SegmentedList`1.cs | 34 +++- .../SegmentedListBenchmarks_Add.cs | 31 ++++ .../SegmentedListBenchmarks_InsertRange.cs | 14 +- 9 files changed, 272 insertions(+), 230 deletions(-) create mode 100644 src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.Capacity.cs create mode 100644 src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_Add.cs diff --git a/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.Capacity.cs b/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.Capacity.cs new file mode 100644 index 0000000000000..93a629932ea7c --- /dev/null +++ b/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.Capacity.cs @@ -0,0 +1,175 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using Microsoft.CodeAnalysis.Collections; +using Xunit; + +namespace Microsoft.CodeAnalysis.UnitTests.Collections +{ + /// + /// Contains tests that ensure the correctness of the List class. + /// + public abstract partial class SegmentedList_Generic_Tests : IList_Generic_Tests + where T : notnull + { + public static IEnumerable TestLengthsAndSegmentCounts + { + get + { + for (var segmentsToAdd = 1; segmentsToAdd < 4; segmentsToAdd++) + { + yield return new object[] { 1, segmentsToAdd }; + yield return new object[] { 10, segmentsToAdd }; + yield return new object[] { 100, segmentsToAdd }; + yield return new object[] { SegmentedArray.TestAccessor.SegmentSize / 2, segmentsToAdd }; + yield return new object[] { SegmentedArray.TestAccessor.SegmentSize, segmentsToAdd }; + yield return new object[] { SegmentedArray.TestAccessor.SegmentSize * 2, segmentsToAdd }; + yield return new object[] { 100000, segmentsToAdd }; + } + } + } + + [Theory] + [MemberData(nameof(ValidCollectionSizes))] + public void Capacity_ArgumentValidity(int count) + { + var list = new SegmentedList(count); + for (var i = 0; i < count; i++) + list.Add(CreateT(i)); + + Assert.Throws(() => list.Capacity = count - 1); + } + + [Theory] + [InlineData(0, 0, 1)] + [InlineData(0, 0, 10)] + [InlineData(4, 4, 6)] + [InlineData(4, 4, 10)] + [InlineData(4, 4, 100_000)] + public void Capacity_MatchesSizeRequested(int initialCapacity, int initialSize, int requestedCapacity) + { + var list = new SegmentedList(initialCapacity); + + for (var i = 0; i < initialSize; i++) + list.Add(CreateT(i)); + + list.Capacity = requestedCapacity; + + Assert.Equal(requestedCapacity, list.Capacity); + } + + [Theory] + [MemberData(nameof(TestLengthsAndSegmentCounts))] + public void Capacity_ReusesSegments( + int length, + int addSegmentCount) + { + var elementCountToAdd = SegmentedArray.TestAccessor.SegmentSize * addSegmentCount; + var o = new object(); + + var segmented = new SegmentedList(length); + for (var i = 0; i < length; i++) + segmented.Add(o); + + var oldSegments = SegmentedCollectionsMarshal.AsSegments(segmented.GetTestAccessor().Items); + var oldSegmentCount = oldSegments.Length; + + segmented.Capacity = length + elementCountToAdd; + + var resizedSegments = SegmentedCollectionsMarshal.AsSegments(segmented.GetTestAccessor().Items); + var resizedSegmentCount = resizedSegments.Length; + + Assert.Equal(oldSegmentCount + addSegmentCount, resizedSegmentCount); + + for (var i = 0; i < oldSegmentCount - 1; i++) + Assert.Same(resizedSegments[i], oldSegments[i]); + + for (var i = oldSegmentCount - 1; i < resizedSegmentCount - 1; i++) + Assert.Equal(resizedSegments[i].Length, SegmentedArray.TestAccessor.SegmentSize); + + Assert.NotSame(resizedSegments[resizedSegmentCount - 1], oldSegments[oldSegmentCount - 1]); + Assert.Equal(resizedSegments[resizedSegmentCount - 1].Length, oldSegments[oldSegmentCount - 1].Length); + } + + [Theory] + [CombinatorialData] + public void Capacity_InOnlySingleSegment( + [CombinatorialValues(1, 2, 10, 100)] int length, + [CombinatorialValues(1, 2, 10, 100)] int addItemCount) + { + var o = new object(); + + var segmented = new SegmentedList(length); + for (var i = 0; i < length; i++) + segmented.Add(o); + + var oldSegments = SegmentedCollectionsMarshal.AsSegments(segmented.GetTestAccessor().Items); + + segmented.Capacity = length + addItemCount; + + var resizedSegments = SegmentedCollectionsMarshal.AsSegments(segmented.GetTestAccessor().Items); + + Assert.Equal(1, oldSegments.Length); + Assert.Equal(1, resizedSegments.Length); + Assert.Same(resizedSegments[0], oldSegments[0]); + Assert.Equal(segmented.Capacity, resizedSegments[0].Length); + } + + [Theory] + [InlineData(0, 0, 1, 4)] + [InlineData(0, 0, 10, 10)] + [InlineData(4, 4, 6, 8)] + [InlineData(4, 4, 10, 10)] + public void EnsureCapacity_ResizesAppropriately(int initialCapacity, int initialSize, int requestedCapacity, int expectedCapacity) + { + var list = new SegmentedList(initialCapacity); + + for (var i = 0; i < initialSize; i++) + list.Add(CreateT(i)); + + list.EnsureCapacity(requestedCapacity); + + Assert.Equal(expectedCapacity, list.Capacity); + } + + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(4)] + public void EnsureCapacity_GrowsBySegment(int segmentCount) + { + var elementCount = SegmentedArray.TestAccessor.SegmentSize * segmentCount; + var list = new SegmentedList(elementCount); + + for (var i = 0; i < elementCount; i++) + list.Add(CreateT(i)); + + Assert.Equal(elementCount, list.Capacity); + + list.EnsureCapacity(elementCount + 1); + Assert.Equal(elementCount + SegmentedArray.TestAccessor.SegmentSize, list.Capacity); + } + + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(4)] + public void EnsureCapacity_MatchesSizeWithLargeCapacityRequest(int segmentCount) + { + var elementCount = SegmentedArray.TestAccessor.SegmentSize * segmentCount; + var list = new SegmentedList(elementCount); + + for (var i = 0; i < elementCount; i++) + list.Add(CreateT(i)); + + Assert.Equal(elementCount, list.Capacity); + + var requestedCapacity = 2 * elementCount + 10; + list.EnsureCapacity(requestedCapacity); + Assert.Equal(requestedCapacity, list.Capacity); + } + } +} diff --git a/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.cs b/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.cs index d9a3c992f99c8..1a05024b7c6ae 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.cs @@ -73,88 +73,5 @@ public void CopyTo_ArgumentValidity(int count) Assert.Throws(null, () => list.CopyTo(0, new T[0], 0, count + 1)); Assert.Throws(null, () => list.CopyTo(count, new T[0], 0, 1)); } - - [Theory] - [MemberData(nameof(ValidCollectionSizes))] - public void Capacity_ArgumentValidity(int count) - { - var list = new SegmentedList(count); - for (var i = 0; i < count; i++) - list.Add(CreateT(i)); - - Assert.Throws(() => list.Capacity = count - 1); - } - - [Theory] - [InlineData(0, 0, 1)] - [InlineData(0, 0, 10)] - [InlineData(4, 4, 6)] - [InlineData(4, 4, 10)] - [InlineData(4, 4, 100_000)] - public void Capacity_MatchesSizeRequested(int initialCapacity, int initialSize, int requestedCapacity) - { - var list = new SegmentedList(initialCapacity); - - for (var i = 0; i < initialSize; i++) - list.Add(CreateT(i)); - - list.Capacity = requestedCapacity; - - Assert.Equal(requestedCapacity, list.Capacity); - } - - [Theory] - [InlineData(0, 0, 1, 4)] - [InlineData(0, 0, 10, 10)] - [InlineData(4, 4, 6, 8)] - [InlineData(4, 4, 10, 10)] - public void EnsureCapacity_ResizesAppropriately(int initialCapacity, int initialSize, int requestedCapacity, int expectedCapacity) - { - var list = new SegmentedList(initialCapacity); - - for (var i = 0; i < initialSize; i++) - list.Add(CreateT(i)); - - list.EnsureCapacity(requestedCapacity); - - Assert.Equal(expectedCapacity, list.Capacity); - } - - [Theory] - [InlineData(1)] - [InlineData(2)] - [InlineData(4)] - public void EnsureCapacity_GrowsBySegment(int segmentCount) - { - var elementCount = SegmentedArray.TestAccessor.SegmentSize * segmentCount; - var list = new SegmentedList(elementCount); - - for (var i = 0; i < elementCount; i++) - list.Add(CreateT(i)); - - Assert.Equal(elementCount, list.Capacity); - - list.EnsureCapacity(elementCount + 1); - Assert.Equal(elementCount + SegmentedArray.TestAccessor.SegmentSize, list.Capacity); - } - - [Theory] - [InlineData(1)] - [InlineData(2)] - [InlineData(4)] - public void EnsureCapacity_MatchesSizeWithLargeCapacityRequest(int segmentCount) - { - var elementCount = SegmentedArray.TestAccessor.SegmentSize * segmentCount; - var list = new SegmentedList(elementCount); - - for (var i = 0; i < elementCount; i++) - list.Add(CreateT(i)); - - Assert.Equal(elementCount, list.Capacity); - - var requestedCapacity = 2 * elementCount + 10; - list.EnsureCapacity(requestedCapacity); - Assert.Equal(requestedCapacity, list.Capacity); - } } } diff --git a/src/Compilers/Core/CodeAnalysisTest/Collections/SegmentedArrayTests.cs b/src/Compilers/Core/CodeAnalysisTest/Collections/SegmentedArrayTests.cs index 7b6494b8d63bf..599103ee1950e 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Collections/SegmentedArrayTests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Collections/SegmentedArrayTests.cs @@ -29,23 +29,6 @@ public static IEnumerable TestLengths } } - public static IEnumerable TestLengthsAndSegmentCounts - { - get - { - for (var segmentsToAdd = 1; segmentsToAdd < 4; segmentsToAdd++) - { - yield return new object[] { 1, segmentsToAdd }; - yield return new object[] { 10, segmentsToAdd }; - yield return new object[] { 100, segmentsToAdd }; - yield return new object[] { SegmentedArray.TestAccessor.SegmentSize / 2, segmentsToAdd }; - yield return new object[] { SegmentedArray.TestAccessor.SegmentSize, segmentsToAdd }; - yield return new object[] { SegmentedArray.TestAccessor.SegmentSize * 2, segmentsToAdd }; - yield return new object[] { 100000, segmentsToAdd }; - } - } - } - private static void ResetToSequence(SegmentedArray array) { for (var i = 0; i < array.Length; i++) @@ -258,74 +241,5 @@ static void initialize(int[] array, SegmentedArray segmented) } } } - - [Theory] - [MemberData(nameof(TestLengths))] - public void Resize_ArgumentValidity(int length) - { - var o = new object(); - - var segmented = new SegmentedArray(length); - for (var i = 0; i < length; i++) - segmented[i] = o; - - Assert.Throws(() => segmented.Resize(length - 1)); - Assert.Throws(() => segmented.Resize(length)); - } - - [Theory] - [MemberData(nameof(TestLengthsAndSegmentCounts))] - public void Resize_ReusesSegments( - int length, - int addSegmentCount) - { - var elementCountToAdd = SegmentedArray.TestAccessor.SegmentSize * addSegmentCount; - var o = new object(); - - var oldSegmented = new SegmentedArray(length); - for (var i = 0; i < length; i++) - oldSegmented[i] = o; - - var oldSegments = SegmentedArray.PrivateMarshal.AsSegments(oldSegmented); - var oldSegmentCount = oldSegments.Length; - - var resizedSegmented = oldSegmented.Resize(length + elementCountToAdd); - var resizedSegments = SegmentedArray.PrivateMarshal.AsSegments(resizedSegmented); - var resizedSegmentCount = resizedSegments.Length; - - Assert.Equal(oldSegmentCount + addSegmentCount, resizedSegmentCount); - - for (var i = 0; i < oldSegmentCount - 1; i++) - Assert.Same(resizedSegments[i], oldSegments[i]); - - for (var i = oldSegmentCount - 1; i < resizedSegmentCount - 1; i++) - Assert.Equal(resizedSegments[i].Length, SegmentedArray.TestAccessor.SegmentSize); - - Assert.NotSame(resizedSegments[resizedSegmentCount - 1], oldSegments[oldSegmentCount - 1]); - Assert.Equal(resizedSegments[resizedSegmentCount - 1].Length, oldSegments[oldSegmentCount - 1].Length); - } - - [Theory] - [CombinatorialData] - public void Resize_InOnlySingleSegment( - [CombinatorialValues(1, 2, 10, 100)] int length, - [CombinatorialValues(1, 2, 10, 100)] int addItemCount) - { - var o = new object(); - - var oldSegmented = new SegmentedArray(length); - for (var i = 0; i < length; i++) - oldSegmented[i] = o; - - var oldSegments = SegmentedArray.PrivateMarshal.AsSegments(oldSegmented); - - var resizedSegmented = oldSegmented.Resize(length + addItemCount); - var resizedSegments = SegmentedArray.PrivateMarshal.AsSegments(resizedSegmented); - - Assert.Equal(1, oldSegments.Length); - Assert.Equal(1, resizedSegments.Length); - Assert.NotSame(resizedSegments[0], oldSegments[0]); - Assert.Equal(resizedSegments[0].Length, oldSegments[0].Length + addItemCount); - } } } diff --git a/src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs b/src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs index d4ad3359886dc..f1ea6345288f3 100644 --- a/src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs +++ b/src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; + namespace Microsoft.CodeAnalysis.Collections; internal readonly partial struct SegmentedArray @@ -14,5 +16,17 @@ internal static class PrivateMarshal /// public static T[][] AsSegments(SegmentedArray array) => array._items; + + public static SegmentedArray AsSegmentedArray(T[][] segments) + { + if (segments is null) + throw new ArgumentNullException(nameof(segments)); + + var length = 0; + foreach (var segment in segments) + length += segment.Length; + + return new SegmentedArray(length, segments); + } } } diff --git a/src/Dependencies/Collections/SegmentedArray`1.cs b/src/Dependencies/Collections/SegmentedArray`1.cs index ed0b407762119..37d05837cbdde 100644 --- a/src/Dependencies/Collections/SegmentedArray`1.cs +++ b/src/Dependencies/Collections/SegmentedArray`1.cs @@ -93,53 +93,6 @@ public SegmentedArray(int length) _length = length; } } - - /// - /// Creates a new SegmentedArray of the specified size. All pages from this - /// SegmentedArray are copied into the new array. Note as this reuses the - /// pages, operations on the returned SegmentedArray and this SegmentedArray - /// will affect each other. - /// - /// The desired length of the returned SegmentedArray. - /// Note that this is currently limited to be larger than the current SegmentedArray's - /// length. - /// - /// The new SegmentedArray - /// Thrown when the desired length is not - /// greater than the current SegmentedArray's length. - public SegmentedArray Resize(int newLength) - { - // For now, only allow growing resizes - if (newLength <= _length) - throw new ArgumentOutOfRangeException(nameof(newLength)); - - var newItems = new T[(newLength + SegmentSize - 1) >> SegmentShift][]; - var lastPageSize = newLength - ((newItems.Length - 1) << SegmentShift); - - // Copy over all old pages - for (var i = 0; i < _items.Length; i++) - newItems[i] = _items[i]; - - // If the previous last page is still the last page, resize it to lastPageSize. - // Otherwise, resize it to SegmentSize. - if (_items.Length > 0) - { - Array.Resize( - ref newItems[_items.Length - 1], - _items.Length == newItems.Length ? lastPageSize : SegmentSize); - } - - // Create all new pages (except the last one which is done separately) - for (var i = _items.Length; i < newItems.Length - 1; i++) - newItems[i] = new T[SegmentSize]; - - // Create a new last page if necessary - if (_items.Length < newItems.Length) - newItems[newItems.Length - 1] = new T[lastPageSize]; - - return new SegmentedArray(newLength, newItems); - } - private SegmentedArray(int length, T[][] items) { _length = length; diff --git a/src/Dependencies/Collections/SegmentedCollectionsMarshal.cs b/src/Dependencies/Collections/SegmentedCollectionsMarshal.cs index cb4c90b3f8c53..2e21b2a1a6f97 100644 --- a/src/Dependencies/Collections/SegmentedCollectionsMarshal.cs +++ b/src/Dependencies/Collections/SegmentedCollectionsMarshal.cs @@ -22,6 +22,24 @@ internal static class SegmentedCollectionsMarshal public static T[][] AsSegments(SegmentedArray array) => SegmentedArray.PrivateMarshal.AsSegments(array); + /// + /// Gets a value wrapping the input T[][]. + /// + /// The type of elements in the input. + /// The input array to wrap in the returned value. + /// A value wrapping . + /// + /// + /// When using this method, callers should take extra care to ensure that they're the sole owners of the input + /// array, and that it won't be modified once the returned value starts + /// being used. Doing so might cause undefined behavior in code paths which don't expect the contents of a given + /// values to change outside their control. + /// + /// + /// Thrown when is + public static SegmentedArray AsSegmentedArray(T[][] segments) + => SegmentedArray.PrivateMarshal.AsSegmentedArray(segments); + /// /// Gets either a ref to a in the or a /// ref null if it does not exist in the . diff --git a/src/Dependencies/Collections/SegmentedList`1.cs b/src/Dependencies/Collections/SegmentedList`1.cs index bb7a4c03cf127..3d4b9f5086a5c 100644 --- a/src/Dependencies/Collections/SegmentedList`1.cs +++ b/src/Dependencies/Collections/SegmentedList`1.cs @@ -135,7 +135,39 @@ public int Capacity { if (value > 0) { - _items = _items.Resize(value); + // Rather than creating a copy of _items, instead reuse as much of it's data as possible. + // This saves as much as 50% of allocations and 70% of CPU cost of Add in large collections. + // See SegmentedListBenchmarks_Add for repro details. + var segments = SegmentedCollectionsMarshal.AsSegments(_items); + + var segmentSize = SegmentedArrayHelper.GetSegmentSize(); + var segmentShift = SegmentedArrayHelper.GetSegmentShift(); + var oldSegmentCount = segments.Length; + var newSegmentCount = (value + segmentSize - 1) >> segmentShift; + + // Grow the array of segments, if necessary + Array.Resize(ref segments, newSegmentCount); + + var lastPageSize = value - ((newSegmentCount - 1) << segmentShift); + + // If the previous last page is still the last page, resize it to lastPageSize. + // Otherwise, resize it to SegmentSize. + if (oldSegmentCount > 0) + { + Array.Resize( + ref segments[oldSegmentCount - 1], + oldSegmentCount == newSegmentCount ? lastPageSize : segmentSize); + } + + // Create all new pages (except the last one which is done separately) + for (var i = oldSegmentCount; i < newSegmentCount - 1; i++) + segments[i] = new T[segmentSize]; + + // Create a new last page if necessary + if (oldSegmentCount < newSegmentCount) + segments[newSegmentCount - 1] = new T[lastPageSize]; + + _items = SegmentedCollectionsMarshal.AsSegmentedArray(segments); } else { diff --git a/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_Add.cs b/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_Add.cs new file mode 100644 index 0000000000000..1221ed8928096 --- /dev/null +++ b/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_Add.cs @@ -0,0 +1,31 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using BenchmarkDotNet.Attributes; + +namespace IdeCoreBenchmarks +{ + [MemoryDiagnoser] + public class SegmentedListBenchmarks_Add + { + [Params(1_000, 10_000, 100_000, 1_000_000)] + public int Count { get; set; } + + [GlobalSetup] + public void GlobalSetup() + { + } + + [Benchmark(Description = "AddToSegmentedList", Baseline = true)] + public void AddList() + { + var array = new Microsoft.CodeAnalysis.Collections.SegmentedList(); + var iterations = Count; + for (var i = 0; i < iterations; i++) + { + array.Add(null); + } + } + } +} diff --git a/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_InsertRange.cs b/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_InsertRange.cs index 4ff71781dfc2c..61a202ba2ccd8 100644 --- a/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_InsertRange.cs +++ b/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_InsertRange.cs @@ -8,7 +8,6 @@ namespace IdeCoreBenchmarks { - [MemoryDiagnoser] [DisassemblyDiagnoser] public class SegmentedListBenchmarks_InsertRange { @@ -24,7 +23,7 @@ public class SegmentedListBenchmarks_InsertRange private Microsoft.CodeAnalysis.Collections.SegmentedList _segmentedValuesObject = null!; private SegmentedArray _segmentedInsertValuesObject; - [Params(1_000, 10_000, 100_000, 1_000_000)] + [Params(100000)] public int Count { get; set; } [GlobalSetup] @@ -41,17 +40,6 @@ public void GlobalSetup() _segmentedInsertValuesObject = new SegmentedArray(100); } - [Benchmark(Description = "AddToSegmentedList", Baseline = true)] - public void AddList() - { - var array = new Microsoft.CodeAnalysis.Collections.SegmentedList(); - var iterations = Count; - for (var i = 0; i < iterations; i++) - { - array.Add(null); - } - } - [Benchmark(Description = "List", Baseline = true)] public void InsertRangeList() { From 492eb1c0145cdd6d6e5995b14b30cf1a5e633a3f Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Wed, 30 Oct 2024 14:02:06 -0700 Subject: [PATCH 06/15] Fix formatting --- src/Dependencies/Collections/SegmentedArray`1.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Dependencies/Collections/SegmentedArray`1.cs b/src/Dependencies/Collections/SegmentedArray`1.cs index 37d05837cbdde..ae0eb89b32c9a 100644 --- a/src/Dependencies/Collections/SegmentedArray`1.cs +++ b/src/Dependencies/Collections/SegmentedArray`1.cs @@ -93,6 +93,7 @@ public SegmentedArray(int length) _length = length; } } + private SegmentedArray(int length, T[][] items) { _length = length; From 55649b51bb90f9214e828682d700738fbfee60bb Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Thu, 31 Oct 2024 09:11:25 -0700 Subject: [PATCH 07/15] Separate out the page reuse part of this PR and just focus this PR on that. --- .../SegmentedList.Generic.Tests.Capacity.cs | 18 --------- .../Collections/SegmentedList`1.cs | 40 ++++--------------- 2 files changed, 8 insertions(+), 50 deletions(-) diff --git a/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.Capacity.cs b/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.Capacity.cs index 93a629932ea7c..b4d5bc70e7d6f 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.Capacity.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.Capacity.cs @@ -135,24 +135,6 @@ public void EnsureCapacity_ResizesAppropriately(int initialCapacity, int initial Assert.Equal(expectedCapacity, list.Capacity); } - [Theory] - [InlineData(1)] - [InlineData(2)] - [InlineData(4)] - public void EnsureCapacity_GrowsBySegment(int segmentCount) - { - var elementCount = SegmentedArray.TestAccessor.SegmentSize * segmentCount; - var list = new SegmentedList(elementCount); - - for (var i = 0; i < elementCount; i++) - list.Add(CreateT(i)); - - Assert.Equal(elementCount, list.Capacity); - - list.EnsureCapacity(elementCount + 1); - Assert.Equal(elementCount + SegmentedArray.TestAccessor.SegmentSize, list.Capacity); - } - [Theory] [InlineData(1)] [InlineData(2)] diff --git a/src/Dependencies/Collections/SegmentedList`1.cs b/src/Dependencies/Collections/SegmentedList`1.cs index 3d4b9f5086a5c..4a8874a07bae6 100644 --- a/src/Dependencies/Collections/SegmentedList`1.cs +++ b/src/Dependencies/Collections/SegmentedList`1.cs @@ -136,36 +136,22 @@ public int Capacity if (value > 0) { // Rather than creating a copy of _items, instead reuse as much of it's data as possible. - // This saves as much as 50% of allocations and 70% of CPU cost of Add in large collections. - // See SegmentedListBenchmarks_Add for repro details. var segments = SegmentedCollectionsMarshal.AsSegments(_items); - var segmentSize = SegmentedArrayHelper.GetSegmentSize(); - var segmentShift = SegmentedArrayHelper.GetSegmentShift(); var oldSegmentCount = segments.Length; - var newSegmentCount = (value + segmentSize - 1) >> segmentShift; + var newSegmentCount = (value + SegmentedArrayHelper.GetSegmentSize() - 1) >> SegmentedArrayHelper.GetSegmentShift(); // Grow the array of segments, if necessary Array.Resize(ref segments, newSegmentCount); - var lastPageSize = value - ((newSegmentCount - 1) << segmentShift); + // Resize all segments to full segment size from the last old segment to the next to last + // new segment. + for (var i = oldSegmentCount > 0 ? oldSegmentCount - 1 : 0; i < newSegmentCount - 1; i++) + Array.Resize(ref segments[i], SegmentedArrayHelper.GetSegmentSize()); - // If the previous last page is still the last page, resize it to lastPageSize. - // Otherwise, resize it to SegmentSize. - if (oldSegmentCount > 0) - { - Array.Resize( - ref segments[oldSegmentCount - 1], - oldSegmentCount == newSegmentCount ? lastPageSize : segmentSize); - } - - // Create all new pages (except the last one which is done separately) - for (var i = oldSegmentCount; i < newSegmentCount - 1; i++) - segments[i] = new T[segmentSize]; - - // Create a new last page if necessary - if (oldSegmentCount < newSegmentCount) - segments[newSegmentCount - 1] = new T[lastPageSize]; + // Resize the last segment + var lastSegmentSize = value - ((newSegmentCount - 1) << SegmentedArrayHelper.GetSegmentShift()); + Array.Resize(ref segments[newSegmentCount - 1], lastSegmentSize); _items = SegmentedCollectionsMarshal.AsSegmentedArray(segments); } @@ -529,17 +515,7 @@ internal void Grow(int capacity) // If the computed capacity is still less than specified, set to the original argument. // Capacities exceeding Array.MaxLength will be surfaced as OutOfMemoryException by Array.Resize. if (newCapacity < capacity) - { newCapacity = capacity; - } - else - { - var segmentSize = SegmentedArrayHelper.GetSegmentSize(); - - // If caller didn't request a large capacity increase, limit the increase to a single page - if (newCapacity > segmentSize) - newCapacity = (((capacity - 1) / segmentSize) + 1) * segmentSize; - } Capacity = newCapacity; } From 542ad7f5963ac378f6d22e91c051a04353e87d94 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Thu, 31 Oct 2024 09:49:24 -0700 Subject: [PATCH 08/15] Separate out empty case as it was a bit tricky to see. Also reduced nesting. --- .../Collections/SegmentedList`1.cs | 51 +++++++++++-------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/src/Dependencies/Collections/SegmentedList`1.cs b/src/Dependencies/Collections/SegmentedList`1.cs index 4a8874a07bae6..bc782e631b392 100644 --- a/src/Dependencies/Collections/SegmentedList`1.cs +++ b/src/Dependencies/Collections/SegmentedList`1.cs @@ -131,34 +131,41 @@ public int Capacity ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.value, ExceptionResource.ArgumentOutOfRange_SmallCapacity); } - if (value != _items.Length) + if (value == _items.Length) + return; + + if (value <= 0) { - if (value > 0) - { - // Rather than creating a copy of _items, instead reuse as much of it's data as possible. - var segments = SegmentedCollectionsMarshal.AsSegments(_items); + _items = s_emptyArray; + return; + } - var oldSegmentCount = segments.Length; - var newSegmentCount = (value + SegmentedArrayHelper.GetSegmentSize() - 1) >> SegmentedArrayHelper.GetSegmentShift(); + if (_items.Length == 0) + { + // No data from existing array to reuse, just create a new one. + _items = new SegmentedArray(value); + } + else + { + // Rather than creating a copy of _items, instead reuse as much of it's data as possible. + var segments = SegmentedCollectionsMarshal.AsSegments(_items); - // Grow the array of segments, if necessary - Array.Resize(ref segments, newSegmentCount); + var oldSegmentCount = segments.Length; + var newSegmentCount = (value + SegmentedArrayHelper.GetSegmentSize() - 1) >> SegmentedArrayHelper.GetSegmentShift(); - // Resize all segments to full segment size from the last old segment to the next to last - // new segment. - for (var i = oldSegmentCount > 0 ? oldSegmentCount - 1 : 0; i < newSegmentCount - 1; i++) - Array.Resize(ref segments[i], SegmentedArrayHelper.GetSegmentSize()); + // Grow the array of segments, if necessary + Array.Resize(ref segments, newSegmentCount); - // Resize the last segment - var lastSegmentSize = value - ((newSegmentCount - 1) << SegmentedArrayHelper.GetSegmentShift()); - Array.Resize(ref segments[newSegmentCount - 1], lastSegmentSize); + // Resize all segments to full segment size from the last old segment to the next to last + // new segment. + for (var i = oldSegmentCount - 1; i < newSegmentCount - 1; i++) + Array.Resize(ref segments[i], SegmentedArrayHelper.GetSegmentSize()); - _items = SegmentedCollectionsMarshal.AsSegmentedArray(segments); - } - else - { - _items = s_emptyArray; - } + // Resize the last segment + var lastSegmentSize = value - ((newSegmentCount - 1) << SegmentedArrayHelper.GetSegmentShift()); + Array.Resize(ref segments[newSegmentCount - 1], lastSegmentSize); + + _items = SegmentedCollectionsMarshal.AsSegmentedArray(segments); } } } From 70a40b76d9dedbea735ec82970c2797e8c50c261 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Thu, 31 Oct 2024 10:15:09 -0700 Subject: [PATCH 09/15] Simplify calculation of segmented array length --- .../Collections/SegmentedArray`1+PrivateMarshal.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs b/src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs index f1ea6345288f3..1cdeabb8d0f51 100644 --- a/src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs +++ b/src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs @@ -2,7 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; +using Microsoft.CodeAnalysis.Collections.Internal; namespace Microsoft.CodeAnalysis.Collections; @@ -19,12 +19,9 @@ public static T[][] AsSegments(SegmentedArray array) public static SegmentedArray AsSegmentedArray(T[][] segments) { - if (segments is null) - throw new ArgumentNullException(nameof(segments)); - - var length = 0; - foreach (var segment in segments) - length += segment.Length; + var length = segments.Length == 0 ? 0 : segments[segments.Length - 1].Length; + if (segments.Length > 1) + length = length + SegmentedArrayHelper.GetSegmentSize() * (segments.Length - 1); return new SegmentedArray(length, segments); } From f3ab719dd04a659056c6a699501d6a1fc5acd14b Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Thu, 31 Oct 2024 10:39:08 -0700 Subject: [PATCH 10/15] Change segmented array marshalling methods to take in the array to remove need for a computation of the length --- .../Collections/SegmentedArray`1+PrivateMarshal.cs | 10 ++-------- .../Collections/SegmentedCollectionsMarshal.cs | 5 +++-- src/Dependencies/Collections/SegmentedList`1.cs | 2 +- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs b/src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs index 1cdeabb8d0f51..76b224a4645c7 100644 --- a/src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs +++ b/src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs @@ -17,13 +17,7 @@ internal static class PrivateMarshal public static T[][] AsSegments(SegmentedArray array) => array._items; - public static SegmentedArray AsSegmentedArray(T[][] segments) - { - var length = segments.Length == 0 ? 0 : segments[segments.Length - 1].Length; - if (segments.Length > 1) - length = length + SegmentedArrayHelper.GetSegmentSize() * (segments.Length - 1); - - return new SegmentedArray(length, segments); - } + public static SegmentedArray AsSegmentedArray(int length, T[][] segments) + => new SegmentedArray(length, segments); } } diff --git a/src/Dependencies/Collections/SegmentedCollectionsMarshal.cs b/src/Dependencies/Collections/SegmentedCollectionsMarshal.cs index 2e21b2a1a6f97..d8a9ebeef1ab6 100644 --- a/src/Dependencies/Collections/SegmentedCollectionsMarshal.cs +++ b/src/Dependencies/Collections/SegmentedCollectionsMarshal.cs @@ -26,6 +26,7 @@ public static T[][] AsSegments(SegmentedArray array) /// Gets a value wrapping the input T[][]. /// /// The type of elements in the input. + /// The combined length of the input arrays /// The input array to wrap in the returned value. /// A value wrapping . /// @@ -37,8 +38,8 @@ public static T[][] AsSegments(SegmentedArray array) /// /// /// Thrown when is - public static SegmentedArray AsSegmentedArray(T[][] segments) - => SegmentedArray.PrivateMarshal.AsSegmentedArray(segments); + public static SegmentedArray AsSegmentedArray(int length, T[][] segments) + => SegmentedArray.PrivateMarshal.AsSegmentedArray(length, segments); /// /// Gets either a ref to a in the or a diff --git a/src/Dependencies/Collections/SegmentedList`1.cs b/src/Dependencies/Collections/SegmentedList`1.cs index bc782e631b392..5a7ffb9202924 100644 --- a/src/Dependencies/Collections/SegmentedList`1.cs +++ b/src/Dependencies/Collections/SegmentedList`1.cs @@ -165,7 +165,7 @@ public int Capacity var lastSegmentSize = value - ((newSegmentCount - 1) << SegmentedArrayHelper.GetSegmentShift()); Array.Resize(ref segments[newSegmentCount - 1], lastSegmentSize); - _items = SegmentedCollectionsMarshal.AsSegmentedArray(segments); + _items = SegmentedCollectionsMarshal.AsSegmentedArray(value, segments); } } } From 33d866f4ebd66e4db769bc748b9a57a89391b9df Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Thu, 31 Oct 2024 11:03:56 -0700 Subject: [PATCH 11/15] Remove unused using after change --- src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs b/src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs index 76b224a4645c7..20433a3451bda 100644 --- a/src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs +++ b/src/Dependencies/Collections/SegmentedArray`1+PrivateMarshal.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using Microsoft.CodeAnalysis.Collections.Internal; - namespace Microsoft.CodeAnalysis.Collections; internal readonly partial struct SegmentedArray From 2d4396655e206c4a81ca8128ec435575c8292811 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Thu, 31 Oct 2024 16:41:17 -0700 Subject: [PATCH 12/15] Add more types representing a variety of sizes to benchmark --- .../SegmentedListBenchmarks_Add.cs | 81 ++++++++++++++++++- 1 file changed, 79 insertions(+), 2 deletions(-) diff --git a/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_Add.cs b/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_Add.cs index 1221ed8928096..5738777e90895 100644 --- a/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_Add.cs +++ b/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_Add.cs @@ -17,15 +17,92 @@ public void GlobalSetup() { } - [Benchmark(Description = "AddToSegmentedList", Baseline = true)] - public void AddList() + [Benchmark(Description = "AddIntToList")] + public void AddIntToList() + { + var array = new Microsoft.CodeAnalysis.Collections.SegmentedList(); + var iterations = Count; + + for (var i = 0; i < iterations; i++) + { + array.Add(i); + } + } + + [Benchmark(Description = "AddObjectToList")] + public void AddObjectToList() { var array = new Microsoft.CodeAnalysis.Collections.SegmentedList(); var iterations = Count; + for (var i = 0; i < iterations; i++) { array.Add(null); } } + + [Benchmark(Description = "AddLargeStructToList")] + public void AddLargeStructToList() + { + var array = new Microsoft.CodeAnalysis.Collections.SegmentedList(); + var item = new LargeStruct(); + var iterations = Count; + + for (var i = 0; i < iterations; i++) + { + array.Add(item); + } + } + + [Benchmark(Description = "AddEnormousStructToList")] + public void AddEnormousStructToList() + { + var array = new Microsoft.CodeAnalysis.Collections.SegmentedList(); + var item = new EnormousStruct(); + var iterations = Count; + + for (var i = 0; i < iterations; i++) + { + array.Add(item); + } + } + + private struct LargeStruct + { + public int i1; + public int i2; + public int i3; + public int i4; + public int i5; + public int i6; + public int i7; + public int i8; + public int i9; + public int i10; + public int i11; + public int i12; + public int i13; + public int i14; + public int i15; + public int i16; + public int i17; + public int i18; + public int i19; + public int i20; + } + + private struct EnormousStruct + { + public LargeStruct s1; + public LargeStruct s2; + public LargeStruct s3; + public LargeStruct s4; + public LargeStruct s5; + public LargeStruct s6; + public LargeStruct s7; + public LargeStruct s8; + public LargeStruct s9; + public LargeStruct s10; + } } } From ad9afeb0941f3d642b22479316f51d0a093b6f90 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Thu, 31 Oct 2024 16:56:22 -0700 Subject: [PATCH 13/15] cleanup --- .../SegmentedListBenchmarks_Add.cs | 74 +++++++++---------- 1 file changed, 35 insertions(+), 39 deletions(-) diff --git a/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_Add.cs b/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_Add.cs index 5738777e90895..e2b6243529211 100644 --- a/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_Add.cs +++ b/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_Add.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using BenchmarkDotNet.Attributes; +using Microsoft.CodeAnalysis.Collections; namespace IdeCoreBenchmarks { @@ -12,15 +13,10 @@ public class SegmentedListBenchmarks_Add [Params(1_000, 10_000, 100_000, 1_000_000)] public int Count { get; set; } - [GlobalSetup] - public void GlobalSetup() - { - } - [Benchmark(Description = "AddIntToList")] public void AddIntToList() { - var array = new Microsoft.CodeAnalysis.Collections.SegmentedList(); + var array = new SegmentedList(); var iterations = Count; for (var i = 0; i < iterations; i++) @@ -32,7 +28,7 @@ public void AddIntToList() [Benchmark(Description = "AddObjectToList")] public void AddObjectToList() { - var array = new Microsoft.CodeAnalysis.Collections.SegmentedList(); + var array = new SegmentedList(); var iterations = Count; for (var i = 0; i < iterations; i++) @@ -44,7 +40,7 @@ public void AddObjectToList() [Benchmark(Description = "AddLargeStructToList")] public void AddLargeStructToList() { - var array = new Microsoft.CodeAnalysis.Collections.SegmentedList(); + var array = new SegmentedList(); var item = new LargeStruct(); var iterations = Count; @@ -57,7 +53,7 @@ public void AddLargeStructToList() [Benchmark(Description = "AddEnormousStructToList")] public void AddEnormousStructToList() { - var array = new Microsoft.CodeAnalysis.Collections.SegmentedList(); + var array = new SegmentedList(); var item = new EnormousStruct(); var iterations = Count; @@ -69,40 +65,40 @@ public void AddEnormousStructToList() private struct LargeStruct { - public int i1; - public int i2; - public int i3; - public int i4; - public int i5; - public int i6; - public int i7; - public int i8; - public int i9; - public int i10; - public int i11; - public int i12; - public int i13; - public int i14; - public int i15; - public int i16; - public int i17; - public int i18; - public int i19; - public int i20; + public int i1 { get; set; } + public int i2 { get; set; } + public int i3 { get; set; } + public int i4 { get; set; } + public int i5 { get; set; } + public int i6 { get; set; } + public int i7 { get; set; } + public int i8 { get; set; } + public int i9 { get; set; } + public int i10 { get; set; } + public int i11 { get; set; } + public int i12 { get; set; } + public int i13 { get; set; } + public int i14 { get; set; } + public int i15 { get; set; } + public int i16 { get; set; } + public int i17 { get; set; } + public int i18 { get; set; } + public int i19 { get; set; } + public int i20 { get; set; } } private struct EnormousStruct { - public LargeStruct s1; - public LargeStruct s2; - public LargeStruct s3; - public LargeStruct s4; - public LargeStruct s5; - public LargeStruct s6; - public LargeStruct s7; - public LargeStruct s8; - public LargeStruct s9; - public LargeStruct s10; + public LargeStruct s1 { get; set; } + public LargeStruct s2 { get; set; } + public LargeStruct s3 { get; set; } + public LargeStruct s4 { get; set; } + public LargeStruct s5 { get; set; } + public LargeStruct s6 { get; set; } + public LargeStruct s7 { get; set; } + public LargeStruct s8 { get; set; } + public LargeStruct s9 { get; set; } + public LargeStruct s10 { get; set; } } } } From b217eec21f924921d843f7c35468dceac21f3759 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Fri, 1 Nov 2024 06:10:59 -0700 Subject: [PATCH 14/15] Test cleanup --- .../SegmentedList.Generic.Tests.Capacity.cs | 67 +++++++------------ 1 file changed, 25 insertions(+), 42 deletions(-) diff --git a/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.Capacity.cs b/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.Capacity.cs index b4d5bc70e7d6f..1eb3fad3c8089 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.Capacity.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Collections/List/SegmentedList.Generic.Tests.Capacity.cs @@ -34,28 +34,26 @@ public static IEnumerable TestLengthsAndSegmentCounts [Theory] [MemberData(nameof(ValidCollectionSizes))] - public void Capacity_ArgumentValidity(int count) + public void Capacity_ArgumentValidity(int initialCapacity) { - var list = new SegmentedList(count); - for (var i = 0; i < count; i++) + var list = new SegmentedList(initialCapacity); + + for (var i = 0; i < initialCapacity; i++) list.Add(CreateT(i)); - Assert.Throws(() => list.Capacity = count - 1); + Assert.Throws(() => list.Capacity = initialCapacity - 1); } [Theory] - [InlineData(0, 0, 1)] - [InlineData(0, 0, 10)] - [InlineData(4, 4, 6)] - [InlineData(4, 4, 10)] - [InlineData(4, 4, 100_000)] - public void Capacity_MatchesSizeRequested(int initialCapacity, int initialSize, int requestedCapacity) + [InlineData(0, 1)] + [InlineData(0, 10)] + [InlineData(4, 6)] + [InlineData(4, 10)] + [InlineData(4, 100_000)] + public void Capacity_MatchesSizeRequested(int initialCapacity, int requestedCapacity) { var list = new SegmentedList(initialCapacity); - for (var i = 0; i < initialSize; i++) - list.Add(CreateT(i)); - list.Capacity = requestedCapacity; Assert.Equal(requestedCapacity, list.Capacity); @@ -63,26 +61,21 @@ public void Capacity_MatchesSizeRequested(int initialCapacity, int initialSize, [Theory] [MemberData(nameof(TestLengthsAndSegmentCounts))] - public void Capacity_ReusesSegments( - int length, - int addSegmentCount) + public void Capacity_ReusesSegments(int initialCapacity, int segmentCountToAdd) { - var elementCountToAdd = SegmentedArray.TestAccessor.SegmentSize * addSegmentCount; - var o = new object(); + var elementCountToAdd = segmentCountToAdd * SegmentedArray.TestAccessor.SegmentSize; - var segmented = new SegmentedList(length); - for (var i = 0; i < length; i++) - segmented.Add(o); + var segmented = new SegmentedList(initialCapacity); var oldSegments = SegmentedCollectionsMarshal.AsSegments(segmented.GetTestAccessor().Items); var oldSegmentCount = oldSegments.Length; - segmented.Capacity = length + elementCountToAdd; + segmented.Capacity = initialCapacity + elementCountToAdd; var resizedSegments = SegmentedCollectionsMarshal.AsSegments(segmented.GetTestAccessor().Items); var resizedSegmentCount = resizedSegments.Length; - Assert.Equal(oldSegmentCount + addSegmentCount, resizedSegmentCount); + Assert.Equal(oldSegmentCount + segmentCountToAdd, resizedSegmentCount); for (var i = 0; i < oldSegmentCount - 1; i++) Assert.Same(resizedSegments[i], oldSegments[i]); @@ -97,18 +90,14 @@ public void Capacity_ReusesSegments( [Theory] [CombinatorialData] public void Capacity_InOnlySingleSegment( - [CombinatorialValues(1, 2, 10, 100)] int length, + [CombinatorialValues(1, 2, 10, 100)] int initialCapacity, [CombinatorialValues(1, 2, 10, 100)] int addItemCount) { - var o = new object(); - - var segmented = new SegmentedList(length); - for (var i = 0; i < length; i++) - segmented.Add(o); + var segmented = new SegmentedList(initialCapacity); var oldSegments = SegmentedCollectionsMarshal.AsSegments(segmented.GetTestAccessor().Items); - segmented.Capacity = length + addItemCount; + segmented.Capacity = initialCapacity + addItemCount; var resizedSegments = SegmentedCollectionsMarshal.AsSegments(segmented.GetTestAccessor().Items); @@ -119,17 +108,14 @@ public void Capacity_InOnlySingleSegment( } [Theory] - [InlineData(0, 0, 1, 4)] - [InlineData(0, 0, 10, 10)] - [InlineData(4, 4, 6, 8)] - [InlineData(4, 4, 10, 10)] - public void EnsureCapacity_ResizesAppropriately(int initialCapacity, int initialSize, int requestedCapacity, int expectedCapacity) + [InlineData(0, 1, 4)] + [InlineData(0, 10, 10)] + [InlineData(4, 6, 8)] + [InlineData(4, 10, 10)] + public void EnsureCapacity_ResizesAppropriately(int initialCapacity, int requestedCapacity, int expectedCapacity) { var list = new SegmentedList(initialCapacity); - for (var i = 0; i < initialSize; i++) - list.Add(CreateT(i)); - list.EnsureCapacity(requestedCapacity); Assert.Equal(expectedCapacity, list.Capacity); @@ -141,12 +127,9 @@ public void EnsureCapacity_ResizesAppropriately(int initialCapacity, int initial [InlineData(4)] public void EnsureCapacity_MatchesSizeWithLargeCapacityRequest(int segmentCount) { - var elementCount = SegmentedArray.TestAccessor.SegmentSize * segmentCount; + var elementCount = segmentCount * SegmentedArray.TestAccessor.SegmentSize; var list = new SegmentedList(elementCount); - for (var i = 0; i < elementCount; i++) - list.Add(CreateT(i)); - Assert.Equal(elementCount, list.Capacity); var requestedCapacity = 2 * elementCount + 10; From e2eb9ace0e132d5e89461aaaf657b4facd468dc6 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Fri, 1 Nov 2024 07:05:54 -0700 Subject: [PATCH 15/15] Cleanup benchmarking code --- .../SegmentedListBenchmarks_Add.cs | 47 +++++-------------- 1 file changed, 11 insertions(+), 36 deletions(-) diff --git a/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_Add.cs b/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_Add.cs index e2b6243529211..ff9629c2302ac 100644 --- a/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_Add.cs +++ b/src/Tools/IdeCoreBenchmarks/SegmentedListBenchmarks_Add.cs @@ -13,54 +13,29 @@ public class SegmentedListBenchmarks_Add [Params(1_000, 10_000, 100_000, 1_000_000)] public int Count { get; set; } - [Benchmark(Description = "AddIntToList")] + [Benchmark] public void AddIntToList() - { - var array = new SegmentedList(); - var iterations = Count; - - for (var i = 0; i < iterations; i++) - { - array.Add(i); - } - } + => AddToList(1); - [Benchmark(Description = "AddObjectToList")] + [Benchmark] public void AddObjectToList() - { - var array = new SegmentedList(); - var iterations = Count; + => AddToList(new object()); - for (var i = 0; i < iterations; i++) - { - array.Add(null); - } - } - - [Benchmark(Description = "AddLargeStructToList")] + [Benchmark] public void AddLargeStructToList() - { - var array = new SegmentedList(); - var item = new LargeStruct(); - var iterations = Count; - - for (var i = 0; i < iterations; i++) - { - array.Add(item); - } - } + => AddToList(new LargeStruct()); - [Benchmark(Description = "AddEnormousStructToList")] + [Benchmark] public void AddEnormousStructToList() + => AddToList(new EnormousStruct()); + + private void AddToList(T item) { - var array = new SegmentedList(); - var item = new EnormousStruct(); + var array = new SegmentedList(); var iterations = Count; for (var i = 0; i < iterations; i++) - { array.Add(item); - } } private struct LargeStruct