MR.EntityFrameworkCore.KeysetPagination icon indicating copy to clipboard operation
MR.EntityFrameworkCore.KeysetPagination copied to clipboard

Consider support for source generators

Open mpetito opened this issue 1 year ago • 3 comments

There is a fair amount of runtime expression manipulation handled by this library that might benefit from using a source generator at compile time. As I'm integrating this into a keyset paginated API, some benefits I could see from a source generated version:

  • Faster initialization time, especially in serverless environments.
  • Generate a serializable key type that corresponds to the columns required for the reference object, so that it's easier to provide pagination tokens to clients.
  • Aligns with the .NET 8 AOT performance goals as the EF Core team also investigates support for AOT.

It may also be reasonable to generate both IQueryable and IEnumerable versions using source generation to resolve #12, or determine if EFCore Async methods are available for IQueryable to resolve #32.

mpetito avatar Jan 18 '24 14:01 mpetito

At the time, my opinion was that since this is built on top of an ORM, any performance gains from using source generation will be negligible compared to the database query that will take place as part of calling this. But you make a good point about supporting AOT. And this library is about performance too after all.

Also, generating bespoke methods to build the keyset might greatly decrease allocations as right now we box to object every time we obtain a column's value if it's a value type (and most columns in keysets are usually value types).

One problem would be generating the code to obtain the column values of a reference, due to loose typing this can be any object. Would need to find a good way or an alternative to remove reflection here.

This will likely require a new major version as I suspect some breaking changes, but that's fine.

As for pagination tokens. The way this library works now, it's up to the consumer how a reference is obtained. Usually, an id is sent between the front and the back. But I could see some code generation giving more flexibility to what you're allowed to provide instead of this reference. i.e. Instead of needing an object reference, it could be a record of the keyset as an alternative, which would allow a frontend to send the keyset values instead of an id, which would remove the need for a db query to load the reference. Not sure if this is similar to what you meant by serializable key.

In any case, this will be a major rewrite but would love to do this eventually.

mrahhal avatar Jan 18 '24 19:01 mrahhal

it could be a record of the keyset as an alternative, which would allow a frontend to send the keyset values instead of an id, which would remove the need for a db query to load the reference. Not sure if this is similar to what you meant by serializable key.

Yes, this is exactly what I'd love to see, as the lookup introduces an extra db call and is the source of a potential bug when that record has been removed in between page navigations.

One problem would be generating the code to obtain the column values of a reference, due to loose typing

Presumably the reference could be of the generic type corresponding to the query or the record of the keyset type. For other arbitrary types, you could require the caller maps their reference object to the keyset record type. Or, since the DTO pattern is common here, you could potentially name additional types during specification of the sort which would generate typed overloads that perform that mapping for you.

mpetito avatar Jan 18 '24 20:01 mpetito

Agreed. Mapping is less ergonomic (keysetContext.HasNextAsync(data.Select(x => ...))), but could be a good base option. An alternative of providing the additional types to be generated might be a good second option as these should be limited.

mrahhal avatar Jan 18 '24 21:01 mrahhal