Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Eliminate array allocations in CustomAttribute.IsDefined() overloads #2632

Closed
jack-pappas wants to merge 4 commits into from
Closed

Eliminate array allocations in CustomAttribute.IsDefined() overloads #2632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5cf72d9
Eliminate array allocations in CustomAttribute.IsDefined() overloads.
jack-pappas Jan 10, 2016
a127ebd
Eliminate uses of the 'var' keyword as per code review comments.
jack-pappas Jan 16, 2016
ed62790
Avoid allocations for empty inputs in Array.AsReadOnly().
jack-pappas Jan 16, 2016
f43f69a
The return value of CustomAttributeData.GetCustomAttributes should be…
jack-pappas Jan 16, 2016
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
4 changes: 3 additions & 1 deletion src/mscorlib/src/System/Array.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@ public static ReadOnlyCollection<T> AsReadOnly<T>(T[] array) {
}
Contract.Ensures(Contract.Result<ReadOnlyCollection<T>>() != null);

// If the array is empty, return the cached, empty ReadOnlyCollection<T>
// instance to avoid allocating an unnecessary object.
// T[] implements IList<T>.
return new ReadOnlyCollection<T>(array);
return array.Length == 0 ? EmptyReadOnlyCollection<T>.Value : new ReadOnlyCollection<T>(array);
}

[ReliabilityContract(Consistency.WillNotCorruptState, Cer.MayFail)]
Expand Down
7 changes: 7 additions & 0 deletions src/mscorlib/src/System/Collections/ObjectModel/ReadOnlyCollection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,4 +228,11 @@ void IList.RemoveAt(int index) {
ThrowHelper.ThrowNotSupportedException(ExceptionResource.NotSupported_ReadOnlyCollection);
}
}

// Useful in number of places that return an empty ReadOnlyCollection<T> to avoid unnecessary memory allocation.
internal static class EmptyReadOnlyCollection<T>
{
public static readonly ReadOnlyCollection<T> Value =
new ReadOnlyCollection<T>(Array.Empty<T>());
}
}
197 changes: 158 additions & 39 deletions src/mscorlib/src/System/Reflection/CustomAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,34 +304,35 @@ private static CustomAttributeType InitCustomAttributeType(RuntimeType parameter
[System.Security.SecurityCritical] // auto-generated
private static IList<CustomAttributeData> GetCustomAttributes(RuntimeModule module, int tkTarget)
{
CustomAttributeRecord[] records = GetCustomAttributeRecords(module, tkTarget);
CustomAttributeRecordCollection records = GetCustomAttributeRecordCollection(module, tkTarget);

CustomAttributeData[] customAttributes = new CustomAttributeData[records.Length];
for (int i = 0; i < records.Length; i++)
customAttributes[i] = new CustomAttributeData(module, records[i]);

return Array.AsReadOnly(customAttributes);
}
#endregion

#region Internal Static Members
[System.Security.SecurityCritical] // auto-generated
internal unsafe static CustomAttributeRecord[] GetCustomAttributeRecords(RuntimeModule module, int targetToken)
{
MetadataImport scope = module.MetadataImport;
CustomAttributeData[] customAttributes = null;
int recordIndex = 0;
foreach (CustomAttributeRecord record in records)
{
// Create the customAttributes array only if it's going to be non-empty.
if (customAttributes == null)
{
customAttributes = new CustomAttributeData[records.Count];
}

MetadataEnumResult tkCustomAttributeTokens;
scope.EnumCustomAttributes(targetToken, out tkCustomAttributeTokens);
customAttributes[recordIndex++] = new CustomAttributeData(module, record);
}

CustomAttributeRecord[] records = new CustomAttributeRecord[tkCustomAttributeTokens.Length];
// If there weren't any custom attributes, return the empty array instance
// instead of allocating a new empty array.
// Array.AsReadOnly() returns a cached instance for an empty input array
// so won't cause an allocation in that case.
return Array.AsReadOnly(customAttributes ?? Array.Empty<CustomAttributeData>());
}
#endregion

for (int i = 0; i < records.Length; i++)
{
scope.GetCustomAttributeProps(
tkCustomAttributeTokens[i], out records[i].tkCtor.Value, out records[i].blob);
}
#region Internal Static Members

return records;
[System.Security.SecurityCritical]
internal unsafe static CustomAttributeRecordCollection GetCustomAttributeRecordCollection(RuntimeModule module, int targetToken)
{
return new CustomAttributeRecordCollection(module.MetadataImport, targetToken);
}

internal static CustomAttributeTypedArgument Filter(IList<CustomAttributeData> attrs, Type caType, int parameter)
Expand Down Expand Up @@ -579,6 +580,127 @@ public virtual IList<CustomAttributeNamedArgument> NamedArguments
}
}
#endregion

