Eliminate array allocations in CustomAttribute.IsDefined() overloads

[jack-pappas]

Implemented struct enumerable and corresponding enumerator wrapping MetadataImport and MetadataEnumResult.

Replaced CustomAttributeData.GetCustomAttributeRecords(RuntimeModule, int) with a new method returning a value of the new struct enumerable type (CustomAttributeRecordCollection); modified call sites of the old method to use the new method and structs instead.

These changes eliminate the short-lived CustomAttributeRecord[] allocations which currently result from calling any of the CustomAttribute.IsDefined() overloads.

[RussKeldorph]

@dotnet-bot test Ubuntu x64 Release Build and Test

[RussKeldorph]

@dotnet-bot test OSX x64 Release Build and Test

[jack-pappas]

@ellismg I've updated the PR per your comments. Uses of the var keyword have been removed, and I've modified the way the empty array case is handled within CustomAttributeData.GetCustomAttributes(RuntimeModule,int) so the type of the returned instance will be consistent with the non-empty array case.

[jack-pappas]

Also -- I signed a CLA several months ago, why does @dnfclas think I haven't? Just to be certain, I signed the CLA again and have received the approval email (from the CLA admin).

[dnfclas]

@jack-pappas, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

[jack-pappas]

Please disregard my last comment -- turns out I'd signed the Microsoft CLA before, not the new .NET Foundation CLA.

[jack-pappas]

@ellismg Is there anything else that needs to be done for this PR before it can be merged?

cc @jkotas

[jack-pappas]

@stephentoub Would you mind taking a look at this? I believe it's ready to be merged.

[jkotas]

Could you please shared any numbers on what it is improving? (see discussion in #3849 on what I am looking for)

[benaadams]

I applied this to #6645 and the performance regressed slightly :(

[Petermarcu]

So where do we sit on this one? @jack-pappas , can you provide some concrete numbers showing the benefits realized by the change? If there is no forward motion with the PR, we should close it until we're ready to actively work through the data and finish it up.

[jack-pappas]

I'll run some benchmarks this weekend to try to get some results so we can make a decision one way or the other on this. I also want to poke around a bit to try to figure out what might've caused the code to be slower when @benadams tried it.

[jkotas]

Closing old inactive PRs. @jack-pappas please feel free to resubmit if you get a chance to finish this. Thanks!

[benaadams]

It doesn't currently allocate for enum ToString anymore so that might have been why I saw a regression (uses Array.Empty)