zzzprojects / System.Linq.Dynamic.Core

The .NET Standard / .NET Core version from the System Linq Dynamic functionality.
https://dynamic-linq.net/
Apache License 2.0
1.57k stars 227 forks source link

.NET 8 (and possible all non-NETSTANDARD and higher versions) attempt to load EF 6 types #790

Closed jnsn closed 6 months ago

jnsn commented 7 months ago

After upgrading one of our projects from .NET Core 3.1 to .NET 8 we were seeing a performance issue when using a Radzen data grid that is loaded or sorted for the first time when an entire list is directly bound to it. After some debugging I was able to narrow it to the usage of the OrderBy method in this library that is being used.

The static constructor of the PredefinedTypesHelper class attempts to load a set of types from Syste.Data.Objects and System.Data.Entity. There is a list of preprocessor directives around it which, I believe, should only be triggered in full .NET framework cases.

https://github.com/zzzprojects/System.Linq.Dynamic.Core/blob/5cf83570b4f6f2d302a6cf669d48c538a78f1967/src/System.Linq.Dynamic.Core/Parser/PredefinedTypesHelper.cs#L54-L75

Our old implementation had a .NET Standard class library loading the Radzen library, which in turn loaded System.Linq.Dynamic.Core. This resulted in the NETSTANDARD directive being set, so the entire block was discarded. Our new implementation upgraded everything to .NET 8, were the NETSTANDARD flag is not set this the entire set of types is loaded.

Due to the nature of our deployment we have a lot of assemblies in the working directory which might or might not need to be loaded depending on the configuration. So they are not necessarily already present in the AppDomain but exist on disk in the current working directory.

The Type.GetType(typeName); call will force load all the assemblies in the current folder to be loaded in the AppDomain. If there are hundreds of assemblies there, this takes quite some time and results in the performance impact we are seeing.

StefH commented 7 months ago

This is probably related to / the same as #783.

Can you try the suggestion described there?

jnsn commented 7 months ago

I initially thought so too, but my understanding is that the TypeFinder is not yet being used. When I'm following my stack trace, I seem to be getting into a path that will always do this. My experience is not the scanning of the assemblies that is taking a long time or causing the issue, it's actually forcing the load dozens of assemblies from disk which only then are attempted to be scanned for the full framework EF types.

The Radzen library calls the .OrderBy method directly: https://github.com/radzenhq/radzen-blazor/blob/421bb3b7012dd59edf2a86995f9a61b3768d4321/Radzen.Blazor/RadzenDataGrid.razor.cs#L1728

Which is here: https://github.com/zzzprojects/System.Linq.Dynamic.Core/blob/f8eddd61cf5026bdd57b0de66694e124437b1bf0/src/System.Linq.Dynamic.Core/DynamicQueryableExtensions.cs#L1495

Which then calls the InternalOrderBy method: https://github.com/zzzprojects/System.Linq.Dynamic.Core/blob/f8eddd61cf5026bdd57b0de66694e124437b1bf0/src/System.Linq.Dynamic.Core/DynamicQueryableExtensions.cs#L1551

Which is instantiating the ExpressionParser: https://github.com/zzzprojects/System.Linq.Dynamic.Core/blob/f8eddd61cf5026bdd57b0de66694e124437b1bf0/src/System.Linq.Dynamic.Core/DynamicQueryableExtensions.cs#L1575

Which is instantiating the KeywordsHelper: https://github.com/zzzprojects/System.Linq.Dynamic.Core/blob/f8eddd61cf5026bdd57b0de66694e124437b1bf0/src/System.Linq.Dynamic.Core/Parser/ExpressionParser.cs#L78

Of which the static constructor is loading the PredefinedTypesHelper: https://github.com/zzzprojects/System.Linq.Dynamic.Core/blob/f8eddd61cf5026bdd57b0de66694e124437b1bf0/src/System.Linq.Dynamic.Core/Parser/KeywordsHelper.cs#L56

It's the static constructor of the PredefinedTypesHelper that results in a force load of all those assemblies from disk.

From my understanding, the IDynamicLinqCustomTypeProvider is only used after these classes have been instantiated. The TypeFinder uses the KeywordsHelper, thus that static constructor will always be invoked: https://github.com/zzzprojects/System.Linq.Dynamic.Core/blob/f8eddd61cf5026bdd57b0de66694e124437b1bf0/src/System.Linq.Dynamic.Core/Parser/TypeFinder.cs#L24

jnsn commented 7 months ago

I had some time to further test this.

