JDK-8232806 : Introduce a system property to disable eager lambda initialization
  • Type: Enhancement
  • Component: core-libs
  • Sub-Component: java.lang.invoke
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2019-10-22
  • Updated: 2022-06-27
  • Resolved: 2019-10-31
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 11 JDK 13 JDK 14
11.0.7-oracleFixed 13.0.4Fixed 14 b22Fixed
Related Reports
CSR :  
JDK-8233090 - Introduce a system property to disable eager lambda initialization
Relates :  
JDK-8242451 - ensure semantics of non-capturing lambdas are preserved independent of execution mode
Description
Lambdas generated by `LambdaMetaFactory` are eagerly and unconditionally initialized with `Unsafe` right after class loading. This interferes with heap-snapshotting tools such as GraalVM Native Image: When a lambda is initialized during parsing, all of the super-interfaces with default methods will be initialized too. Static initializers of those interfaces pollute the heap snapshot and lead to unexpected behavior. 

We would like to introduce a flag `jdk.internal.lambda.eagerlyInitialize`, which is `true` by default, and that can be used to prevent the eager initialization of lambdas. The proposed solution is summarised in the two following commits: 

 1) Introduces a new shape of non-capturing lambdas, where the lambda instance is kept in a static field: 
     
      https://github.com/graalvm/labs-openjdk-11/commit/00b9ecd85dedd0411837eee33635dd83e8b7def8

 2) Introduces the conditional initialization of lambdas as a pure performance optimization:

      https://github.com/graalvm/labs-openjdk-11/commit/273e8590a7b57c0c10d9213ca9e0ba581e2817b8

With the proposed scheme we get simpler code, that can be used for heap snapshotting and 1.5% better startup time for `HelloLambda.java`. The detailed numbers follow.

For all the tests we used the same configuration: `Intel(R) Xeon(R) CPU E5-2690 v4 @ 2.60GHz` bound to a single socket with turbo disabled. We measured the Java 11 master (master), Java 11 with the first patch (patched) and Java 11 with the patch and lambdas pre-initialized with Unsafe (patched-init). For the instructions count, we used `perf stat` and executed 3 times a 100 runs and took the average. The standard deviation is very small for the interpreter case so I don’t report it. Here are the results:

 (1) I measured only the number of instructions due to the OS noise related to time. The "Hello, Lambda!" numbers are with the JIT enabled.
      |    Branch     | Instructions    |
      |-----------------|--------------------|
      | master         | 500,490,396  |
      | patched       | 492,933,691  |
      | patched-init | 483,938,234  |

  (2) The instruction count is measured in the interpreter (-Xint) and JIT and the time is measured only with the JIT enabled:
      |     Branch     | Instructions (-Xint) | Instructions (JIT)  | Time in us (JIT)|
      |------------------|--------------------------|-------------------------|---------------------|
      | master          | 1,285,945,379       | 1,922,974,572      | 227,837           |
      | patched        | 1,315,914,375       | 2,058,175,796      | 233,457           |
      | patched-init  | 1,286,079,726       | 1,986,969,597      | 218,579           |
