AutoFactory: Add a way to inject directly, without Provider

[anuraaga]

Currently it's only possible for factory parameters to be injected as Providers. In certain DI frameworks (e.g., spring), Provider#get can be slow (lots of reflection) or even dangerous (dependency graph isn't validated until request processing as opposed to startup).

Make sense to add this?

/**
 * An annotation to be applied to parameters that should be provided by an
 * {@linkplain javax.inject.Inject injected} value in a generated factory.
 * Unlike {@link Provided}, this does not use {@link Provider} injection.
 */
@Target(PARAMETER)
public @interface Injected {}

Could also be an option to Provided but naming seems like it'd be clunky.

@Provided(direct=true) String hmm,
@Provided(noProvider=true) String maybe,
@Provided(provider=false) String err

[anuraaga]

@gk5885 Hey - let me know if either of these approaches sounds ok, and I can send a PR. Going back to writing manual factories is painful ;)

[gk5885]

I don't follow the logic here. Are you suggesting that Provider.get() is somehow more expensive than having just injected the value? I agree that it can be slow, but it doesn't make sense to me that that cost would be any greater than the cost that you would have incurred before creating the object when you inject the value directly.

[anuraaga]

Provider.get needs to first figure out what object to return before then returning it, and depending on the DI framework the performance of this will vary. In dagger, injection is all resolved at compile time so indeed this has almost no cost, but with something like Spring, it happens at runtime, using slow reflection (stuff like resolving qualifer annotations). A CPU profile of a server using autofactory with spring on critical paths showed a lot of samples stuck in this reflection - migrating them to handwritten, directly injected factories saved about 2% CPU overall, which is pretty significant for just removing the words Provider and get.

To me, the autofactory behavior of using Providers was surprising too - if you were to handwrite a factory, wouldn't you inject the values directly in almost all cases? The complexity of supporting direct injection seems low enough.

[gk5885]

Provider.get needs to first figure out what object to return before then returning it, and depending on the DI framework the performance of this will vary.

Yes, but the same holds true for any injection. The fact that you're doing it with a Provider wrapper doesn't change the fact that all injections require dependency resolution.

In dagger, injection is all resolved at compile time so indeed this has almost no cost, but with something like Spring, it happens at runtime, using slow reflection (stuff like resolving qualifer annotations)

Again, Spring must still do dependency resolution using reflection in order to inject either an instance or a Provider.

A CPU profile of a server using autofactory with spring on critical paths showed a lot of samples stuck in this reflection

It sounds like Spring is doing that reflection on every provision rather than once per provider. That sounds moderately broken. Guice, for example, does the reflective lookup once per provider. If I were you I'd file a bug there.

To me, the autofactory behavior of using Providers was surprising too - if you were to handwrite a factory, wouldn't you inject the values directly in almost all cases?

Absolutely not. If you inject an instance rather than a Provider you are sharing that instance amongst all types created by the factory. This essentially overrides whatever scoping semantics you may or may not have applied to the binding of the dependency. By injecting a Provider you are guaranteed that each instance returned from the AutoFactory has each of its dependencies in the proper scope.

[anuraaga]

I agree that spring could do better, but would also be ok if they said that Provider injection is such a minor use case that it's not a target of optimization - generally injecting a Supplier works without relying on magic semantics. Even the dagger docs, http://google.github.io/dagger/users-guide.html, heavily discourage them in favor of, hey factories :)

The autofactory is injected, so it's part of the object graph, and scoping is applied by nature of that injection, so I don't think you get any scope-related correctness from using Providers - the factory itself needs to be in the correct scope, either way, right? And anyways, I thought factories were mainly designed to allow making objects that share some, injected dependencies, and have others set at creation time, so "sharing that instance amongst all types created by the factory" feels correct, at least enough so that Provider shouldn't be the only supported behavior.

[tbroyer]

You may be biased because IIRC Spring uses singleton scope by default, but using "prototype" scope (to use Spring terminology IIRC), which is the norm in other DI libs such as Guice and Dagger, then you really expect that calling your factory twice will produce classes with distinct dependencies, rather than shared ones. And you can't assume that a factory will only be used once per instance; I'm sure we can find many examples (starting with Dagger compiler internals, using manual DI but still)

[anuraaga]

Spring is pretty new to me still, have written many more autofactories in the past that were injected with dagger.

Factories are part of the object graph and are providing dependencies from the object graph - a dependency will only be present in the graph once, so I'm not sure what you mean by having distinct dependencies. Even in dagger, where you may have many scopes that form layers in an object graph, any instantiated graph itself is still a singleton and the bindings are all shared. I would expect the dependency provided by the factory to be the same regardless of where or how often in the graph the factory method is called.

If I understand correctly, you make it sound like a Singleton-scoped factory should be able to construct items with Request-scoped dependencies but this sounds like it violates dagger's scoping principles - instead the factory should be request-scoped, like its dependencies, right? If you're completely stuck doing this scope-breaking, you may need to use a Provider, but it's a surprising default to me, and definitely doesn't seem like it should be forced.