So, there are 2 things in play:

  1. The loading of the EF libraries from the PredefinedTypesHelper, which I was able to bypass by hooking into the AppDomain.CurrentDomain.AssemblyResolve event and skipping that if the requesting assembly is System.Linq.Dynamic.Core.

  2. The force loading of all assemblies by the DefaultDynamicLinqCustomTypeProvider, which I could indeed bypass by setting my custom provider on the ParserConfig.Default.CustomTypeProvider.

For that, I believe the same as with the PredefinedTypesHelper, the conditions here should be extended with something like NETCOREAPP3_1_OR_GREATER: https://github.com/zzzprojects/System.Linq.Dynamic.Core/blob/f8eddd61cf5026bdd57b0de66694e124437b1bf0/src/System.Linq.Dynamic.Core/ParsingConfig.cs#L52

So, for now I've managed worked around it.

BoudewijnPopkema commented 7 months ago

We see the exact same issue in our application (which happens to also use the Radzen DataGrid that ends up calling the .Order). Our application is quite large with a lot of dependencies, which leads to a slowdown of 16 seconds on every first query after application startup, because of the loading of the assemblies.

I'm not sure how to implement a custom type provider as was suggested, but I'd very much appreciate any fix or workaround to improve the performance as the wait is considerable.

jnsn commented 7 months ago

@BoudewijnPopkema

I've used the following implementation, which is a simplified type finder. It doesn't use anything related to the KeywordsHelper that exists in the default implementation:

public class CustomDynamicLinqTypeProvider : AbstractDynamicLinqCustomTypeProvider, IDynamicLinkCustomTypeProvider
{
    public HashSet<Type> GetCustomTypes()
        => GetCustomTypesInternal();

    public Dictionary<Type, List<MethodInfo>> GetExtensionMethods()
        => GetExtensionMethodsInternal();

    public Type? ResolveType(string typeName)
    {
        var assemblies = AppDomain.CurrentDomain.GetAssemblies();
        var type = ResolveType(assemblies, typeName);

        return type;
    }

    public Type? ResolveTypeBySimpleName(string simpleTypeName)
    {
        var assemblies = AppDomain.CurrentDomain.GetAssemblies();
        var type = ResolveTypeBySimpleName(assemblies, simpleTypeName);

        return type;
    }

    private HashSet<Type> GetCustomTypesInternal()
    {
        var assemblies = AppDomain.CurrentDomain.GetAssemblies().ToList();
        var hashSet = new HashSet<Type>(FindTypesMarkedWithDynamicLinqTypeAttribute(assemblies));

        return hashSet;
    }

    private Dictionary<Type, List<MethodInfo>> GetExtensionMethodsInternal()
    {
        var types = GetCustomTypes();

        var list = new List<Tuple<Type, MethodInfo>>();

        foreach (var type in types)
        {
            var extensionMethods = type
                .GetMethods(BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic)
                .Where(x => x.IsDefined(typeof(ExtensionAttribute), false))
                .ToList();

            extensionMethods.ForEach(x => list.Add(new Tuple<Type, MethodInfo>(x.GetParameters()[0].ParameterType, x)));
        }

        return list.GroupBy(x => x.Item1, tuple => tuple.Item2).ToDictionary(key => key.Key, methods => methods.ToList());
    }
}

Then, in my Program.cs I've set this up using the following line:

ParsingConfig.Default.CustomTypeProvider = new CustomDynamicLinqTypeProvider();
StefH commented 7 months ago

@jnsn / @BoudewijnPopkema Which project are you using?

  1. System.Linq.Dynamic.Core ? or
  2. Microsoft.EntityFrameworkCore.DynamicLinq.EFCore8 ?
BoudewijnPopkema commented 7 months ago

We are using 2. Microsoft.EntityFrameworkCore.DynamicLinq version 8.3.10

StefH commented 7 months ago

I've added some workaround logic to only load the old EF when a special EFType is found. https://github.com/zzzprojects/System.Linq.Dynamic.Core/pull/807

Would this solve your issue in .NET 8 ?

jnsn commented 7 months ago

I'm using the Radzen library, so my use of System.Linq.Dynamic.Core is implicit. At first glance, this would indeed solve the issue.

But as I mentioned earlier, some of these types are excluded using a NETSTANDARD preprocessor directive. Is there a reason these aren't excluded in anything more recent than NETSTANDARD, like NETCOREAPP3_1 and higher?

StefH commented 7 months ago

@jnsn I did use the same workaround for NETSTANDARD in that file.

Can you please provide a simple console app which shows your issue? This makes it easier for me to check if this fix solves that problem.

jnsn commented 7 months ago

@jnsn I did use the same workaround for NETSTANDARD in that file.

