Disable injection of LazyLoader in some situations

[Charles113]

Hello,

var ctx = new DbContextSG(options);
ctx.ChangeTracker.AutoDetectChangesEnabled = false;

// should this disabled the creation of LazyLoader() ?
ctx.ChangeTracker.LazyLoadingEnabled = false;

public class Customer {

    public DbContext DbContext { get; set; } // has only minor performance impact
	
    public ILazyLoader LazyLoader { get; set; } // has Huge performance impact

}


for (int i = 0; i < 100_000; i++)
{
    var c1 = new Customer();
    ctx.Customers.Add(c1); // attaches entitiy to the context in state "Added", also creates and injects all Services
    
    // side note: `ctx.AddRangeAsync()` does not do a batch action, it's slow and Synchronous, the name Async is confusing. 
    
}

A few things i've noticed: Injecting the ILazyLoader is slow.
Around 30% slower than not having the "ILazyLoader LazyLoader" property in my customer object.
The 100k loop takes: 1300ms vs 1000ms without "ILazyLoader LazyLoader".

The LazyLoader is created and injected for new Entities.
The LazyLoader is created and injected for all Entities of all Queries.
The LazyLoader is created and injected if "ChangeTracker.LazyLoadingEnabled = false"

Is this the desired behaviour?

I feel like we can save some resources here if we don't create and inject 100k of LazyLoader() that are not needed.

Are LazyLoader objects ever needed for new entities?
I would love to disable the creation of LazyLoader objects if i know that i don't need LazyLoaders for a query.

Best Regards

[roji]

@Charles113 while "30% slower" sounds like a lot, you're reporting a 300ms perf diff for 100k loop iterations, i.e. 3 microseconds per iteration. That's negligible in all but the most high-performance scenarios, and in addition, lazy loading generally isn't considered a high-performance technique (for other reasons). So I'm not sure I see a problem here.

[Charles113]

@Charles113 while "30% slower" sounds like a lot, you're reporting a 300ms perf diff for 100k loop iterations, i.e. 3 microseconds per iteration. That's negligible in all but the most high-performance scenarios, and in addition, lazy loading generally isn't considered a high-performance technique (for other reasons). So I'm not sure I see a problem here.

What do you mean? Why do we have to waste resources?
Yes i don't want lazy loading, but our project is too big to ensure that devs never forget to "include" everything.
So lazy loading is a necessary evil for us.

Class InternalEntityEntry

public void SetIsLoaded(INavigationBase navigation, bool loaded = true)
{
   var lazyLoader = GetLazyLoader();
   if (lazyLoader != null)
   {
	   lazyLoader.SetLoaded(Entity, navigation.Name, loaded);
   }
   else
   {
	   _stateData.FlagProperty(navigation.GetIndex(), PropertyFlag.IsLoaded, isFlagged: loaded);
   }
}

private ILazyLoader? GetLazyLoader()
{
   if (!EntityType.HasServiceProperties())
   {
	   return null;
   }

   var lazyLoaderProperty = EntityType.GetServiceProperties().FirstOrDefault(p => p.ClrType == typeof(ILazyLoader));
   return lazyLoaderProperty != null ? (ILazyLoader?)this[lazyLoaderProperty] : null;
}

Problem here "GetLazyLoader()" only looks in ServiceProperties for the lazy loader.
It would be cool if we could register our own property for the lazy loader,
or override GetLazyLoader().

Then i could decide myself when i want to instantiate the LazyLoader().

I tried creating my own property for my custom lazy loader, but then "lazyLoader.SetLoaded" is not automatically called.
So the framework is unaware of my custom Property.

And the Service Property "ILazyLoader LazyLoader" is always instantiated, so i'm looking for alternatives.

[roji]

lazy loading generally isn't considered a high-performance technique (for other reasons).

My point here is this: if you're using lazy loading, you're already doing additional network roundtrips to lazy load stuff that could have been loaded eagerly, in the same roundtrip as the original query. Such extra roundtrips represents orders of magnitude more slowing down of your application compared to this single microsecond delay; so you've already opted into making your application slower to such a degree, that I don't think worrying about this makes sense.

