My immediate win is that this includes migrating off the old version of bazel-common that includes `google_common_workspace_rules()`: Calling `google_common_workspace_rules()` leads to trouble under macos-latest (as previously in https://github.com/google/guice/commit/17011080a118fc2d23314ccf0d5546ce53e8b650):
```
ERROR: /Users/runner/work/guice/guice/WORKSPACE:26:30: fetching android_sdk_repository rule //external:androidsdk: Android SDK api level 34 was requested but it is not installed in the Android SDK at /Users/runner/Library/Android/sdk. The api levels found were [36, 35]. Please choose an available api level or install api level 34 from the Android SDK Manager.
ERROR: Analysis of target '//tools:osgi_wrapper' failed; build aborted: Android SDK api level 34 was requested but it is not installed in the Android SDK at /Users/runner/Library/Android/sdk. The api levels found were [36, 35]. Please choose an available api level or install api level 34 from the Android SDK Manager.
```
RELNOTES=n/a
PiperOrigin-RevId: 957370304
This change optimizes DeclaredMembers sorting of methods and fields by first
filtering for the fields or methods with relevant annotations, and then only
sorting the members of interest.
PiperOrigin-RevId: 904608829
Truth will soon:
- print only the simple name for some well-known classes (e.g., print "NullPointerException" instead of "java.lang.NullPointerException")
- print the canonical name where available instead of the binary name (e.g., print "com.google.Foo.BarException" instead of "com.google.Foo$BarException"
This CL updates callers to accept both versions.
RELNOTES=n/a
PiperOrigin-RevId: 775721999
This series addresses known issues in the experimental method handle implementation
- MethodHandles produced by InternalFactory.getHandle are now cached. This should speed up linking and reduce the number of 'lambda forms' we generate.
- Doing this required demoting `Dependency` from the `getHandle` to be a runtime method parameter and changing InternalFactory to be an abstract class
- We no longer generate custom `Provider` subclasses to hold `invokedynamic` instructions and instead use a simple hand written Provider class that lazily allocates a `MethodHandle` and calls invokeExact.
- This should greatly speed up injector creation at the cost of a slightly more expensive `invokeExact` call.
- requestStaticInjection and MembersInjectors are now fully supported with MethodHandles.
I imagine there are some as yet undiscovered issues but this should unblock broader experimentation.
Fixes#1894
COPYBARA_INTEGRATE_REVIEW=https://github.com/google/guice/pull/1894 from lukesandberg:method_handles 2166e1275e
PiperOrigin-RevId: 745281859
* avoid `asCollector` when the size is not bound by an existing method
- otherwise we can easily hit limits in the number of method arguments that java allows which is 255. This was a problem for some of our multibinders since multibinders can easily have hundres or thousands of entries
* be careful about deep `foldArguments` chains, method handles use a tail-call pattern which can lead to stack overflow, so instead use simple divide and conquor techniques.
* detect when we are binding to very high arity methods and bail out. `MethodHandle` infra reserves 1 or two parameters and thus limits the methods/constructors we can bind to to those with _only_ 253 or 254 parameters (WHAT IS THIS, A VM FOR ANTS!).
- This should probably become a guice error, but for the time being we just back off.
PiperOrigin-RevId: 741507607
If a `javax.inject.Provider` was bound in a scope, we would enforce that it was a `com.google.inject.Provider` which was wrong. This is due to logic in `MoreTypes.canonicalizeForKey` which canonicalizes `javax.inject.Provider` to the guice interface.
This behavior makes sense for injection points since guice only ever hands out `com.google.inject.Provider` instances but we don't always canonicalize them at every phase of injection which is what lead to the bug. Now we no longer use `Dependency` objects to enforce types so we don't risk this issue anymore
PiperOrigin-RevId: 741155148
I thought modeling native types would be better but on reflection it just adds complexity for little gain. So turn all generated factories into `(InternalContext)->Object` types. This eliminates a bunch of `asType` adapters and should improve linkage performance a bit (not that this is a huge concern), but the major benefit is just simplicity.
Performance is an interesting question here. In principle we are just adding 'no op' cast to `Object` on constructor and provides method invocations, and then `checkCast` instructions at constructor and method parameter injection points. So the waste is those extra `cast` operations. However, in the JVM those are nearly free (simple pointer check, can often be elided as part of normal invocation type checking). So my guess is that this is a no-op performance wise. Some limited benchmarking of `@Provides` and `@Inject` constructors bears this out.
PiperOrigin-RevId: 740791709
This is the most complicated one since it uses a long tail of rarer features (InjectionListeners, member injection). But mostly it builds on established patterns so the trickiest part was actually getting the exception handling correct, luckily this is pretty well tested!
With this there is enough to start broad based testing. Beyond this there are open questions about how to handle various member injection APIs for which I have left some TODOs
PiperOrigin-RevId: 740475465
This follows the pattern of how `InjectorImpl.getProvider` works and calls into the same method for generating a provider subtype. There are 2 critical differences
1. We always set the `linked` flag to `true` for these 'Scoped' providers.
2. The `dependency` is inferred from the `InternalContext.getDependency` method
The latter point adds some non-trivial complexity since `getHandle` accepts `Dependency` as a parameter and generally specializes behavior on it (checks if it is nullable, used in error messages, raw types are used for proxies and error messages). So we use the 'callsite' trick to bind the implementation to a patchable callsite that we can update whenever we see a new dependency.
The callsite will be implemented as a `tableSwitch` when possible and a hashmap holding method handles otherwise (<jdk17).
PiperOrigin-RevId: 740381461
Previously I had assumed that the 'current injector options' would be static for a particular `InternalFactory` and thus we could optimize code generation based on it (drop some if-statements, eliminate some field writes). However, this is not true when factories are shared across parent and child injectors. There is no simple way to detect this so we simply back off.
Remove `InjectorOptions` from LinkageContext to account for this and eliminate the downstream dependencies on it. Add a test to show how options can flip both ways at runtime.
PiperOrigin-RevId: 740365320
To support 'native-multibinders' we have specialized the construction and initialization of subtypes of `InternalProviderInstanceBindingImpl.Factory`, however this creates some ambiguity when such providers are shared across injectors. To resolve that, we exclusively 'bind' each factory to a single injector and any injector that fails to do so simply treats it as a normal `ProviderInstanceBinding`. As far as I can tell this issue has been around for a very long time (2016?!), but only becomes problematic as we attempt to specialize more of the implementation on the `disableCircularProxies` flag.
This simplifies future efforts to specialize the implementations of Provider methods and multibinders.
PiperOrigin-RevId: 740107103
We can take advantage of the fact that `setDependency` is a no-op when `disableCircularProxies` is set, but otherwise this is a pretty straightforward translation.
For Singleton scoped factories, we can follow the pattern of `Initializable` and create a 'patchable' callsite which should enable constant folding of resolved singletons.
PiperOrigin-RevId: 739912256
This includes multibinders and provider methods. Many of the multibinders were pretty straightforward if tedious. The ProviderMethod implementation raises some interesting API questions:
* Should `skipfastclassgeneration` apply to the methodhandle implementation?
* I decided "no" out of simplicity. Constructing the methodhandle implementation should be very cheap compared to a fast-class.
* How should non-public members be handled?
* I decided to follow the pattern of the `ReflectionProviderMethod` and just call `setAccessible`. This is not the best solution so I wrote a copious note about better solutions.
PiperOrigin-RevId: 738411883
This will save a tiny bit of memory and time and should simplify the `MethodHandle` implementations when we get there. Behavior is nearly perfectly concerned, but as the `CircularDependencyTest` points out, error messages will change when failing with cycles that go through scoped bindings.
PiperOrigin-RevId: 738385449
If 2 `InternalFactory` objects get linked to each other recursively we need to handle and detect cycles to prevent `StackOverflow`
For `get` calls this is managed by `InternalContext` and it is extremely performance sensitive.
For `getHandle` calls, this is currently unhandled which can lead to StackOverflow as more InternalFactory types are migrated. This change addresses that via a new class `LinkageContext` which can detect and automatically resolve a cycle.
The specific strategy is to use a `MutableCallSite` as a layer of indirection to link back to the original method handle once it is done being constructed. This constructs a recursive `MethodHandle`, much in the same way the normal cycle resolution uses a `Proxy` for that same layer of indirection.
See also [The farnsworth parabox](https://en.wikipedia.org/wiki/The_Farnsworth_Parabox).
Testing this scenario right now is difficult since we don't support many kinds of factories, but the next change introducing provider methods requires this.
PiperOrigin-RevId: 737708332
These will be useful implementing multibinders. The approach is to just construct method handles that developers would normally write.
PiperOrigin-RevId: 737694837
This required supporting a number of 'standard features'
* Initializables
* null checking for injection points
* ProvisionListeners
* implementing the cyclic dependency protocol.
The ProvisionListener implementation is simpler/slower than it might be since the ideal solution would be to generate classes for each ProvisionCallback, instead I am 'manually constructing' them and calling `invokeExact` myself. As these are rare the slightly higher overhead of this approach is justifiable imho (or could be separately addressed in the future).
PiperOrigin-RevId: 737658072
This is hidden behind a new system property `-Dguice_use_experimental_method_handles=YES` enables it (`NO` is the default). This is also conditioned on `BytecodeGeneration` being enabled.
For now the providers simply delegate to `InternalFactory.get` and thus are not very useful so this just proves out some of the basic mechanisms.
There were a few interesting details to work out.
* A new method was added to `ClassDefining` to always create these new `Provider` subtypes in a way that allows them to be garbage collected.
- This was necessary to preserve some behavior with child injectors and the WeakKeySet, otherwise the fact that we bind references to factories via invokedynamic would leak injectors.
* To actually install a generated `MethodHandle` into class we need to use `invokedynamic` and some custom bytecode generation.
- This should be the only bytecode generation required.
- We use `invokedynamic` plus a lazy `Supplier` to actually construct the `MethodHandle` objects. This means everything is lazily computed on the first call.
PiperOrigin-RevId: 735805730
This doesn't change any Guice behavior; it just lets nullness checkers (like Kotlin and the Checker Framework) allow you to bind to such providers more easily.
Guice's handling of `null` is not entirely principled, and this CL doesn't attempt to "solve" that, nor does it attempt to thoroughly annotate for nullness, let alone enable a nullness checker for the Guice codebase. Rather, it aims only to remove a restriction that I don't think is serving any particular purpose and that no on "intended"; I think it just kind of happened because the Checker Framework treats unannotated code leniently and strictly at different times, and this happens to be one of the strict cases. (We might also have once had the hope of cracking down on injection of `null`?)
The immediate motivation for this CL is https://github.com/google/inject-common/pull/26: Soon, nullness checkers will know that `Providers.of(null)` returns *gasp* a `Provider` that can return `null`. That makes `bind(Foo.class).to(Providers.of(null))` an error. This CL lets those callers continue to compile.
PiperOrigin-RevId: 734755975
The map implementation used by `MapMaker` is fairly old and requires 2 levels of indirection (segments->table). Switching to `ConcurrentHashMap` would be better but this will still be redundant with logic inside of `Proxy` so instead we just use that. Based on some simple benchmarks this is about 5X as fast, and should save a bit memory.
A number of alternatives were considered
* adding a marker interface to the list of proxy interfaces
This is a completely ideal solution but fails due to our need to support proxying interfaces from all `ClassLoaders` and it isn't possible to proxy interfaces that cannot mutually 'see' each other. So we would need to 'inject' our interface into the Bootstrap classloader which seems impossible and risky, or create special 'child' classloaders that can bridge it, but this triggers issues with proxying package-private interfaces... sigh.
* using a `ClassValue`, bootstrapping the value is tricky and while this is faster than the status quo, it would allocate an entry for every class we queried which could add up in a large application, and the approach in this change is faster.
* using `Proxy.isProxyInstance` this works but is slower than an `instanceof` query.
PiperOrigin-RevId: 721984180
This enables a few optimizations
- A number of implementations can avoid `InternalContext` overheads addressing https://github.com/google/guice/issues/1802
- `ConstantBindings`, one time bindings like `Initializable` and `@Singleton` bound values, and a number of multibinder bindings in particular.
- We can leverage the cache within `BindingImpl` to share provider instances.
The tricky part is about multibindings, which have complex multi-phase initialization semantics, so we need to be defensive about `makeProvider` getting called prior to being initialized. To help manage this I
* reordered the `LookupProcessor` to run _after_ multibinders.
- The comment justifying its current location in initialization is obsolete since multibinders were rewritten to be 'native'.
- This ensures lookups get optimized providers!
* made the `SyntheticProviderFactory` allocate its delegate lazily.
* make `OptionalBinder` explicitly initialize their delegate bindings and also be defensive against recursive and duplicate initialization. This is necessary to force initialization to be correct when our `getExistingBinding` query ends up delegating to another multibinder.
* be defensive about initialization failing to complete due to errors
PiperOrigin-RevId: 721080468
This ensures that other phases can see the jit bindings they create, notably this is important for `OptionalBinder` which queries for bindings using `getExistingBinding`
This will allow us to optimize the way `Provider` instances are created in a later change.
PiperOrigin-RevId: 720621327
Every implementation needs access to the current `InternalContext` so we can save a bit of memory by just passing it.
This is pretty minor since `ProvisionListeners` are rare, but it satisfies a long running TODO and allows us to eliminate some allocations from the normal provisioning path.
PiperOrigin-RevId: 715467849
* accept an `InternalContext` parameter from `Initializable.get` which all callers can provide.
- This speeds up the `injectAll` phrase of injector creation since we no longer need to create an `InternalContext` object for each instance binding.
* for 'constant' bindings with no injection points use a simpler set of `InternalFactory` types.
- This provides some immediate benefits by eliminating indirection (though admittedly the impacts are minor), but when we adopt method handles this will enable a cheaper implementation there as well.
PiperOrigin-RevId: 715093467
Additionally optimize the implementation of DelegatingInvocationHandler to eliminate an unnecessary field
With these two optimizations in place, the maximum size of a leak from resolving multithreaded lock cycles in SingletonScope is now a single tiny object, so instead of eagerly clearing out proxies to avoid a leak we can just wait to satisfy them if someone attempts to construct the singleton a second time, and if we don't we just won't sweat these 16 bytes.
PiperOrigin-RevId: 714082982
Guice Team