[dstocking-ext]

Can we just add something like

public class Providers {

    private Providers() {

    }

    public static <T> Provider<T> get(final T item) {
        return new Provider<T> {
            @Override
            public T get() {
                return item;
            }
        };
    }
}

I did this is a scratch file, so I'm not 100% sure it's correct, but this makes it very convenient for making simple direct providers. Then, we at least can have a good alternative for it right now while we decide what we want to do with this. I personally think there are people who want to use AutoFactory to just build basic factories to replace existing factories in legacy code without requiring JSR330's full on dependency injection. I don't know if its worth supporting them and letting new code be written in a less maintainable way though.

[bubenheimer]

@gk5885, using Provider-based injection on Android has high overhead, as confirmed by Dagger team. Factories are a common pattern in general, and can also be used to provide functionality similar to a Provider with less overhead. Making good on this issue would provide much-needed Dagger boilerplate relief.

[raghsriniv]

Added type label for proper categorization

[dustinlacewell]

Just ran into this and hope to see support for non-provider factory dependencies in the future :)

[leourbina7r]

I'm reviving this issue in hopes that someone will see it. I'm currently using HelidonMP which uses Weld under the hood. Here Provider's aren't used, instead types are provided by using the @Produces annotation directly on methods that instantiate the types. Something like this:

import jakarta.enterprise.inject.Produces;

@ApplicationScoped
public class RandomNumberGenerator {

   private java.util.Random random = new java.util.Random(System.currentTimeMillis());

   @Produces @Named @Random int getRandomNumber() {
      return random.nextInt(100);
   }

}

I'm happy to do the work to make the annotation the OP mentioned, curious on what thoughts there are around this.

[leourbina]

Ok, just submitted a PR that addresses this issue. It adds a new attribute to the @AutoFactory annotation so you can now do:

import com.google.auto.factory.AutoFactory;
import com.google.auto.factory.Provided;

@AutoFactory(inject = true)
public class SimpleClassInject {
  private final String one;
  private final String two;

  public SimpleClassInject(String one, @Provided String two) {
    this.one = one;
    this.two = two;
  }
}

And the following factory is generated:

import javax.annotation.processing.Generated;
import javax.inject.Inject;

@Generated(
    value = "com.google.auto.factory.processor.AutoFactoryProcessor",
    comments = "https://github.com/google/auto/tree/main/factory"
)
public final class SimpleClassInjectFactory {
  private final String twoProvider;

  @Inject
  public SimpleClassInjectFactory(String twoProvider) {
    this.twoProvider = checkNotNull(twoProvider, 1, 1);
  }

  public SimpleClassInject create(String one) {
    return new SimpleClassInject(checkNotNull(one, 1, 2), checkNotNull(twoProvider, 2, 2));
  }

  private static <T> T checkNotNull(T reference, int argumentNumber, int argumentCount) {
    if (reference == null) {
      throw new NullPointerException("@AutoFactory method argument is null but is not marked @Nullable. Argument " + argumentNumber + " of " + argumentCount);
    }
    return reference;
  }
}

Seems like should be enough for the uses cases described here. Thoughts?

[tbroyer]

Providers are what gets injected into object, @Produces is a way to declare a factory method (just like @Provides in Guice or Dagger, or @Bean in Spring DI); those are different concerns. Weld is the reference implementation for Jakarta CDI, which builds upon Jakarta Inject, that provides the Provider interface (and CDI Instance implements Provider for instance); I would be surprised if Weld wasn't capable of injecting a Provider into an Auto Factory.

As said above (https://github.com/google/auto/issues/318#issuecomment-215819610), this would bypass any scoping semantics that could have been applied to the dependency.

[leourbina]

Hi @tbroyer, thanks for the super quick response! I really appreciate it - especially on a 8 year old issue :-)

I understand your comments, I think this is a fundamental difference between how Guice and Weld operate. In Guice, wherever you inject a value you can also inject a provider for that value - which allows for scoping.

Weld on the other hand does not do this. Instead scoping is done using qualifier annotations. If I have a factory method that @Produces a MyClass type, and I inject into an object a Provider<MyClass> - Weld just faceplants and claims that no instance of Provider<MyClass> is available in the container.

I think the main issue here is different approaches by different framework to accomplish the same thing. Just going through the Weld docs there is no single mention of using the Provider interface once.

Would love your thoughts here.

[tbroyer]

How can Weld be the reference implementation for CDI if it fails to implement CDI? https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0#builtin_instance

[rgzh]

I would be interested to have no Providers for the purpose of thread safety annotations. You generally do not know whether a Provider is thread safe. Most are, but it is not guaranteed. So without the Provider the injection happens single-threaded when the factory is created. And then create() can be called from different threads.