Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add instantiation binding interception and use it for proxies #28127

Merged
merged 5 commits into from
Jun 1, 2022

Conversation

ajcvickers
Copy link
Member

Part of #626
Part of #15911
Fixes #20135
Fixes #14554
Fixes #24902

This is the lowest level of materialization interception--it allows the actual constructor/factory binding to be changed, such that the expression tree for the compiled delegate is altered.

Introduces singleton interceptors, which cannot be changed per context instance without re-building the internal service provider. (Note that we throw by default if this is abused and results in many service providers being created.)

The use of this for proxies has two big advantages:

/// </summary>
public virtual Func<MaterializationContext, object> GetEmptyMaterializer(
IEntityType entityType)
=> EmptyMaterializers.GetOrAdd(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be here? By caching in the entity type we can avoid the dictionary lookup

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storing it in the model means that when services change, the value in the model will still use an interceptor that is no longer configured. And since this is lazy, it means some entity types could have the interceptor, and some not.

That being said, most of the time a new service provider means a new model will be built, so in reality this won't make much difference for most uses. And we have this problem in other places already (for example, the fact that UseXxxProxies() changes the built model) but I was at least trying to be clean here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to change it, if you prefer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used in any potential hot path? Or are all usages cached anyway?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used when synthesizing join entity instances in navigation fixup. So I highly doubt it is a hot path for anything.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then leave it as it. We can always move it later

ajcvickers and others added 5 commits June 1, 2022 10:18
Part of #626
Part of #15911
Fixes #20135
Fixes #14554
Fixes #24902

This is the lowest level of materialization interception--it allows the actual constructor/factory binding to be changed, such that the expression tree for the compiled delegate is altered.

Introduces singleton interceptors, which cannot be changed per context instance without re-building the internal service provider. (Note that we throw by default if this is abused and results in many service providers being created.)

The use of this for proxies has two big advantages:
- Proxy types are created lazily, which vastly improves model building time for big models with proxies. See #20135.
- Proxies can now be used with the compiled model, since the proxy types are not compiled into the model. See
Co-authored-by: Andriy Svyryd <AndriySvyryd@users.noreply.github.com>
Co-authored-by: Andriy Svyryd <AndriySvyryd@users.noreply.github.com>
Co-authored-by: Andriy Svyryd <AndriySvyryd@users.noreply.github.com>
@ajcvickers ajcvickers force-pushed the SomeBoysTryAndSomeBoysLie0430 branch from dcb0a12 to b648d07 Compare June 1, 2022 10:07
@ajcvickers ajcvickers merged commit 13b563a into main Jun 1, 2022
@ajcvickers ajcvickers deleted the SomeBoysTryAndSomeBoysLie0430 branch June 1, 2022 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants