Skip to content

Commit

Permalink
Unify Array enumerators between CoreCLR and NativeAOT (#82751)
Browse files Browse the repository at this point in the history 
* Unify Array enumerators between CoreCLR and NativeAOT

Contributes to #82732

Reduces BasicMinimalApi native AOT size on Windows x64 by 68kB

* Update AOT compiler

* Naming convention

Co-authored-by: Stephen Toub <stoub@microsoft.com>
  • Loading branch information
@jkotas @stephentoub
jkotas and stephentoub authored Mar 2, 2023
1 parent 68e8cd9 commit 55910db
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 123 deletions.
32 changes: 16 additions & 16 deletions src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,51 +464,51 @@ internal IEnumerator<T> GetEnumerator<T>()
{
// ! Warning: "this" is an array, not an SZArrayHelper. See comments above
// ! or you may introduce a security hole!
T[] _this = Unsafe.As<T[]>(this);
return _this.Length == 0 ? SZGenericArrayEnumerator<T>.Empty : new SZGenericArrayEnumerator<T>(_this);
T[] @this = Unsafe.As<T[]>(this);
return @this.Length == 0 ? SZGenericArrayEnumerator<T>.Empty : new SZGenericArrayEnumerator<T>(@this);
}

private void CopyTo<T>(T[] array, int index)
{
// ! Warning: "this" is an array, not an SZArrayHelper. See comments above
// ! or you may introduce a security hole!

T[] _this = Unsafe.As<T[]>(this);
Array.Copy(_this, 0, array, index, _this.Length);
T[] @this = Unsafe.As<T[]>(this);
Array.Copy(@this, 0, array, index, @this.Length);
}

internal int get_Count<T>()
{
// ! Warning: "this" is an array, not an SZArrayHelper. See comments above
// ! or you may introduce a security hole!
T[] _this = Unsafe.As<T[]>(this);
return _this.Length;
T[] @this = Unsafe.As<T[]>(this);
return @this.Length;
}

internal T get_Item<T>(int index)
{
// ! Warning: "this" is an array, not an SZArrayHelper. See comments above
// ! or you may introduce a security hole!
T[] _this = Unsafe.As<T[]>(this);
if ((uint)index >= (uint)_this.Length)
T[] @this = Unsafe.As<T[]>(this);
if ((uint)index >= (uint)@this.Length)
{
ThrowHelper.ThrowArgumentOutOfRange_IndexMustBeLessException();
}

return _this[index];
return @this[index];
}

internal void set_Item<T>(int index, T value)
{
// ! Warning: "this" is an array, not an SZArrayHelper. See comments above
// ! or you may introduce a security hole!
T[] _this = Unsafe.As<T[]>(this);
if ((uint)index >= (uint)_this.Length)
T[] @this = Unsafe.As<T[]>(this);
if ((uint)index >= (uint)@this.Length)
{
ThrowHelper.ThrowArgumentOutOfRange_IndexMustBeLessException();
}

_this[index] = value;
@this[index] = value;
}

private void Add<T>(T _)
Expand All @@ -521,8 +521,8 @@ private bool Contains<T>(T value)
{
// ! Warning: "this" is an array, not an SZArrayHelper. See comments above
// ! or you may introduce a security hole!
T[] _this = Unsafe.As<T[]>(this);
return Array.IndexOf(_this, value, 0, _this.Length) >= 0;
T[] @this = Unsafe.As<T[]>(this);
return Array.IndexOf(@this, value, 0, @this.Length) >= 0;
}

private bool get_IsReadOnly<T>()
Expand All @@ -542,8 +542,8 @@ private int IndexOf<T>(T value)
{
// ! Warning: "this" is an array, not an SZArrayHelper. See comments above
// ! or you may introduce a security hole!
T[] _this = Unsafe.As<T[]>(this);
return Array.IndexOf(_this, value, 0, _this.Length);
T[] @this = Unsafe.As<T[]>(this);
return Array.IndexOf(@this, value, 0, @this.Length);
}

private void Insert<T>(int _, T _1)
Expand Down
88 changes: 9 additions & 79 deletions src/coreclr/nativeaot/System.Private.CoreLib/src/System/Array.NativeAot.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1157,38 +1157,6 @@ private static int LastIndexOfImpl<T>(T[] array, T value, int startIndex, int co
}
}

internal class ArrayEnumeratorBase : ICloneable
{
protected int _index;
protected int _endIndex;

internal ArrayEnumeratorBase()
{
_index = -1;
}

public bool MoveNext()
{
if (_index < _endIndex)
{
_index++;
return (_index < _endIndex);
}
return false;
}

public object Clone()
{
return MemberwiseClone();
}

#pragma warning disable CA1822 // https://github.com/dotnet/roslyn-analyzers/issues/5911
public void Dispose()
{
}
#pragma warning restore CA1822
}

//
// Note: the declared base type and interface list also determines what Reflection returns from TypeInfo.BaseType and TypeInfo.ImplementedInterfaces for array types.
// This also means the class must be declared "public" so that the framework can reflect on it.
Expand All @@ -1200,17 +1168,15 @@ private Array() { }

public new IEnumerator<T> GetEnumerator()
{
// get length so we don't have to call the Length property again in ArrayEnumerator constructor
// and avoid more checking there too.
int length = this.Length;
return length == 0 ? ArrayEnumerator.Empty : new ArrayEnumerator(Unsafe.As<T[]>(this), length);
T[] @this = Unsafe.As<T[]>(this);
return @this.Length == 0 ? SZGenericArrayEnumerator<T>.Empty : new SZGenericArrayEnumerator<T>(@this);
}

public int Count
{
get
{
return this.Length;
return Unsafe.As<T[]>(this).Length;
}
}

Expand Down Expand Up @@ -1240,13 +1206,14 @@ public void Clear()

public bool Contains(T item)
{
T[] array = Unsafe.As<T[]>(this);
return Array.IndexOf(array, item, 0, array.Length) >= 0;
T[] @this = Unsafe.As<T[]>(this);
return Array.IndexOf(@this, item, 0, @this.Length) >= 0;
}

public void CopyTo(T[] array, int arrayIndex)
{
Array.Copy(Unsafe.As<T[]>(this), 0, array, arrayIndex, this.Length);
T[] @this = Unsafe.As<T[]>(this);
Array.Copy(@this, 0, array, arrayIndex, @this.Length);
}

public bool Remove(T item)
Expand Down Expand Up @@ -1284,8 +1251,8 @@ public T this[int index]

public int IndexOf(T item)
{
T[] array = Unsafe.As<T[]>(this);
return Array.IndexOf(array, item, 0, array.Length);
T[] @this = Unsafe.As<T[]>(this);
return Array.IndexOf(@this, item, 0, @this.Length);
}

public void Insert(int index, T item)
Expand All @@ -1297,43 +1264,6 @@ public void RemoveAt(int index)
{
ThrowHelper.ThrowNotSupportedException();
}

private sealed class ArrayEnumerator : ArrayEnumeratorBase, IEnumerator<T>
{
private readonly T[] _array;

// Passing -1 for endIndex so that MoveNext always returns false without mutating _index
internal static readonly ArrayEnumerator Empty = new ArrayEnumerator(null, -1);

internal ArrayEnumerator(T[] array, int endIndex)
{
_array = array;
_endIndex = endIndex;
}

public T Current
{
get
{
if ((uint)_index >= (uint)_endIndex)
ThrowHelper.ThrowInvalidOperationException();
return _array[_index];
}
}

object IEnumerator.Current
{
get
{
return Current;
}
}

void IEnumerator.Reset()
{
_index = -1;
}
}
}

public class MDArray
Expand Down
12 changes: 5 additions & 7 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1047,8 +1047,7 @@ public MetadataType ArrayOfTClass
{
get
{
_systemArrayOfTClass ??= _systemArrayOfTClass = _context.SystemModule.GetKnownType("System", "Array`1");
return _systemArrayOfTClass;
return _systemArrayOfTClass ??= _context.SystemModule.GetKnownType("System", "Array`1");
}
}