Maybe I'm not understanding it entirely. To me, it looks like these types should only exist in a Full Framework environment, as they are from EF 6. In .NET Core or .NET 5 and higher projects, these will never exist. Or at least never loaded from the Full Framework EF 6 libraries, but always from potential EFCore libraries, if at all.

The NETSTANDARD preprocessor directive is not set in .NET 5 or higher, so my initial idea was that the list should be extended with the NET5_0_OR_GREATER directive. This would mean that it's also excluded from any newer projects (like .NET 8).

Can you please provide a simple console app which shows your issue? This makes it easier for me to check if this fix solves that problem.

I've attached a ZIP file which contains 2 projects: linq-core-demo.zip

I've also included a build.ps1 file which will build the solution and publishes the Demo.Plugin and Demo.Host projects into a output folder. It's important to note in my scenario that the Demo.Host project never references the Demo.Plugin project.

The Demo.Host project can then be run from the output folder, which produces the following output:

PS E:\jnsn\linq-core-demo\output> dotnet Demo.Host.dll
11:50:57.127 [Warning] Attempted to resolve assembly "System.Data.Entity, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089" by "System.Linq.Dynamic.Core"
11:50:57.145 [Warning] Attempted to resolve assembly "System.Data.Entity, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089" by "System.Linq.Dynamic.Core"
11:50:57.146 [Warning] Attempted to resolve assembly "System.Data.Entity, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089" by "System.Linq.Dynamic.Core"
11:50:57.147 [Warning] Attempted to resolve assembly "EntityFramework, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089" by "System.Linq.Dynamic.Core"
11:50:57.148 [Warning] Attempted to resolve assembly "EntityFramework, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089" by "System.Linq.Dynamic.Core"
11:50:57.148 [Warning] Attempted to resolve assembly "EntityFramework, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089" by "System.Linq.Dynamic.Core"
11:50:57.149 [Warning] Attempted to resolve assembly "EntityFramework.SqlServer, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089" by "System.Linq.Dynamic.Core"
11:50:57.150 [Warning] Attempted to resolve assembly "EntityFramework.SqlServer, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089" by "System.Linq.Dynamic.Core"
11:50:57.176 [Warning] Attempted to resolve assembly "Demo.Plugin, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null" by "System.Private.CoreLib"
11:50:57.252 [Information] Found 1 customers: [Customer {Id=0de3c65d-d6ee-4289-9aec-1eba2c8fc46d, Orders=[Order {Id=6acedbef-9570-4f99-a3de-27a51a577033}, Order {Id=9f464cc5-8c9a-455f-8a3d-ec33d9bab75d}, Order {Id=632afba1-387c-406a-aa7a-891d05e65af3}, Order {Id=f5cc1f15-0058-470a-bb8c-3161e49843de}]}]

As you can see from the output, all the EF types are attempted to be resolved from by System.Linq.Dynamic.Core. This isn't a major problem. They are not found, I log them and the program continues. So these are to me not really the issue.

The main issue is the attempted load of the Demo.Plugin assembly by System.Private.CoreLib, which is triggered by the DefaultDynamicLinqCustomTypeProvider, as I mentioned in an earlier reply. That's the case that should be avoided.

StefH commented 7 months ago

:one:

I did change the code for PredefinedTypesHelper so that it checks for

static PredefinedTypesHelper()
    {
        if (Type.GetType("EntityFramework.DynamicLinq.EFType, EntityFramework.DynamicLinq") != null)
        {
            TryAdd("System.Data.Objects.EntityFunctions, System.Data.Entity, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089", 1);
            TryAdd("System.Data.Objects.SqlClient.SqlFunctions, System.Data.Entity, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089", 1);
            TryAdd("System.Data.Objects.SqlClient.SqlSpatialFunctions, System.Data.Entity, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089", 1);
            TryAdd("System.Data.Entity.Core.Objects.EntityFunctions, EntityFramework, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089", 2);
            TryAdd("System.Data.Entity.DbFunctions, EntityFramework, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089", 2);
            TryAdd("System.Data.Entity.Spatial.DbGeography, EntityFramework, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089", 2);
            TryAdd("System.Data.Entity.SqlServer.SqlFunctions, EntityFramework.SqlServer, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089", 2);
            TryAdd("System.Data.Entity.SqlServer.SqlSpatialFunctions, EntityFramework.SqlServer, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089", 2);
        }

        if (Type.GetType($"Microsoft.EntityFrameworkCore.DynamicLinq.EFType, Microsoft.EntityFrameworkCore.DynamicLinq, Version={Version}, Culture=neutral, PublicKeyToken={PublicKeyToken}") != null)
        {
            TryAdd($"Microsoft.EntityFrameworkCore.DynamicLinq.DynamicFunctions, Microsoft.EntityFrameworkCore.DynamicLinq, Version={Version}, Culture=neutral, PublicKeyToken={PublicKeyToken}", 3);
        }
    }

Which results now in:

12:54:46.910 [Warning] Attempted to resolve assembly "EntityFramework.DynamicLinq, Culture=neutral, PublicKeyToken=null" by "System.Linq.Dynamic.Core"
12:54:46.944 [Warning] Attempted to resolve assembly "Microsoft.EntityFrameworkCore.DynamicLinq, Version=1.3.13.0, Culture=neutral, PublicKeyToken=974e7e1b462f3693" by "System.Linq.Dynamic.Core"
12:54:47.006 [Warning] Attempted to resolve assembly "Demo.Plugin, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null" by "System.Private.CoreLib"

(Maybe this is not yet perfect, but it's a workaround)

:two:

Note that the last call is done via the DefaultAssemblyHelper which loads indeed all .dll files from the same folder where the main program is running. So in this case it also finds the Demo.Plugin.dll file

Loading from these extra files could be made configurable via the Config. So by default it will not load extra files which are not referenced by the main program.

jnsn commented 6 months ago

I don't think that will solve the issue for me, as nothing changes in the behavior that I'm trying to avoid. It's still loading in the hundreds of Demo.PluginX DLL's that are in my folder.

From my testing, I believe the problem is that the Type.GetType() call force loads all the assemblies that were not yet loaded from disk into the AppDomain, and that should not happen.

I still believe the issue is located here: https://github.com/zzzprojects/System.Linq.Dynamic.Core/blob/f8eddd61cf5026bdd57b0de66694e124437b1bf0/src/System.Linq.Dynamic.Core/ParsingConfig.cs#L48-L58

As the comment on line 53 suggest, that should only be done in case of the full framework. I believe the fix is to add the NET5_0_OR_GREATER directive to line 52.

StefH commented 6 months ago

A]

I've added if NET452_OR_GREATER || NETSTANDARD2_1 to PredefinedTypesHelper.cs to only load the older EntityFramework types for old NET-framework and standard2.1 (because EntityFramework can also be used for netstandard 2.1).

See: https://github.com/zzzprojects/System.Linq.Dynamic.Core/blob/da82150b984cd43616c54263b5bf50906d570db0/src/System.Linq.Dynamic.Core/Parser/PredefinedTypesHelper.cs#L64

B]

Adding NET5_0_OR_GREATER to line 52 will have a side-effect that no default CustomTypeProvider is loaded, which means that types with the [DynamicLinqTypeAttribute] will not be found anymore, which breaks current behavior.

C]

I don't think that will solve the issue for me, as nothing changes in the behavior that I'm trying to avoid. It's still loading in the hundreds of Demo.PluginX DLL's that are in my folder.

I've implemented option :two: by adding a extra config setting:

        /// <summary>
        /// Load additional assemblies from the current domain base directory.
        ///
        /// Note: only used when full .NET Framework and .NET Core App 2.x and higher.
        ///
        /// Default value is <c>false</c>.
        /// </summary>
        public bool LoadAdditionalAssembliesFromCurrentDomainBaseDirectory { get; set; }

Default it's false so the Demo.PluginX DLL's are not loaded. When set to true, the Demo.PluginX DLL's in that folder are loaded.

See logging from that demo-app:

11:20:29.245 [Information] --- LoadAdditionalAssembliesFromCurrentDomainBaseDirectory = False ---
11:20:29.284 [Warning] Attempted to resolve assembly "Microsoft.EntityFrameworkCore.DynamicLinq, Version=1.3.14.0, Culture=neutral, PublicKeyToken=974e7e1b462f3693" by "System.Linq.Dynamic.Core"
11:20:29.390 [Information] Found 1 customers: ["Customer { Id = 2ff7ca04-0753-4b50-bd40-a99342cfb3af, Orders = System.Collections.Generic.List`1[Order] }"]
11:20:29.394 [Information] ********************************************************************************
11:20:29.395 [Information] --- LoadAdditionalAssembliesFromCurrentDomainBaseDirectory = True ---
11:20:29.422 [Warning] Attempted to resolve assembly "Demo.Plugin, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null" by "System.Private.CoreLib"
11:20:29.453 [Information] Found 1 customers: ["Customer { Id = 2ff7ca04-0753-4b50-bd40-a99342cfb3af, Orders = System.Collections.Generic.List`1[Order] }"]
StefH commented 6 months ago

@jnsn Did you have time to take a look at my comment?

jnsn commented 6 months ago

Hi,

Sorry for the late reply.

This indeed looks like it would fix the issue for me. Thanks!