#region Record Enumeration / Enumerator

internal struct CustomAttributeRecordCollection : IReadOnlyList<CustomAttributeRecord>
{
private readonly MetadataImport _scope;
private readonly MetadataEnumResult _tkCustomAttributeTokens;

internal CustomAttributeRecordCollection(MetadataImport scope, int targetToken)
{
_scope = scope;
_scope.EnumCustomAttributes(targetToken, out _tkCustomAttributeTokens);
}

public CustomAttributeRecord this[int index]
{
get
{
if (index < 0 || index >= this.Count)
{
throw new IndexOutOfRangeException();
}
Contract.EndContractBlock();

CustomAttributeRecord currentRecord;
_scope.GetCustomAttributeProps(
_tkCustomAttributeTokens[index], out currentRecord.tkCtor.Value, out currentRecord.blob);
return currentRecord;
}
}

public int Count
{
get
{
Contract.Ensures(Contract.Result<int>() >= 0);
Contract.EndContractBlock();

return _tkCustomAttributeTokens.Length;
}
}

public CustomAttributeRecordEnumerator GetEnumerator()
{
return new CustomAttributeRecordEnumerator(_scope, _tkCustomAttributeTokens);
}

IEnumerator<CustomAttributeRecord> IEnumerable<CustomAttributeRecord>.GetEnumerator()
{
return GetEnumerator();
}

IEnumerator IEnumerable.GetEnumerator()
{
return GetEnumerator();
}
}

internal struct CustomAttributeRecordEnumerator : IEnumerator<CustomAttributeRecord>
{
private readonly MetadataImport _scope;
private readonly MetadataEnumResult _tkCustomAttributeTokens;
private int _nextIndex; // I.e., current index + 1

internal CustomAttributeRecordEnumerator(MetadataImport scope, MetadataEnumResult customAttributeTokens)
{
_scope = scope;
_tkCustomAttributeTokens = customAttributeTokens;
_nextIndex = 0;
}

public CustomAttributeRecord Current
{
get
{
if (_nextIndex <= 0)
{
throw new InvalidOperationException();
}
Contract.EndContractBlock();

CustomAttributeRecord currentRecord;
_scope.GetCustomAttributeProps(
_tkCustomAttributeTokens[_nextIndex - 1], out currentRecord.tkCtor.Value, out currentRecord.blob);
return currentRecord;
}
}

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

public void Dispose()
{
// Nothing to dispose.
}

public bool MoveNext()
{
if (_nextIndex >= _tkCustomAttributeTokens.Length)
{
return false;
}
else
{
_nextIndex += 1;
return true;
}
}

public void Reset()
{
_nextIndex = 0;
}
}

#endregion
}

