Reduce nullable and array reflection and code generation #3813
Conversation
|
Just pushed another commit completely splitting ArrayTypeInfo and ListTypeInfo. We only access ListTypeInfo after any checks on ArrayTypeInfo do not match. This enables array reading for aot without reflection, for example dotnet/aspnetcore#31561. EDIT: It would be good to actually tests this. |
roji
left a comment
roji
left a comment
There was a problem hiding this comment.
Here's a partial review... Just came across a MakeGenericType in the new code, is there some reason that's OK?
|
I tested this against NativeAOT, you can read some of my findings below. As it stands, due to missing ado.net apis and our own dynamic lookup of handlers an illink descriptor file or rd.xml is a necessity. Testing this immediately prompted me to move some of the classes a bit up in the hiarchy, all the nesting really isn't nice. But for an example of what it would look like after the latest commit see: <Directives xmlns="http://schemas.microsoft.com/netfx/2013/01/metadata">
<Application>
<Assembly Name="Npgsql">
<Type Name="Npgsql.Internal.TypeHandling.NullableHandler`2[[System.Nullable`1[[System.Int64, System.Runtime]], System.Runtime],[System.Int64, System.Runtime]]" Dynamic="Required All" />
<Type Name="Npgsql.Internal.TypeHandlers.ArrayHandler`2[[System.Nullable`1[[System.Int64, System.Runtime]][], System.Runtime],[System.Nullable`1[[System.Int64, System.Runtime]], System.Runtime]]" Dynamic="Required All" />
</Assembly>
</Application>
</Directives>I had hoped we could keep these entries at using System;
using Npgsql;
await using var conn = new NpgsqlConnection("Server=localhost;User ID=postgres;Password=postgres123;Database=postgres");
await conn.OpenAsync();
await using var reader = await new NpgsqlCommand("SELECT '{1,2,3}'::bigint[]", conn).ExecuteReaderAsync();
while (await reader.ReadAsync())
{
var value = reader.GetFieldValue<long?[]>(0);
Console.WriteLine(string.Join(", ", value));
}Next up I tried to force so this is an important one to tackle in a nice way. After doing that link trimming 'just worked and for my RID osx-x64 I got a 100MB executable out (as opposed to ~150MB). Stripping the binary of debug info moved it down to a 'respectable' 44MB and playing with the feature switches https://docs.microsoft.com/en-us/dotnet/core/deploying/trimming-options#trimming-framework-library-features brought another ~5/3MB respectively. Contrast this with a hello world app which takes up around 10MB and 5MB stripped, or even 3.5MB for reflection free and 2MB stripped. If you're determined you can get Npgsql working with NativeAOT. We're quite far out before reflection free mode is really an option though and regardless of which mode you try we have improvements to make before it feels like it mostly just works. The changes here are strictly an improvement even though they can't solve the problem of missing apis. It's much easier to opt into a few types you need, there is no unnecessary bloat from Linq.Expression use and the error messages if you run into a type that you missed are ok. (though the linked configurator shows misleading 'dotnet native' type name syntax ... I should probably make a PR to change those links) An example error:
All the warnings for Npgsql from ilc when referenced in a simple aot console app. |
|
Thanks @NinoFloris, this is awesome work. We obviously have some refactoring work to do before things work seamlessly/out-of-the-box without any hacks. FYI Opened #3815 to track the DefaultTypeHandlerFactory stuff and linked from #3300. |
NinoFloris
changed the title
NinoFloris
force-pushed
the
enhancement/no-codegen
branch
from
a9b670a to
45647a1
Compare
|
@NinoFloris this is still waiting on you right? |
|
Any plans on picking this up for 7.0 @NinoFloris? Not that I have lots of time to review/discuss right at this moment, but just wondering. |
…h only ships with array reflection
# Conflicts: # src/Npgsql/Internal/TypeHandlers/ArrayHandler.cs
NinoFloris
force-pushed
the
enhancement/no-codegen
branch
from
45647a1 to
78568ac
Compare
2 participants
Tadah, as discussed.
Checks off
Array reader delegatesin #3300 and I also threw in reflectionless nullable for free 😄I searched the codebase for
System.ReflectionandSystem.Linq.Expressionsand the only ones left are for composites. Seems unavoidable, though we could expose some apis for users to hand npgsql the required composite code as an alternative to reflection.