Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,30 @@ public void Append(ArrayBuilder<T> newItems)
_count += newItems.Count;
}

public void AddRange(IEnumerable<T> items)
{
Debug.Assert(items != null);

if (items is ICollection<T> collection)
{
int count = collection.Count;
if (count > 0)
{
EnsureCapacity(_count + count);

collection.CopyTo(_items, _count);
_count += count;
}
}
else
{
foreach (T item in items)
{
Add(item);
}
}
}

public void ZeroExtend(int numItems)
{
Debug.Assert(numItems >= 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,43 @@ public void Add(T item)
{
if (_count == Capacity)
{
EnsureCapacity(_count + 1);
Grow(_count + 1);
}

UncheckedAdd(item);
}

/// <summary>
/// Adds a range of items to the backing array, resizing it as necessary.
/// </summary>
/// <param name="items">The items to add.</param>
public void AddRange(IEnumerable<T> items)
{
Debug.Assert(items != null);

if (items is ICollection<T> collection)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@333fred, all of the cited uses won't have an ICollection, as they all appear to be iterators. Are there other callers where this code will be used?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There aren't currently. However, if AddRange is an overload that is offered on ArrayBuilder, then it seems likely that at some point, someone will try to call it with an ICollection; while I didn't suggest that Copilot add this optimization, it seems like a reasonable thing to keep (assuming, of course, it uses CopyTo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The internal helper types like ArrayBuilder are implemented to be minimal. They are often not behaving like you would expect from a public BCL API and they tend to expose methods to squeeze bits of extra performance (e.g. ArrayBuilder.UncheckedAdd).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to go either way here. Just tell whether you want me to keep the optimization around ICollection or not 🙂.

{
int count = collection.Count;
if (count > 0)
{
if (_count + count > Capacity)
{
Grow(_count + count);
}

collection.CopyTo(_array!, _count);
_count += count;
}
}
else
{
foreach (T item in items)
{
Add(item);
}
}
}

/// <summary>
/// Gets the first item in this builder.
/// </summary>
Expand Down Expand Up @@ -136,7 +167,7 @@ public void UncheckedAdd(T item)
_array![_count++] = item;
}

private void EnsureCapacity(int minimum)
private void Grow(int minimum)
{
Debug.Assert(minimum > Capacity);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,140 @@ public void UncheckedAdd(int capacity)
VerifyBuilderContents(Enumerable.Repeat(default(T), capacity), builder);
}

[Theory]
[MemberData(nameof(EnumerableData))]
public void AddRange(IEnumerable<T> seed)
{
var builder = new ArrayBuilder<T>();
builder.AddRange(seed);

int count = builder.Count;
T[] array = builder.ToArray();

Assert.Equal(count, array.Length);
Assert.Equal(seed, array);
}

[Fact]
public void AddRange_EmptyEnumerable()
{
var builder = new ArrayBuilder<T>();
builder.AddRange(Enumerable.Empty<T>());

Assert.Equal(0, builder.Count);
Assert.Same(Array.Empty<T>(), builder.ToArray());
}

[Theory]
[MemberData(nameof(EnumerableData))]
public void AddRange_AfterAdd(IEnumerable<T> seed)
{
var builder = new ArrayBuilder<T>();
builder.Add(default(T));
builder.AddRange(seed);

int expectedCount = 1 + seed.Count();
Assert.Equal(expectedCount, builder.Count);

T[] array = builder.ToArray();
Assert.Equal(expectedCount, array.Length);
Assert.Equal(default(T), array[0]);
Assert.Equal(seed, array.Skip(1));
}

[Theory]
[MemberData(nameof(CountData))]
public void AddRange_ICollection_PreallocatesCapacity(int count)
{
// Use a List<T> which implements ICollection<T>
List<T> collection = s_generator.GenerateEnumerable(count).ToList();

var builder = new ArrayBuilder<T>();
builder.AddRange(collection);

Assert.Equal(count, builder.Count);

// When adding an ICollection<T>, capacity should be at least
// enough for the collection (0 if empty)
if (count > 0)
{
Assert.True(builder.Capacity >= count);
}
else
{
Assert.Equal(0, builder.Capacity);
}

Assert.Equal(collection, builder.ToArray());
}

[Fact]
public void AddRange_ICollection_EmptyCollection()
{
// Use an empty List<T> which implements ICollection<T>
List<T> emptyCollection = new List<T>();

var builder = new ArrayBuilder<T>();
builder.AddRange(emptyCollection);

Assert.Equal(0, builder.Count);
Assert.Equal(0, builder.Capacity);
Assert.Same(Array.Empty<T>(), builder.ToArray());
}

[Theory]
[MemberData(nameof(CountData))]
public void AddRange_ICollection_AfterExistingItems(int count)
{
// Add some initial items
var builder = new ArrayBuilder<T>();
builder.Add(default(T));
builder.Add(default(T));
int initialCount = 2;
int initialCapacity = builder.Capacity;

// Use a List<T> which implements ICollection<T>
List<T> collection = s_generator.GenerateEnumerable(count).ToList();
builder.AddRange(collection);

int expectedCount = initialCount + count;
Assert.Equal(expectedCount, builder.Count);

// When adding ICollection<T> after existing items, capacity should be
// at least enough for all items
Assert.True(builder.Capacity >= expectedCount);

T[] array = builder.ToArray();
Assert.Equal(expectedCount, array.Length);
Assert.Equal(default(T), array[0]);
Assert.Equal(default(T), array[1]);
Assert.Equal(collection, array.Skip(2));
}

[Theory]
[MemberData(nameof(CountData))]
public void AddRange_NonICollection_GrowsIncrementally(int count)
{
// Use a generator method to create a pure IEnumerable<T> that is NOT an ICollection<T>
IEnumerable<T> NonCollectionEnumerable()
{
for (int i = 0; i < count; i++)
{
yield return default(T);
}
}

var builder = new ArrayBuilder<T>();
builder.AddRange(NonCollectionEnumerable());

Assert.Equal(count, builder.Count);

// Non-ICollection path should grow incrementally (powers of 2)
Assert.Equal(CalculateExpectedCapacity(count), builder.Capacity);

Assert.Equal(NonCollectionEnumerable(), builder.ToArray());
}

public static TheoryData<int> CapacityData()
{
var data = new TheoryData<int>();
Expand Down
18 changes: 15 additions & 3 deletions src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,11 @@ public static string[] GetFiles(string path, string searchPattern, SearchOption
=> GetFiles(path, searchPattern, EnumerationOptions.FromSearchOption(searchOption));

public static string[] GetFiles(string path, string searchPattern, EnumerationOptions enumerationOptions)
=> new List<string>(InternalEnumeratePaths(path, searchPattern, SearchTarget.Files, enumerationOptions)).ToArray();
{
ArrayBuilder<string> builder = default;
builder.AddRange(InternalEnumeratePaths(path, searchPattern, SearchTarget.Files, enumerationOptions));
return builder.ToArray();
}

public static string[] GetDirectories(string path) => GetDirectories(path, "*", enumerationOptions: EnumerationOptions.Compatible);

Expand All @@ -181,7 +185,11 @@ public static string[] GetDirectories(string path, string searchPattern, SearchO
=> GetDirectories(path, searchPattern, EnumerationOptions.FromSearchOption(searchOption));

public static string[] GetDirectories(string path, string searchPattern, EnumerationOptions enumerationOptions)
=> new List<string>(InternalEnumeratePaths(path, searchPattern, SearchTarget.Directories, enumerationOptions)).ToArray();
{
ArrayBuilder<string> builder = default;
builder.AddRange(InternalEnumeratePaths(path, searchPattern, SearchTarget.Directories, enumerationOptions));
return builder.ToArray();
}

public static string[] GetFileSystemEntries(string path) => GetFileSystemEntries(path, "*", enumerationOptions: EnumerationOptions.Compatible);

Expand All @@ -191,7 +199,11 @@ public static string[] GetFileSystemEntries(string path, string searchPattern, S
=> GetFileSystemEntries(path, searchPattern, EnumerationOptions.FromSearchOption(searchOption));

public static string[] GetFileSystemEntries(string path, string searchPattern, EnumerationOptions enumerationOptions)
=> new List<string>(InternalEnumeratePaths(path, searchPattern, SearchTarget.Both, enumerationOptions)).ToArray();
{
ArrayBuilder<string> builder = default;
builder.AddRange(InternalEnumeratePaths(path, searchPattern, SearchTarget.Both, enumerationOptions));
return builder.ToArray();
}

internal static IEnumerable<string> InternalEnumeratePaths(
string path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,11 @@ public FileInfo[] GetFiles(string searchPattern, SearchOption searchOption)
=> GetFiles(searchPattern, EnumerationOptions.FromSearchOption(searchOption));

public FileInfo[] GetFiles(string searchPattern, EnumerationOptions enumerationOptions)
=> new List<FileInfo>((IEnumerable<FileInfo>)InternalEnumerateInfos(FullPath, searchPattern, SearchTarget.Files, enumerationOptions)).ToArray();
{
ArrayBuilder<FileInfo> builder = default;
builder.AddRange((IEnumerable<FileInfo>)InternalEnumerateInfos(FullPath, searchPattern, SearchTarget.Files, enumerationOptions));
return builder.ToArray();
}

// Returns an array of strongly typed FileSystemInfo entries which will contain a listing
// of all the files and directories.
Expand All @@ -130,7 +134,11 @@ public FileSystemInfo[] GetFileSystemInfos(string searchPattern, SearchOption se
=> GetFileSystemInfos(searchPattern, EnumerationOptions.FromSearchOption(searchOption));

public FileSystemInfo[] GetFileSystemInfos(string searchPattern, EnumerationOptions enumerationOptions)
=> new List<FileSystemInfo>(InternalEnumerateInfos(FullPath, searchPattern, SearchTarget.Both, enumerationOptions)).ToArray();
{
ArrayBuilder<FileSystemInfo> builder = default;
builder.AddRange(InternalEnumerateInfos(FullPath, searchPattern, SearchTarget.Both, enumerationOptions));
return builder.ToArray();
}

// Returns an array of Directories in the current directory.
public DirectoryInfo[] GetDirectories() => GetDirectories("*", enumerationOptions: EnumerationOptions.Compatible);
Expand All @@ -143,7 +151,11 @@ public DirectoryInfo[] GetDirectories(string searchPattern, SearchOption searchO
=> GetDirectories(searchPattern, EnumerationOptions.FromSearchOption(searchOption));

public DirectoryInfo[] GetDirectories(string searchPattern, EnumerationOptions enumerationOptions)
=> new List<DirectoryInfo>((IEnumerable<DirectoryInfo>)InternalEnumerateInfos(FullPath, searchPattern, SearchTarget.Directories, enumerationOptions)).ToArray();
{
ArrayBuilder<DirectoryInfo> builder = default;
builder.AddRange((IEnumerable<DirectoryInfo>)InternalEnumerateInfos(FullPath, searchPattern, SearchTarget.Directories, enumerationOptions));
return builder.ToArray();
}

public IEnumerable<DirectoryInfo> EnumerateDirectories()
=> EnumerateDirectories("*", enumerationOptions: EnumerationOptions.Compatible);
Expand Down
Loading