Runtime: Performance Regression: GetCustomAttributes() Allocations Increased from netcoreapp2.1

Created on 9 Dec 2018  路  4Comments  路  Source: dotnet/runtime

I'm not sure how much time I'll have to dig here, so filing an issue up front:
Allocations have increased and performance has regressed on certain Attribute paths sometime between netcoreapp2.1 (2.1.6) and netcoreapp3.0. I was adding the benchmark set I created for dotnet/coreclr#20779 and dotnet/coreclr#20795 to the new dotnet/performance repo in https://github.com/dotnet/performance/pull/177 and too illustrate the utility, I was getting a screenshot for a tweet and...hey I found regressions. So this whole dotnet/performance thing is already paying off!

Here's a run comparing net472, netcoreapp2.1 (2.1.6) and netcoreapp3.0 (3.0.0-preview-27122-01 specifically):
image

Note the performance and allocation regressions specifically in these 4 benchmarks:

To reproduce this, in the dotnet/performance repo:

C:\git\NickCraver\performance\src\benchmarks\micro (master)
位 dotnet run -c Release -f netcoreapp3.0 -- --runtimes netcoreapp2.1 netcoreapp3.0 --filter System.Reflection.Attributes.GetCustom*

You can see in the initial benchmarks of dotnet/coreclr#20779 that the regression predates that PR, but that's the only date bound I have at the moment - we'll need to dig into history here and see what happened. Luckily, benchmarks in dotnet/performance being used for regression testing should prevent this sort of thing in the future and that's awesome. Kudos to @adamsitnik for putting that together...I never would have seen this otherwise.

cc @benaadams @danmosemsft @jkotas @stephentoub

area-System.Reflection tenet-performance

Most helpful comment

Closing out - fixed via dotnet/coreclr#21462 :)

Thanks everyone!

All 4 comments

This is regression introduce by https://github.com/dotnet/coreclr/pull/9189 . Bulk of the regression is in FilterCustomAttributeRecord method.

This change modified ModuleHandle.ResolveMethodHandleInternal(decoratedModule.GetNativeHandle(), caRecord.tkCtor) to much more expensive ctor = decoratedModule.ResolveMethod(caRecord.tkCtor, attributeType.GenericTypeArguments, null).MethodHandle.GetMethodInfo().

@AviAvni Is it something you can take a look at?

I opened up a PR that short-circuits out of the costly path generics currently need in https://github.com/dotnet/coreclr/pull/21462 - hopefully that's an acceptable quick improvement, though ResolveMethod() can still get a lot of love.

@jkotas the fix look great thanks @NickCraver

Closing out - fixed via dotnet/coreclr#21462 :)

Thanks everyone!

Was this page helpful?
0 / 5 - 0 ratings