To summarize, if I'm understanding the above, It seems like you're asking us to do what will likely be a non-trivial amount of work, in order to shave off a single microsecond (let me know if that's incorrect). If that's the case, it's unlikely we'd spend the time to do this; it's always possible to micro-optimize any piece of code - the question is always whether it's worth it. And in this case, I'm not sure how the answer would be yes.

[Charles113]

lazy loading generally isn't considered a high-performance technique (for other reasons).

My point here is this: if you're using lazy loading, you're already doing additional network roundtrips to lazy load stuff that could have been loaded eagerly, in the same roundtrip as the original query. Such extra roundtrips represents orders of magnitude more slowing down of your application compared to this single microsecond delay; so you've already opted into making your application slower to such a degree, that I don't think worrying about this makes sense.

To summarize, if I'm understanding the above, It seems like you're asking us to do what will likely be a non-trivial amount of work, in order to shave off a single microsecond (let me know if that's incorrect). If that's the case, it's unlikely we'd spend the time to do this; it's always possible to micro-optimize any piece of code - the question is always whether it's worth it. And in this case, I'm not sure how the answer would be yes.

I need something to ensure that all the properties that we access in an entity tree are loaded.
The goal is to never use lazy loading, but it's not that simple in a big project.
The default behavior of just returning null or an empty collection for navigation props can easily cause bugs.

300ms for 100k records is too much of an overhead. This would affect all queries.
Especially if i know that a query doesn't need any lazy loaders.

What alternative to lazy loading can you suggest?

[roji]

The goal is to never use lazy loading, but it's not that simple in a big project.

Lots of big projects using EF don't use lazy loading, but if you decide you need to use it, that's of course perfectly fine. But as I wrote above, if you do that, you're opting into lots of extra database roundtrips, which generally represent a much worse performance hit than what we're discussing here. In addition, since lazy loading doesn't support async I/O, you're limiting your application's scalability by using sync I/O.

To summarize, if you're concerned about performance to an extent that 300ms for 100k records is too much for you, you really want to get rid of lazy loading because the other - unrelated - perf disadvantages of lazy loading represent considerably more of a problem than what we're discussing here.

[Charles113]

Lots of big projects using EF don't use lazy loading, but if you decide you need to use it, that's of course perfectly fine. But as I wrote above, if you do that, you're opting into lots of extra database roundtrips, which generally represent a much worse performance hit than what we're discussing here. In addition, since lazy loading doesn't support async I/O, you're limiting your application's scalability by using sync I/O.

No worries, the goal is to eliminate all lazy loading sooner or later. But i can't migrate the project to EF core without some safeguards like lazy loading.

TBH i'm not sure why injecting the Lazy Loader is so slow.
I tried another query and ILazyLoader LazyLoader increases the query load time from 130ms to 180ms.

That's 25% to 30% slower just because of LazyLoader injection?!

I think i've found a solution that im 99% happy with.
Basically i no longer use the injected LazyLoader().

I have created a custom LazyLoader in my Base Entity.
It's no longer a ServiceProperty and InternalEntityEntry.GetLazyLoader() returns nothing.

var lazyLoader = obj.GetLazyLoader(); // don't create LazyLoader under some circumstances

if (lazyLoader == null)
   return;

// fast path to check if prop is loaded
if (lazyLoader.IsLoaded(obj, propName))
   return;

// more secure way to check for loaded, slower
var isLoaded = ctx.Entry(obj).Reference(propName).IsLoaded;

if (isLoaded)
{
   // save the fast path
   lazyLoader.SetLoaded(obj, propName, loaded: true);
   return;
}

And then later in LazyLoader.Load()

try
{
	 entry.Load(behav); // sets entry.IsLoaded = true;
	 
	 // manually call LazyLoader.SetLoaded()
	 // SetLoaded() is automatically called if we use an injected LazyLoader
	 this.SetLoaded(entity, navigationName, loaded: true);
}
catch
{
	 entry.IsLoaded = false;
	 this.SetLoaded(entity, navigationName, loaded: false);

	 throw;
}

Not sure if i have thought of all cases.
I can now create the lazy loader only on demand and it no longer wastes resources.

EDIT:
I would prefer to hook into InternalEntityEntry.SetIsLoaded() tho or into InternalEntityEntry.GetLazyLoader()
I can't easily mark the properties as loaded without those internal APIs.

[roji]

@ajcvickers leaving to you to decide if we should do something here.