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

Commit

Permalink
Fix for #21456 (Regressions in attribute allocations for non-generic …
Browse files Browse the repository at this point in the history
…attributes) (#21462)

* Fix for #21456 - restrict increased generic attribute allocations to only generic attributes

This is a trivial quick-fix for #21456 where regressions between 2.1 and 3.0 were discovered on most attibute pathways due to the allocation overhead in the generic-supporting pathways. The workaround is to simply not take that slow/expensive path for non-generics.

While I'd like to optimize `RuntimeModule.ResolveMethod` further, there's a public surface area in play there that makes the changes non-trivial. There, we'll have to choose overhead on the public path (which may still be a net win), or duplication in code for another path.

* Update comments
  • Loading branch information
NickCraver authored and morganbr committed Dec 14, 2018
1 parent eb86377 commit 1c43264
Showing 1 changed file with 9 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -1616,7 +1616,15 @@ private static unsafe bool FilterCustomAttributeRecord(
if (ctorHasParameters)
{
// Resolve method ctor token found in decorated decoratedModule scope
ctor = decoratedModule.ResolveMethod(caRecord.tkCtor, attributeType.GenericTypeArguments, null).MethodHandle.GetMethodInfo();
// See https://github.com/dotnet/coreclr/issues/21456 for why we fast-path non-generics here (fewer allocations)
if (attributeType.IsGenericType)
{
ctor = decoratedModule.ResolveMethod(caRecord.tkCtor, attributeType.GenericTypeArguments, null).MethodHandle.GetMethodInfo();
}
else
{
ctor = ModuleHandle.ResolveMethodHandleInternal(decoratedModule.GetNativeHandle(), caRecord.tkCtor);
}
}
else
{
Expand Down

0 comments on commit 1c43264

Please sign in to comment.