JDK-8133299 : Nashorn Java adapters should not early bind to functions
  • Type: Enhancement
  • Component: core-libs
  • Sub-Component: jdk.nashorn
  • Affected Version: 9
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2015-08-10
  • Updated: 2017-06-14
  • Resolved: 2016-01-25
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 9
9 b103Fixed
Related Reports
Relates :  
JDK-8067935 - Don't create adapter classes in JavaAdapterFactory.isAutoConvertibleFromFunction
Relates :  
JDK-8179024 - FX Samples broken
Description
Currently, Nashorn Java adapters bind early (on construction) to the functions in their implementation-object. This can be a problem if the implemented class or interface has a lot of methods, as they'll all be looked up at once. It is also a problem as we use bound functions, which currently don't support type specialization.

Instead, we should just store the implementation object, and have the body of every delegator method contain the usual INVOKEDYNAMIC dyn:getMethod, INVOKEDYNAMIC dyn:call sequence.

Benefits:
- adapters will work with any JS callables, not just ScriptFunction
- constant-time initialization of adapter instances, as it just has to store the object and doesn't have to bind functions
- we won't force creation of bound functions, so invoked functions will still type-specialize
- no need to maintain separate code for return type conversions (either to SAM types or to char and String) as normal Nashorn bootstrap linking will take care of them.

Drawbacks:
- more complicated delegator body code, esp. needing to distinguish between being constructed with a function (SAM) vs. object, and detecting unimplemented methods (to delegate to superclass or throw UnsupportedOperationException)
Comments
I made some further changes: - while the code would now be able to invoke just about any Nashorn callable object as an implementation for an adapter method, I restricted it to ScriptFunction only following a discussion with Sundar. The old adapter only supported ScriptFunction, so there's no backwards compatibility issue. - I have also made the bytecode of the adapter methods much smaller, by extracting some oft-repeated bytecode sequences into static methods on JavaAdapterServices (wrapThrowable(), unsupported()). - I reworked the way setGlobal works, again drastically reducing the bytecode size and moving all of the logic into JavaAdapterServices. JavaAdapterServices.setGlobal() now sets the necessary global and returns a Runnable that when run() will restore the previous global. The bytecode thus no longer has any logic for dealing with globals; it just invokes JAS.setGlobal(), stores the returned Runnable, and invokes run() on it in finally block. - The generator also detects when the INVOKEDYNAMIC dyn:call would need more than 255 slots for its parameter list, and introduces a Nashorn-compliant (Object callee, Object this, Object[] args) variable arity invocation in its place. - Added fairly comprehensive test suite.
15-01-2016
Some further implementation notes now that I'm done with this. It is hard to overstate just how much better the code is this way. All the benefits listed in the description apply. The constructors are very simple, they don't create a galore of method handles. Methods will be linked on first invocation. Return type conversions work automatically (we just need to take care of filtering hidden objects for Object return type). A note on return type conversions: we used to maintain copies of converters for converting return values to adapter classes (e.g. if we adapted a method that was returning "Runnable" etc). This is now removed and we let the bootstrap put these converters into a return value filter, just like elsewhere in Nashorn. While this converter maintenance might have seemed like a security-related functionality, it wasn't. It was only used to reduce the variability at the former invokeExact call sites in adapter methods by extracting their return conversion out of the bound method handles. Since we no longer use invokeExact with multitude of bound method handles, the problem is no longer relevant. (FWIW, keeping the explicit adapter converter logic would still be a minor optimization. If there is more than one target linked into the call site, with the former logic we wouldn't have the converter linked into a return value filter of each target. It doesn't seem worth the additional complexity, though.) As a minor improvement, I managed to make autoConvertibleFromFunction field in JavaAdapterBytecodeGenerator final. I added a "Throwable cause" field to AdaptationResult. It is used to preserve root causes of exceptions with AdaptationResult.Outcome.ERROR_OTHER. This can only happen if there are bugs in our code ��� it helped me a great deal when I was debugging this, and can be valuable if users can report this back to us in case a bug is still lurking in there. I haven't added any tests, as this is above all a refactoring. First attempts to run the code when I finished coding produced 79 failing tests that I eventually whittled down to 0; the current test base covers the adapters pretty extensively. Functionality like super$ calls, finalizers, etc. haven't been modified, only constructors and invocations of normal adapter methods.
02-01-2016