[Serializable]
Expand Down Expand Up @@ -1594,7 +1716,8 @@ private static bool IsCustomAttributeDefined(
throw new InvalidOperationException(Environment.GetResourceString("Arg_ReflectionOnlyCA"));
Contract.EndContractBlock();

CustomAttributeRecord[] car = CustomAttributeData.GetCustomAttributeRecords(decoratedModule, decoratedMetadataToken);
CustomAttributeData.CustomAttributeRecordCollection cars =
CustomAttributeData.GetCustomAttributeRecordCollection(decoratedModule, decoratedMetadataToken);

if (attributeFilterType != null)
{
Expand All @@ -1609,10 +1732,8 @@ private static bool IsCustomAttributeDefined(
// we can cache the successful APTCA check between the decorated and the declared assembly.
Assembly lastAptcaOkAssembly = null;

for (int i = 0; i < car.Length; i++)
foreach (CustomAttributeRecord caRecord in cars)
{
CustomAttributeRecord caRecord = car[i];

if (FilterCustomAttributeRecord(caRecord, scope, ref lastAptcaOkAssembly,
decoratedModule, decoratedMetadataToken, attributeFilterType, mustBeInheritable, null, null,
out attributeType, out ctor, out ctorHasParameters, out isVarArg))
Expand All @@ -1624,10 +1745,8 @@ private static bool IsCustomAttributeDefined(
Contract.Assert(attributeFilterType == null);
Contract.Assert(!MetadataToken.IsNullToken(attributeCtorToken));

for (int i = 0; i < car.Length; i++)
foreach (CustomAttributeRecord caRecord in cars)
{
CustomAttributeRecord caRecord = car[i];

if (caRecord.tkCtor == attributeCtorToken)
return true;
}
Expand All @@ -1653,15 +1772,16 @@ private unsafe static object[] GetCustomAttributes(
Contract.EndContractBlock();

MetadataImport scope = decoratedModule.MetadataImport;
CustomAttributeRecord[] car = CustomAttributeData.GetCustomAttributeRecords(decoratedModule, decoratedMetadataToken);
CustomAttributeData.CustomAttributeRecordCollection cars =
CustomAttributeData.GetCustomAttributeRecordCollection(decoratedModule, decoratedMetadataToken);

bool useObjectArray = (attributeFilterType == null || attributeFilterType.IsValueType || attributeFilterType.ContainsGenericParameters);
Type arrayType = useObjectArray ? typeof(object) : attributeFilterType;

if (attributeFilterType == null && car.Length == 0)
if (attributeFilterType == null && cars.Count == 0)
return CreateAttributeArrayHelper(arrayType, 0);

object[] attributes = CreateAttributeArrayHelper(arrayType, car.Length);
object[] attributes = CreateAttributeArrayHelper(arrayType, cars.Count);
int cAttributes = 0;

// Custom attribute security checks are done with respect to the assembly *decorated* with the
Expand All @@ -1678,10 +1798,9 @@ private unsafe static object[] GetCustomAttributes(
// we can cache the successful APTCA check between the decorated and the declared assembly.
Assembly lastAptcaOkAssembly = null;

for (int i = 0; i < car.Length; i++)
foreach (CustomAttributeRecord caRecord in cars)
{
object attribute = null;
CustomAttributeRecord caRecord = car[i];

IRuntimeMethodInfo ctor = null;
RuntimeType attributeType = null;
Expand Down Expand Up @@ -1827,7 +1946,7 @@ private unsafe static object[] GetCustomAttributes(
// finally or CERs here.
frame.Pop();

if (cAttributes == car.Length && pcaCount == 0)
if (cAttributes == cars.Count && pcaCount == 0)
return attributes;

object[] result = CreateAttributeArrayHelper(arrayType, cAttributes + pcaCount);
Expand Down Expand Up @@ -2005,13 +2124,13 @@ internal static AttributeUsageAttribute GetAttributeUsage(RuntimeType decoratedA
{
RuntimeModule decoratedModule = decoratedAttribute.GetRuntimeModule();
MetadataImport scope = decoratedModule.MetadataImport;
CustomAttributeRecord[] car = CustomAttributeData.GetCustomAttributeRecords(decoratedModule, decoratedAttribute.MetadataToken);
CustomAttributeData.CustomAttributeRecordCollection cars =
CustomAttributeData.GetCustomAttributeRecordCollection(decoratedModule, decoratedAttribute.MetadataToken);

AttributeUsageAttribute attributeUsageAttribute = null;

for (int i = 0; i < car.Length; i++)
foreach (CustomAttributeRecord caRecord in cars)
{
CustomAttributeRecord caRecord = car[i];
RuntimeType attributeType = decoratedModule.ResolveType(scope.GetParentToken(caRecord.tkCtor), null, null) as RuntimeType;

if (attributeType != (RuntimeType)typeof(AttributeUsageAttribute))
Expand Down