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

Prevent predicates for conditional registrations from running twice #346

Closed
dotnetjunkie opened this Issue Dec 13, 2016 · 4 comments

Comments

2 participants
@dotnetjunkie
Collaborator

dotnetjunkie commented Dec 13, 2016

Deep down in the Registration class, during the generation of a NewExpression, the class causes the InstanceProducer instance for each dependency twice.

The first request is through the IDependencyInjectionBehavior.BuildExpression interface and is used to build up the actual expression tree. The BuildExpression implemention than calls back into the container to call the Container's internal GetRegistrationEvenIfInvalid method.

The second request when the Registration builds up the list of 'known relationships'. It does this by calling (again) into the GetRegistrationEvenIfInvalid method.

Requesting such InstanceProducer twice normally has no observable effect to the user, except when the dependency is a conditional registration. In that case the registration's predicate will be called twice, with the exact same context.

Although the performance penalty is usually low, it is confusing and counter intuitive that the library allways calls the predicate least twice for every consumer the dependency is injected into.

This unfortunate behavior is caused by the way Simple Injector has evolved over time. Simple Injector's diagnostic abilities (that use known relationships) did not exist during the time that the XXXBehavior strategies where added.

Since IDependencyInjectionBehavior is an important extension point through which users can change the way dependencies are constructed, the use of IDependencyInjectionBehavior can't be removed from Registration. Likewise does Registration need to get the InstanceProducer dependencies to build the list of known types; the diagnostic system completely relies on this. The only way to fix this issue is to make a breaking change to the IDependencyInjectionBehavior interface to let it return the InstanceProducer. There are a few ways this can be done, depending on the level of change we allow to be made:

  • Add an out parameter of type InstanceProducer to the IDependencyInjectionBehavior.BuildExpression method.
  • Change the return value of IDependencyInjectionBehavior.BuildExpression to be a tuple of some sort that contains both the Expression and the InstanceProducer.
  • Rename the IDependencyInjectionBehavior.BuildExpression to GetProducer and let it return the InstanceProducer.

Every option has its pro's and con's. The third option will for instance require the creation of Expression based InstanceProducers to be simplified, since users now need to return a full InstanceProducer instead of just an Expression.

@dotnetjunkie dotnetjunkie added this to the v4 milestone Dec 13, 2016

@dotnetjunkie dotnetjunkie changed the title from Return InstanceProducer from IDependencyInjectionBehavior.BuildExpression to Prevent predicates for conditional registrations from running twice Dec 18, 2016

dotnetjunkie added a commit that referenced this issue Dec 21, 2016

Made required structural changes
Made required structural changes to make it possible to call the
conditional predicate just once (Related to #346)

dotnetjunkie added a commit that referenced this issue Dec 21, 2016

Small cleanups.
(related to #346)

@dotnetjunkie dotnetjunkie self-assigned this Dec 28, 2016

@YehudahA

This comment has been minimized.

YehudahA commented Apr 2, 2017

@dotnetjunkie

This comment has been minimized.

Collaborator

dotnetjunkie commented Apr 2, 2017

@YehudahA, could you provide me with some more context of why you think that article needs to be updated?

@YehudahA

This comment has been minimized.

YehudahA commented Apr 2, 2017

See the implementation of IDependencyInjectionBehavior.

public Expression BuildExpression(InjectionConsumerInfo consumer) 
{
    return this.convention.CanResolve(consumer.Target)
            ? this.convention.BuildExpression(consumer)
            : this.decoratee.BuildExpression(consumer);
}
@dotnetjunkie

This comment has been minimized.

Collaborator

dotnetjunkie commented Apr 2, 2017

Now I see. You are correct, the definition of the IDependencyInjectionBehavior has changed in Simple Injector v4. You should replace that code snippet with the following:

public InstanceProducer GetInstanceProducer(InjectionConsumerInfo consumer, bool throwOnFailure)
{
    if (!this.convention.CanResolve(consumer.Target))
    {
        return this.decorated.GetInstanceProducer(consumer, throwOnFailure);
    }

    return InstanceProducer.FromExpression(
        serviceType: consumer.Target.TargetType,
        expression: this.convention.BuildExpression(consumer),
        container: this.container);
}

For the full code sample, please take a look here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment