AutoFixture icon indicating copy to clipboard operation
AutoFixture copied to clipboard

🐛 Auto properties on large record types causes severe performance degradation

Open putnap opened this issue 2 years ago • 4 comments

Describe the Bug

It looks like record type with constructor is a cause for performance degradation, which can be worked around by using OmitAutoProperties() or by customizing each property using Without().

Scenario

In our project we are using rather wide and deeply nested structure as our main DTO and we are using xUnit, AutoFixture and Moq to write our tests. We have about 120 unit tests a lot of them create this large DTO using AutoMoqDataAttribute. The suite of tests in our CI consistently run for around 25 minutes, which is horrible. I started exploring why that is and I had a strong suspicion that our large DTO is causing this, because tests without it completed very quickly, I tried then tried reducing the amount of auto properties on the structure to only header values and tests started to run quick, but I still had all the values automatically generated. I was happy I found a workaround that helps us in significant way, but at the same time I am concerned why this is happening.

I've created a repository which has minimum setup needed to reproduce the issue and basic explanation is this:

  • Record types are created by using it's constructor
  • Customizations that use OmitAutoProperties() or Without() don't work on constructors for record types
  • Even by omitting auto properties on record type, the record still has all properties set (assuming via constructor)
  • When omitting Auto properties in our setup tests started executing at least ~10 times faster (sample project executes tests ~40 times faster)

I've created a customization that checks if property is record type property by assuming that it has CompilerGeneratedAttribute.

Expected Behavior

By using OmitAutoProperties() or Without() it's expected that record type will not have a value automatically set.

Tasks

  • [ ] Explain what is going on
  • [ ] Updated all AutoFixture and related libraries
  • [ ] Confirmed in support forums if behavior is expected

Immediate question

I am fine having this workaround for now, but maybe someone can confirm that it won't cause any other issues for running tests?

Thanks!

putnap avatar Dec 21 '23 10:12 putnap

Hi @putnap, Thank you for raising this issue. What I think the issue is, it's that records create classes that do not properly encapsulate data, when looked at from the perspective of reflection (see below).

AutoFixture currently looks at the list of constructors takes the least demanding public constructor and uses it to initialize the data, then it looks at your other members and assumes that since you exposed a public setter you need it initialized from outside the constructor, so it does just that. This is why you're observing the behavior that you described.

I'm currently working on an API for easier customization of record-like structures (customizing constructor parameters, selecting constructors, etc), but it's still in early development. Feature requests like this have been raised before, but as far as I remember this wasn't implemented before because there is no static way to define constructor parameters like you would with members (ie. .With(x => x.Member, "MemberValue") ).

// Example record
record Customer(string FirstName, string LastName);

// Equivalent class generated in IL
class Customer
{
    public Customer(string FirstName, string LastName)
    {
        this.FirstName = FirstName;
        this.LastName = LastName;
    }

    public string FirstName { get; set; }
    public string FirstName { get; set; }
}

aivascu avatar Dec 21 '23 10:12 aivascu

Thanks for a quick reply!

I guess there are two topics - customizing via record constructors feature and performance issues. Right now I want to understand why performance hit is so severe.

Also, I believe that class you mention is not equivalent, since record types are immutable by default and generated properties don't have public setters, only init setters.

    public string FirstName { get; init; }
    public string LastName { get; init; }

And I would like some peace of mind that my ICustomization with CompilerGeneratedAttribute filter in this file is a good enough general workaround for now. I don't fully grasp all repercussions of using new OmitSpecimen(); so dramatically.

putnap avatar Dec 21 '23 10:12 putnap

About the init setters you're kinda correct. It does generate the equivalent IL for init for positional records, however the difference between a public setter and init is an attribute set on the return value of the setter method, which AutoFixture at the moment doesn't know exists. Everything else in terms of accessibility modifiers is exactly the same as a public setter.

This said I don't think the customization in your sample is doing anything else than ignoring all properties. A more correct way to ignore all redundant property setter is to explicitly account for them.

Here is a gist that shows an example, how to ignore specifically those properties that are init only and constructor initialized https://gist.github.com/aivascu/739f2a845eac587ca4df11cd9c5f7d31

Expand for IL examples
// usual public setter
  .property instance string Name()
  {
    .get instance string TestProject.ClassA::get_Name()
    .set instance void TestProject.ClassA::set_Name(string)
  } // end of property ClassA::Name

// positional record property
  .property instance string Name()
  {
    .get instance string TestProject.ClassB::get_Name()
    .set instance void modreq ([System.Runtime]System.Runtime.CompilerServices.IsExternalInit) TestProject.ClassB::set_Name(string)
  } // end of property ClassB::Name

// init-only property
  .property instance string Name()
  {
    .get instance string TestProject.ClassC::get_Name()
    .set instance void modreq ([System.Runtime]System.Runtime.CompilerServices.IsExternalInit) TestProject.ClassC::set_Name(string)
  } // end of property ClassC::Name

aivascu avatar Dec 21 '23 12:12 aivascu

Thanks, I'll use the workaround you suggested, seems to work!

putnap avatar Dec 21 '23 13:12 putnap