Comments
Fix request (13u): 13u looks like a good place to try it. And it is already in 11u. The patch applies cleanly.
26-05-2020
Fix Request (11u): This is useful for graalvm and has also already been backported into 11.0.7-oracle, so should be included at least for parity. The patch did not function cleanly and a review thread was opened here: https://mail.openjdk.java.net/pipermail/jdk-updates-dev/2019-December/002260.html
19-12-2019
I am currently looking at backporting this into jdk11u-dev, at the least for parity with Oracle as a backport bug has been resolved for 11.0.7-oracle.
17-12-2019
URL: https://hg.openjdk.java.net/jdk/jdk/rev/27c2d2a4b695 User: mr Date: 2019-10-31 16:20:11 +0000
31-10-2019
OK, agreed.
30-10-2019
[~thomaswue] As we discussed offline earlier today, let's reframe the discussion away from acceptable/not-acceptable, towards "better for now" / "better for later." The first patch that was suggested essentially forks the translation strategy. There is nothing intrinsically wrong with the direction proposed in that patch, other than it is a fork, which turns out to be on a collision course with several planned enhancements for this code, including migrating from VMACs to hidden classes, migrating the translation for non-capturing lambdas from indy to condy (JEP 309), and others — several of which will affect the translation strategy. Since it is more effective to do these changes together, along with the attendant testing, than to do them piecemeal, my strong preference would be to select the least invasive means of solving today’s problem, aimed at enabling NI to use a stock OpenJDK build, and then come back to the finer points of this issue when the other translation strategy issues are also on the table, and come to a less “forky” solution. I am confident that, once the dust clears, we can come up with a more unified translation strategy that serves all the use cases well -- but I would rather not try and do that right now. In the meantime, I like Vojin's latest approach, which merely disables the caching optimization and suppresses the eager initialization. (As you observed, the allocation probably gets EAed away anyway by the Graal compiler, so there is unlikely to be a performance issue with this approach.) Hopefully this is good enough for now and we can revisit when we are able to make the other changes (which are currently blocked on other work.)
30-10-2019
Note that we have an attached patch that allows us to keep the original lambda shape for the JIT case: https://bugs.openjdk.java.net/secure/attachment/85126/keep-original-lambda-shape.patch With this kind of change, regular Java users would not be impacted at all.
30-10-2019
Brian: OK. Can you just summarize for us the reasoning why the change that would solve the full issue is not acceptable? I think you mentioned that the patch was too invasive and prohibits future evolution of the code.
29-10-2019
The problem with intrinsics (substitutions) in the Native Image code base is that they are complicated and difficult to maintain. Our goal was to push out several changes to OpenJDK so that we minimize the long-term maintenance effort for Native Image, and also to make OpenJDK in itself more friendly to AOT compilation. The current HotSpot-based AOT compilation has many of the same difficulties that we have, and any other AOT effort as part of OpenJDK will have these difficulties too. So no, we do not have a "good enough plan". The good plan is to make OpenJDK ready for AOT compilation.
29-10-2019
Thomas; No, I don't think the cache was a mistake. The optimization is a fine one; what is not fine is _depending on it_, and the language spec even goes as far as to say that code that _depends on_ the identity is simply broken. There's nothing wrong with providing the optimization where it can be done reasonably, but we are not constrained to carry forward this optimization in all cases for all times, just because someone might have made a bad assumption. This is the nature of opportunistic optimizations; you can't rely on them, but you're happen to have them when they work. But in any case, we're in the weeds, so let's get back on the road. The main value of this optimization, BTW, is not the elimination of the allocation-per-evaluation directly, but the hint it provides to the JIT (in the form of the `MethodHandles::constant` combinator) that the link target is a constant, and therefore that the JIT can propagate the constant. NI could have used that too, of course, except the classloading side-effects got in the way. So NI has to get there differently, but you should be able to achieve the same result through other means. So, I don't think there's an additional problem here that needs to be solved. Yes, we could argue back and forth about which solution seems better from a variety of subjective utility metrics, but there are many competing considerations and that would be total value-destroying, so let's not do that. I think we have a good enough plan: this patch provides the minimally invasive change necessary to lay the groundwork, doesn't get in the way of future plans for this code, and an NI intrinsic can recover the compilation behavior you want.
29-10-2019
Brian: No, we don't give recommendations for users. We just want to support running existing code from users that depend on HotSpot's implementation of the feature. It seems like this caching should have never been introduced (similar to the java.lang.Integer boxing cache), but this implementation detail has already leaked out to many user programs. Yes, we can always maintain some modifications of the code base (or some patching via an intrinsic).
29-10-2019
Brian: So you are saying we should remove the special 0-parameter optimization completely from LMF, so that no one is tempted to exploit something that is explicitly called out as unreliable?
29-10-2019
Doug, Thomas; it sounds like you are saying that you are recommending that Native Image users code in a way that is _explicitly called out in the JLS as unreliable_. (Ideally, you should be educating your users to avoid such unreliable techniques!) No code should rely on the caching behavior, so claims of "but code relies on the caching behavior" is not a compelling reason to choose a more invasive way to handle this issue. Further, if NI is able to improve lambda performance through better code generation, it can and should do so with an intrinsic, as Vojin suggested above. The protocol of LambdaMetafactory is clearly specified so that platforms which wish to translate it differently can do so using the symbolic information present at the indy site, or transform the bytecode prior to execution. Surely the NI compiler can recognize that an indy site has been linked to a no-arg constructor, and resurrect the desired caching in the native code?
29-10-2019
Yes. We need to support code that relies on the caching behavior exposed by HotSpot to Java user programs. So both observable behavior from the user program's perspective and performance characteristics must be identical.
29-10-2019
It's not a change in specified semantics but definitely a change in observed and measurable execution. But let’s drop the use of the word “semantics" and focus on performance. It seems that Native Image users will reasonably expect the same performance for non-capturing lambdas as they get in non-Native Image execution, despite the spec making no guarantees. If performance doesn't matter, why the optimization of 0-parameter lambdas in the first place?
29-10-2019
I take strong exception to the implication this is a change in semantics. The JLS is _exceedingly_ clear that any code that attempts to ascribe meaning to the identity of lambda objects is skating on extremely thin ice. Just because one particular implementation happens to sometimes - under unspecified circumstances - provide certain behavior that is explicitly disavowed by the spec, does not mean that it is reasonable to develop a dependence on said behavior.
29-10-2019
[~briangoetz] given that the current implementation of LMF provides referential equality for non-capturing lambdas, why should these semantics change when making initialization of these lamdba classes non-eager? This can penalize the performance of non-eagerly initialized lambdas due to increased allocation. Also, I'm curious as to how this limitation “minimizes the constraints on future evolution for this code”?
29-10-2019
Webrev: https://cr.openjdk.java.net/~mr/rev/8232806/
29-10-2019
I've changed the summary to describe what this RFE does, rather than the problem that it solves.
28-10-2019
also 'disableEagerInitialization' should be final.
28-10-2019
FYI, small recommendation regarding the latest patch: see `sun.security.action.GetBooleanAction` with the same semantics as `java.lang.Boolean.getBoolean`, the latter of which can be used in the tests.
28-10-2019
The new patch proposed by [~briangoetz] (lambda-disable-initialization.diff) contains the following changes: 1) It introduces the flag `jdk.internal.lambda.disableEagerInitialization` that is off by default. 2) This flag makes all lambdas go through the factory (get$Lambda) and disables eager initialization with Unsafe. 3) This breaks the referential equality so we will have to implement an intrinsic in Native Image that does what the original patch did: https://github.com/graalvm/labs-openjdk-11/commit/00b9ecd85dedd0411837eee33635dd83e8b7def8 4) We tested with make run-test TEST="jtreg:test/langtools/tools/javac/lambda" when the flag is on and off by default. With disabled initialization (on by default) of lambdas two tests failed. We fixed those two tests to account for this flag and make sure that the shape of lambdas is correct. Test changes were not necessary in the original change.
28-10-2019
I applaud your desire to go beyond the spec here; it's a good general approach (and I'm glad you fixed your internal bugs as a result!) Still, in this particular case, I think I greatly prefer your original approach -- disable the caching of the instance when running with "no eager init" (in part because it minimizes the constraints on future evolution for this code.) The only change I would make to your original-attempt patch is to invert the sense of the boolean `initializeLambdas` to something like `noEagerInitialization`, to make it more clear these code paths are unusual. Then we'll need smoke tests to verify that lambdas behave properly with both the flag set and not set.
22-10-2019
With my original approach, I started getting bug reports even within our own codebase. We fixed those as people didn't follow the spec, but I am pretty sure that at the global scale the number of issues that I will have is not worth relying on the spec. I believe this is one of those cases where people just follow the implementation instead of the spec, so implementation becomes a spec one way or another. The patch that is needed for the static field is attached (keep-original-lambda-shape.patch).
22-10-2019
Can you further expand on: > We need this as we need to have reference equality of non-capturing lambdas. This is not required by the Java spec but the user code assumes reference equality in many places. The spec explicitly says (15.27.4): > This implies that the identity of the result of evaluating a lambda expression (or, of > serializing and deserializing a lambda expression) is unpredictable, and therefore identitysensitive > operations (such as reference equality (§15.21.3), object locking (§14.19), and > the System.identityHashCode method) may produce different results in different > implementations of the Java programming language, or even upon different lambda > expression evaluations in the same implementation. So now that I am fully tracking what you are doing, I think like your original approach better -- only use the optimized version when the flag is not set; when the flag is set, put it in the "capturing" bucket. (The flag exists entirely for use in performance-insensitive situations.)
22-10-2019
I think what you're (not) saying here is: in order for the removal of `ensureInitialized` to be effective, the instantiation of the instance has to be behind a static initializer, otherwise, even if you remove the `eI` call, it still gets initialized when we prepare the `MethodHandle::constant` instance. Right?
22-10-2019
I would much prefer an approach where _all_ the new code were guarded by the `if lazy init` check -- especially if there is exactly one of these checks.
22-10-2019
The second patch is based on the first one so alone it will not work. An equivalent of the second patch in the original version of `InnerClassLambdaMetafactory` would be: 1) Introduce a case for code generation where lambda is held in the static field. We need this as we need to have reference equality of non-capturing lambdas. This is not required by the Java spec but the user code assumes reference equality in many places. Here was our first attempt that failed due to reference equality issues: https://github.com/graalvm/labs-openjdk-11/commit/eee0d047fe53285076c9745c74a068bb1089b0a7#diff-f5e92838a463f03bfe2e43137b670a6a 2) When the native-image-specific flag is enabled (we would have to call it differently), we would use the path that generates the static field and we would avoid calling initialization via Unsafe. I can make a patch for that easily tomorrow and post it here. That way you can see if the extra complexity is preferred for the sake of preserving this frequently used code path.
22-10-2019
Don't underestimate how high the bar is for changing code like this, which is literally holding up the entirety of the software world. So, there's an extremely strong bias against "refactorings" like the first patch just because it seems "cleaner", in the absence of a clear and compelling benefit to the 99.9999% of users that are not running this code at NI snapshot time. The second patch is nicely localized; while I find the flag to be an ugly hack, the patch looks acceptably low-risk, would not affect the mainstream users, and would not interfere with future evolution of the translation strategy. So it is in the category of defensible, if there is no better way to do it (though I would hope there were a better way, that involved dealing with the side-effects of classloading at image build time.) In any case, you still haven't explained why the second patch alone doesn't solve your problem. If it doesn't, can you show me what the "more complex" version is?
22-10-2019
[~briangoetz] the lambda generation happens during native-image build time while the lambda initialization happens at runtime following regular rules for class initialization. If we initialize the lambda during the image-build time the side-effects remain captured in the image heap. As for the `MethodHandle::constant`, leaving it would increase the code complexity: We would require an extra path in lambda code generation for the Native Image case. Also, I benchmarked `MethodHandle::constant` and it performs 1.5% worse in `HelloLambda.java`.
22-10-2019
I am still unclear on how delaying the initialization for a small amount of time helps with managing the side-effects of initializes the supertypes, since its still going to happen very soon. I would like to see an explanation of why. Separately, I am unclear on why the first patch is needed or desirable at all. The handling of non-capturing lambdas with `MethodHandle::constant` seems preferable.
22-10-2019
[~briangoetz]: [~vjovanovic] did some experiments on bootstrapping many lambdas - similar to what I've done in the past - and keeping the U.eI reduces overhead by 6-7% on bootstrap and invocation time in a test over 1000 lambdas (-0.015ms per lambda). Without U.eI this patch would instead be a small regression for repeat initialization of many lambdas, so I suggested it remain as default with an option to turn the eager initialization off for cases that need to avoid it.
22-10-2019
The first lambda creation (which may well happen during the LMF call) will trigger all those inits anyway. When a LMF is called, the lambda is about to be instantiated for the first time. At that point the clinits of all its supers will be called. Removing U.eI just defers the inevitable for a microsecond. How exactly does this help?
22-10-2019
AFAICT, the eager class initialization of the lambda proxy class avoids the need of class initialization barrier generated for method invocation that incurs overhead to the runtime invocation. It'd be good to show no performance regression for this patch.
22-10-2019