Expand All @@ -1057,8 +1056,9 @@ public TypeDesc ArrayOfTEnumeratorType
{
get
{
_systemArrayOfTEnumeratorType ??= _systemArrayOfTEnumeratorType = ArrayOfTClass.GetNestedType("ArrayEnumerator");
return _systemArrayOfTEnumeratorType;
// This type is optional, but it's fine for this cache to be ineffective if that happens.
// Those scenarios are rare and typically deal with small compilations.
return _systemArrayOfTEnumeratorType ??= _context.SystemModule.GetType("System", "SZGenericArrayEnumerator`1", throwIfNotFound: false);
}
}

Expand All @@ -1069,9 +1069,7 @@ public MethodDesc InstanceMethodRemovedHelper
{
// This helper is optional, but it's fine for this cache to be ineffective if that happens.
// Those scenarios are rare and typically deal with small compilations.
_instanceMethodRemovedHelper ??= TypeSystemContext.GetOptionalHelperEntryPoint("ThrowHelpers", "ThrowInstanceBodyRemoved");

return _instanceMethodRemovedHelper;
return _instanceMethodRemovedHelper ??= TypeSystemContext.GetOptionalHelperEntryPoint("ThrowHelpers", "ThrowInstanceBodyRemoved");
}
}

Expand Down
56 changes: 35 additions & 21 deletions src/libraries/System.Private.CoreLib/src/System/Array.Enumerators.cs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Runtime.CompilerServices;
using System.Diagnostics;

namespace System
Expand All @@ -26,9 +28,10 @@ public object Clone()
public bool MoveNext()
{
nint index = _index + 1;
if ((nuint)index >= _array.NativeLength)
nuint length = _array.NativeLength;
if ((nuint)index >= length)
{
_index = (nint)_array.NativeLength;
_index = (nint)length;
return false;
}
_index = index;
Expand Down Expand Up @@ -64,18 +67,12 @@ public void Reset()
}
}

internal sealed class SZGenericArrayEnumerator<T> : IEnumerator<T>
internal class SZGenericArrayEnumeratorBase
{
private readonly T[] _array;
private int _index;
protected readonly Array _array;
protected int _index;

// Array.Empty is intentionally omitted here, since we don't want to pay for generic instantiations that
// wouldn't have otherwise been used.
#pragma warning disable CA1825
internal static readonly SZGenericArrayEnumerator<T> Empty = new SZGenericArrayEnumerator<T>(new T[0]);
#pragma warning restore CA1825

internal SZGenericArrayEnumerator(T[] array)
protected SZGenericArrayEnumeratorBase(Array array)
{
Debug.Assert(array != null);

Expand All @@ -86,21 +83,44 @@ internal SZGenericArrayEnumerator(T[] array)
public bool MoveNext()
{
int index = _index + 1;
if ((uint)index >= (uint)_array.Length)
uint length = (uint)_array.NativeLength;
if ((uint)index >= length)
{
_index = _array.Length;
_index = (int)length;
return false;
}
_index = index;
return true;
}

public void Reset() => _index = -1;

#pragma warning disable CA1822 // https://github.com/dotnet/roslyn-analyzers/issues/5911
public void Dispose()
{
}
#pragma warning restore CA1822
}

internal sealed class SZGenericArrayEnumerator<T> : SZGenericArrayEnumeratorBase, IEnumerator<T>
{
// Array.Empty is intentionally omitted here, since we don't want to pay for generic instantiations that
// wouldn't have otherwise been used.
#pragma warning disable CA1825
internal static readonly SZGenericArrayEnumerator<T> Empty = new SZGenericArrayEnumerator<T>(new T[0]);
#pragma warning restore CA1825

public SZGenericArrayEnumerator(T[] array)
: base(array)
{
}

public T Current
{
get
{
int index = _index;
T[] array = _array;
T[] array = Unsafe.As<T[]>(_array);

if ((uint)index >= (uint)array.Length)
{
Expand All @@ -112,11 +132,5 @@ public T Current
}

object? IEnumerator.Current => Current;

void IEnumerator.Reset() => _index = -1;

public void Dispose()
{
}
}
}

0 comments on commit 55910db

Please sign in to comment.