Hi, Please review the following patch for minimal dynamic constant support: http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-hs/webrev/> https://bugs.openjdk.java.net/browse/JDK-8186046 <https://bugs.openjdk.java.net/browse/JDK-8186046> https://bugs.openjdk.java.net/browse/JDK-8186209 <https://bugs.openjdk.java.net/browse/JDK-8186209> This patch is based on the JDK 10 unified HotSpot repository. Testing so far looks good. By minimal i mean just the support in the runtime for a dynamic constant pool entry to be referenced by a LDC instruction or a bootstrap method argument. Much of the work leverages the foundations built by invoke dynamic but is arguably simpler since resolution is less complex. A small set of bootstrap methods will be proposed as a follow on issue for 10 (these are currently being refined in the amber repository). Bootstrap method invocation has not changed (and the rules are the same for dynamic constants and indy). It is planned to enhance this in a further major release to support lazy resolution of bootstrap method arguments. The CSR for the VM specification is here: https://bugs.openjdk.java.net/browse/JDK-8189199 <https://bugs.openjdk.java.net/browse/JDK-8189199> the j.l.invoke package documentation was also updated but please consider the VM specification as the definitive "source of truth" (we may clean up this area further later on so it becomes more informative, and that may also apply to duplicative text on MethodHandles/VarHandles). Any AoT-related work will be deferred to a future release. — This patch only supports x64 platforms. There is a small set of changes specific to x64 (specifically to support null and primitives constants, as prior to this patch null was used as a sentinel for resolution and certain primitives types would never have been encountered, such as say byte). We will need to follow up with the SPARC platform and it is hoped/anticipated that OpenJDK members responsible for other platforms (namely ARM and PPC) will separately provide patches. — Many of tests rely on an experimental byte code API that supports the generation of byte code with dynamic constants. One test uses class file bytes produced from a modified version of asmtools. The modifications have now been pushed but a new version of asmtools need to be rolled into jtreg before the test can operate directly on asmtools information rather than embedding class file bytes directly in the test. — Paul.
Paul, templateTable_x86.cpp: 564 const Register flags = rcx; 565 const Register rarg = NOT_LP64(rcx) LP64_ONLY(c_rarg1); Should we use another register for rarg under NOT_LP64 ? -Dmitry On 10/26/2017 08:03 PM, Paul Sandoz wrote:
Hi,
Please review the following patch for minimal dynamic constant support:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-hs/webrev/>
https://bugs.openjdk.java.net/browse/JDK-8186046 <https://bugs.openjdk.java.net/browse/JDK-8186046> https://bugs.openjdk.java.net/browse/JDK-8186209 <https://bugs.openjdk.java.net/browse/JDK-8186209>
This patch is based on the JDK 10 unified HotSpot repository. Testing so far looks good.
By minimal i mean just the support in the runtime for a dynamic constant pool entry to be referenced by a LDC instruction or a bootstrap method argument. Much of the work leverages the foundations built by invoke dynamic but is arguably simpler since resolution is less complex.
A small set of bootstrap methods will be proposed as a follow on issue for 10 (these are currently being refined in the amber repository).
Bootstrap method invocation has not changed (and the rules are the same for dynamic constants and indy). It is planned to enhance this in a further major release to support lazy resolution of bootstrap method arguments.
The CSR for the VM specification is here:
https://bugs.openjdk.java.net/browse/JDK-8189199 <https://bugs.openjdk.java.net/browse/JDK-8189199>
the j.l.invoke package documentation was also updated but please consider the VM specification as the definitive "source of truth" (we may clean up this area further later on so it becomes more informative, and that may also apply to duplicative text on MethodHandles/VarHandles).
Any AoT-related work will be deferred to a future release.
—
This patch only supports x64 platforms. There is a small set of changes specific to x64 (specifically to support null and primitives constants, as prior to this patch null was used as a sentinel for resolution and certain primitives types would never have been encountered, such as say byte).
We will need to follow up with the SPARC platform and it is hoped/anticipated that OpenJDK members responsible for other platforms (namely ARM and PPC) will separately provide patches.
—
Many of tests rely on an experimental byte code API that supports the generation of byte code with dynamic constants.
One test uses class file bytes produced from a modified version of asmtools. The modifications have now been pushed but a new version of asmtools need to be rolled into jtreg before the test can operate directly on asmtools information rather than embedding class file bytes directly in the test.
—
Paul.
Hi, Thanks for reviewing.
On 30 Oct 2017, at 11:05, Dmitry Samersoff <dmitry.samersoff@bell-sw.com> wrote:
Paul,
templateTable_x86.cpp:
564 const Register flags = rcx; 565 const Register rarg = NOT_LP64(rcx) LP64_ONLY(c_rarg1);
Should we use another register for rarg under NOT_LP64 ?
I think it should be ok, it i ain’t an expert here on the interpreter and the calling conventions, so please correct me. Some more context: + const Register flags = rcx; + const Register rarg = NOT_LP64(rcx) LP64_ONLY(c_rarg1); + __ movl(rarg, (int)bytecode()); The current bytecode code is loaded into “rarg” + call_VM(obj, CAST_FROM_FN_PTR(address, InterpreterRuntime::resolve_ldc), rarg); Then “rarg" is the argument to the call to InterpreterRuntime::resolve_ldc, after which it is no longer referred to. +#ifndef _LP64 + // borrow rdi from locals + __ get_thread(rdi); + __ get_vm_result_2(flags, rdi); + __ restore_locals(); +#else + __ get_vm_result_2(flags, r15_thread); +#endif The result from the call is then loaded into flags. So i don’t think it matters in this case if rcx is aliased. Paul.
-Dmitry
On 10/26/2017 08:03 PM, Paul Sandoz wrote:
Hi,
Please review the following patch for minimal dynamic constant support:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-hs/webrev/>
https://bugs.openjdk.java.net/browse/JDK-8186046 <https://bugs.openjdk.java.net/browse/JDK-8186046> https://bugs.openjdk.java.net/browse/JDK-8186209 <https://bugs.openjdk.java.net/browse/JDK-8186209>
This patch is based on the JDK 10 unified HotSpot repository. Testing so far looks good.
By minimal i mean just the support in the runtime for a dynamic constant pool entry to be referenced by a LDC instruction or a bootstrap method argument. Much of the work leverages the foundations built by invoke dynamic but is arguably simpler since resolution is less complex.
A small set of bootstrap methods will be proposed as a follow on issue for 10 (these are currently being refined in the amber repository).
Bootstrap method invocation has not changed (and the rules are the same for dynamic constants and indy). It is planned to enhance this in a further major release to support lazy resolution of bootstrap method arguments.
The CSR for the VM specification is here:
https://bugs.openjdk.java.net/browse/JDK-8189199 <https://bugs.openjdk.java.net/browse/JDK-8189199>
the j.l.invoke package documentation was also updated but please consider the VM specification as the definitive "source of truth" (we may clean up this area further later on so it becomes more informative, and that may also apply to duplicative text on MethodHandles/VarHandles).
Any AoT-related work will be deferred to a future release.
—
This patch only supports x64 platforms. There is a small set of changes specific to x64 (specifically to support null and primitives constants, as prior to this patch null was used as a sentinel for resolution and certain primitives types would never have been encountered, such as say byte).
We will need to follow up with the SPARC platform and it is hoped/anticipated that OpenJDK members responsible for other platforms (namely ARM and PPC) will separately provide patches.
—
Many of tests rely on an experimental byte code API that supports the generation of byte code with dynamic constants.
One test uses class file bytes produced from a modified version of asmtools. The modifications have now been pushed but a new version of asmtools need to be rolled into jtreg before the test can operate directly on asmtools information rather than embedding class file bytes directly in the test.
—
Paul.
I’m seeing no issue with rcx being aliased in this code. Fred
On Oct 30, 2017, at 15:44, Paul Sandoz <paul.sandoz@oracle.com> wrote:
Hi,
Thanks for reviewing.
On 30 Oct 2017, at 11:05, Dmitry Samersoff <dmitry.samersoff@bell-sw.com> wrote:
Paul,
templateTable_x86.cpp:
564 const Register flags = rcx; 565 const Register rarg = NOT_LP64(rcx) LP64_ONLY(c_rarg1);
Should we use another register for rarg under NOT_LP64 ?
I think it should be ok, it i ain’t an expert here on the interpreter and the calling conventions, so please correct me.
Some more context:
+ const Register flags = rcx; + const Register rarg = NOT_LP64(rcx) LP64_ONLY(c_rarg1); + __ movl(rarg, (int)bytecode());
The current bytecode code is loaded into “rarg”
+ call_VM(obj, CAST_FROM_FN_PTR(address, InterpreterRuntime::resolve_ldc), rarg);
Then “rarg" is the argument to the call to InterpreterRuntime::resolve_ldc, after which it is no longer referred to.
+#ifndef _LP64 + // borrow rdi from locals + __ get_thread(rdi); + __ get_vm_result_2(flags, rdi); + __ restore_locals(); +#else + __ get_vm_result_2(flags, r15_thread); +#endif
The result from the call is then loaded into flags.
So i don’t think it matters in this case if rcx is aliased.
Paul.
-Dmitry
On 10/26/2017 08:03 PM, Paul Sandoz wrote:
Hi,
Please review the following patch for minimal dynamic constant support:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-hs/webrev/>
https://bugs.openjdk.java.net/browse/JDK-8186046 <https://bugs.openjdk.java.net/browse/JDK-8186046> https://bugs.openjdk.java.net/browse/JDK-8186209 <https://bugs.openjdk.java.net/browse/JDK-8186209>
This patch is based on the JDK 10 unified HotSpot repository. Testing so far looks good.
By minimal i mean just the support in the runtime for a dynamic constant pool entry to be referenced by a LDC instruction or a bootstrap method argument. Much of the work leverages the foundations built by invoke dynamic but is arguably simpler since resolution is less complex.
A small set of bootstrap methods will be proposed as a follow on issue for 10 (these are currently being refined in the amber repository).
Bootstrap method invocation has not changed (and the rules are the same for dynamic constants and indy). It is planned to enhance this in a further major release to support lazy resolution of bootstrap method arguments.
The CSR for the VM specification is here:
https://bugs.openjdk.java.net/browse/JDK-8189199 <https://bugs.openjdk.java.net/browse/JDK-8189199>
the j.l.invoke package documentation was also updated but please consider the VM specification as the definitive "source of truth" (we may clean up this area further later on so it becomes more informative, and that may also apply to duplicative text on MethodHandles/VarHandles).
Any AoT-related work will be deferred to a future release.
—
This patch only supports x64 platforms. There is a small set of changes specific to x64 (specifically to support null and primitives constants, as prior to this patch null was used as a sentinel for resolution and certain primitives types would never have been encountered, such as say byte).
We will need to follow up with the SPARC platform and it is hoped/anticipated that OpenJDK members responsible for other platforms (namely ARM and PPC) will separately provide patches.
—
Many of tests rely on an experimental byte code API that supports the generation of byte code with dynamic constants.
One test uses class file bytes produced from a modified version of asmtools. The modifications have now been pushed but a new version of asmtools need to be rolled into jtreg before the test can operate directly on asmtools information rather than embedding class file bytes directly in the test.
—
Paul.
Paul and Frederic, Thank you. One more question. Do we need to call verify_oop below? 509 { // Check for the null sentinel. ... 517 xorptr(result, result); // NULL object reference ... 521 if (VerifyOops) { 522 verify_oop(result); 523 } -Dmitry On 31.10.2017 00:56, Frederic Parain wrote:
I’m seeing no issue with rcx being aliased in this code.
Fred
On Oct 30, 2017, at 15:44, Paul Sandoz <paul.sandoz@oracle.com> wrote:
Hi,
Thanks for reviewing.
On 30 Oct 2017, at 11:05, Dmitry Samersoff <dmitry.samersoff@bell-sw.com> wrote:
Paul,
templateTable_x86.cpp:
564 const Register flags = rcx; 565 const Register rarg = NOT_LP64(rcx) LP64_ONLY(c_rarg1);
Should we use another register for rarg under NOT_LP64 ?
I think it should be ok, it i ain’t an expert here on the interpreter and the calling conventions, so please correct me.
Some more context:
+ const Register flags = rcx; + const Register rarg = NOT_LP64(rcx) LP64_ONLY(c_rarg1); + __ movl(rarg, (int)bytecode());
The current bytecode code is loaded into “rarg”
+ call_VM(obj, CAST_FROM_FN_PTR(address, InterpreterRuntime::resolve_ldc), rarg);
Then “rarg" is the argument to the call to InterpreterRuntime::resolve_ldc, after which it is no longer referred to.
+#ifndef _LP64 + // borrow rdi from locals + __ get_thread(rdi); + __ get_vm_result_2(flags, rdi); + __ restore_locals(); +#else + __ get_vm_result_2(flags, r15_thread); +#endif
The result from the call is then loaded into flags.
So i don’t think it matters in this case if rcx is aliased.
Paul.
-Dmitry
On 10/26/2017 08:03 PM, Paul Sandoz wrote:
Hi,
Please review the following patch for minimal dynamic constant support:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-hs/webrev/>
https://bugs.openjdk.java.net/browse/JDK-8186046 <https://bugs.openjdk.java.net/browse/JDK-8186046> https://bugs.openjdk.java.net/browse/JDK-8186209 <https://bugs.openjdk.java.net/browse/JDK-8186209>
This patch is based on the JDK 10 unified HotSpot repository. Testing so far looks good.
By minimal i mean just the support in the runtime for a dynamic constant pool entry to be referenced by a LDC instruction or a bootstrap method argument. Much of the work leverages the foundations built by invoke dynamic but is arguably simpler since resolution is less complex.
A small set of bootstrap methods will be proposed as a follow on issue for 10 (these are currently being refined in the amber repository).
Bootstrap method invocation has not changed (and the rules are the same for dynamic constants and indy). It is planned to enhance this in a further major release to support lazy resolution of bootstrap method arguments.
The CSR for the VM specification is here:
https://bugs.openjdk.java.net/browse/JDK-8189199 <https://bugs.openjdk.java.net/browse/JDK-8189199>
the j.l.invoke package documentation was also updated but please consider the VM specification as the definitive "source of truth" (we may clean up this area further later on so it becomes more informative, and that may also apply to duplicative text on MethodHandles/VarHandles).
Any AoT-related work will be deferred to a future release.
—
This patch only supports x64 platforms. There is a small set of changes specific to x64 (specifically to support null and primitives constants, as prior to this patch null was used as a sentinel for resolution and certain primitives types would never have been encountered, such as say byte).
We will need to follow up with the SPARC platform and it is hoped/anticipated that OpenJDK members responsible for other platforms (namely ARM and PPC) will separately provide patches.
—
Many of tests rely on an experimental byte code API that supports the generation of byte code with dynamic constants.
One test uses class file bytes produced from a modified version of asmtools. The modifications have now been pushed but a new version of asmtools need to be rolled into jtreg before the test can operate directly on asmtools information rather than embedding class file bytes directly in the test.
—
Paul.
On 31 Oct 2017, at 05:58, Dmitry Samersoff <dmitry.samersoff@bell-sw.com> wrote:
Paul and Frederic,
Thank you.
One more question. Do we need to call verify_oop below?
509 { // Check for the null sentinel. ... 517 xorptr(result, result); // NULL object reference ...
521 if (VerifyOops) { 522 verify_oop(result); 523 }
I believe it’s harmless. When the flag is on it eventually results in a call to the stub generated by generate_verify_oop: http://hg.openjdk.java.net/jdk10/hs/file/tip/src/hotspot/cpu/x86/stubGenerat... // make sure object is 'reasonable' __ testptr(rax, rax); __ jcc(Assembler::zero, exit); // if obj is NULL it is OK If the oop is null the verification will exit safely. Paul.
-Dmitry
On 31.10.2017 00:56, Frederic Parain wrote:
I’m seeing no issue with rcx being aliased in this code.
Fred
On Oct 30, 2017, at 15:44, Paul Sandoz <paul.sandoz@oracle.com> wrote:
Hi,
Thanks for reviewing.
On 30 Oct 2017, at 11:05, Dmitry Samersoff <dmitry.samersoff@bell-sw.com> wrote:
Paul,
templateTable_x86.cpp:
564 const Register flags = rcx; 565 const Register rarg = NOT_LP64(rcx) LP64_ONLY(c_rarg1);
Should we use another register for rarg under NOT_LP64 ?
I think it should be ok, it i ain’t an expert here on the interpreter and the calling conventions, so please correct me.
Some more context:
+ const Register flags = rcx; + const Register rarg = NOT_LP64(rcx) LP64_ONLY(c_rarg1); + __ movl(rarg, (int)bytecode());
The current bytecode code is loaded into “rarg”
+ call_VM(obj, CAST_FROM_FN_PTR(address, InterpreterRuntime::resolve_ldc), rarg);
Then “rarg" is the argument to the call to InterpreterRuntime::resolve_ldc, after which it is no longer referred to.
+#ifndef _LP64 + // borrow rdi from locals + __ get_thread(rdi); + __ get_vm_result_2(flags, rdi); + __ restore_locals(); +#else + __ get_vm_result_2(flags, r15_thread); +#endif
The result from the call is then loaded into flags.
So i don’t think it matters in this case if rcx is aliased.
Paul.
-Dmitry
On 10/26/2017 08:03 PM, Paul Sandoz wrote:
Hi,
Please review the following patch for minimal dynamic constant support:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-hs/webrev/>
https://bugs.openjdk.java.net/browse/JDK-8186046 <https://bugs.openjdk.java.net/browse/JDK-8186046> https://bugs.openjdk.java.net/browse/JDK-8186209 <https://bugs.openjdk.java.net/browse/JDK-8186209>
This patch is based on the JDK 10 unified HotSpot repository. Testing so far looks good.
By minimal i mean just the support in the runtime for a dynamic constant pool entry to be referenced by a LDC instruction or a bootstrap method argument. Much of the work leverages the foundations built by invoke dynamic but is arguably simpler since resolution is less complex.
A small set of bootstrap methods will be proposed as a follow on issue for 10 (these are currently being refined in the amber repository).
Bootstrap method invocation has not changed (and the rules are the same for dynamic constants and indy). It is planned to enhance this in a further major release to support lazy resolution of bootstrap method arguments.
The CSR for the VM specification is here:
https://bugs.openjdk.java.net/browse/JDK-8189199 <https://bugs.openjdk.java.net/browse/JDK-8189199>
the j.l.invoke package documentation was also updated but please consider the VM specification as the definitive "source of truth" (we may clean up this area further later on so it becomes more informative, and that may also apply to duplicative text on MethodHandles/VarHandles).
Any AoT-related work will be deferred to a future release.
—
This patch only supports x64 platforms. There is a small set of changes specific to x64 (specifically to support null and primitives constants, as prior to this patch null was used as a sentinel for resolution and certain primitives types would never have been encountered, such as say byte).
We will need to follow up with the SPARC platform and it is hoped/anticipated that OpenJDK members responsible for other platforms (namely ARM and PPC) will separately provide patches.
—
Many of tests rely on an experimental byte code API that supports the generation of byte code with dynamic constants.
One test uses class file bytes produced from a modified version of asmtools. The modifications have now been pushed but a new version of asmtools need to be rolled into jtreg before the test can operate directly on asmtools information rather than embedding class file bytes directly in the test.
—
Paul.
Paul, Thank you! -Dmitry On 31.10.2017 20:32, Paul Sandoz wrote:
On 31 Oct 2017, at 05:58, Dmitry Samersoff <dmitry.samersoff@bell-sw.com> wrote:
Paul and Frederic,
Thank you.
One more question. Do we need to call verify_oop below?
509 { // Check for the null sentinel. ... 517 xorptr(result, result); // NULL object reference ...
521 if (VerifyOops) { 522 verify_oop(result); 523 }
I believe it’s harmless.
When the flag is on it eventually results in a call to the stub generated by generate_verify_oop:
http://hg.openjdk.java.net/jdk10/hs/file/tip/src/hotspot/cpu/x86/stubGenerat...
// make sure object is 'reasonable' __ testptr(rax, rax); __ jcc(Assembler::zero, exit); // if obj is NULL it is OK
If the oop is null the verification will exit safely.
Paul.
-Dmitry
On 31.10.2017 00:56, Frederic Parain wrote:
I’m seeing no issue with rcx being aliased in this code.
Fred
On Oct 30, 2017, at 15:44, Paul Sandoz <paul.sandoz@oracle.com> wrote:
Hi,
Thanks for reviewing.
On 30 Oct 2017, at 11:05, Dmitry Samersoff <dmitry.samersoff@bell-sw.com> wrote:
Paul,
templateTable_x86.cpp:
564 const Register flags = rcx; 565 const Register rarg = NOT_LP64(rcx) LP64_ONLY(c_rarg1);
Should we use another register for rarg under NOT_LP64 ?
I think it should be ok, it i ain’t an expert here on the interpreter and the calling conventions, so please correct me.
Some more context:
+ const Register flags = rcx; + const Register rarg = NOT_LP64(rcx) LP64_ONLY(c_rarg1); + __ movl(rarg, (int)bytecode());
The current bytecode code is loaded into “rarg”
+ call_VM(obj, CAST_FROM_FN_PTR(address, InterpreterRuntime::resolve_ldc), rarg);
Then “rarg" is the argument to the call to InterpreterRuntime::resolve_ldc, after which it is no longer referred to.
+#ifndef _LP64 + // borrow rdi from locals + __ get_thread(rdi); + __ get_vm_result_2(flags, rdi); + __ restore_locals(); +#else + __ get_vm_result_2(flags, r15_thread); +#endif
The result from the call is then loaded into flags.
So i don’t think it matters in this case if rcx is aliased.
Paul.
-Dmitry
On 10/26/2017 08:03 PM, Paul Sandoz wrote:
Hi,
Please review the following patch for minimal dynamic constant support:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-hs/webrev/>
https://bugs.openjdk.java.net/browse/JDK-8186046 <https://bugs.openjdk.java.net/browse/JDK-8186046> https://bugs.openjdk.java.net/browse/JDK-8186209 <https://bugs.openjdk.java.net/browse/JDK-8186209>
This patch is based on the JDK 10 unified HotSpot repository. Testing so far looks good.
By minimal i mean just the support in the runtime for a dynamic constant pool entry to be referenced by a LDC instruction or a bootstrap method argument. Much of the work leverages the foundations built by invoke dynamic but is arguably simpler since resolution is less complex.
A small set of bootstrap methods will be proposed as a follow on issue for 10 (these are currently being refined in the amber repository).
Bootstrap method invocation has not changed (and the rules are the same for dynamic constants and indy). It is planned to enhance this in a further major release to support lazy resolution of bootstrap method arguments.
The CSR for the VM specification is here:
https://bugs.openjdk.java.net/browse/JDK-8189199 <https://bugs.openjdk.java.net/browse/JDK-8189199>
the j.l.invoke package documentation was also updated but please consider the VM specification as the definitive "source of truth" (we may clean up this area further later on so it becomes more informative, and that may also apply to duplicative text on MethodHandles/VarHandles).
Any AoT-related work will be deferred to a future release.
—
This patch only supports x64 platforms. There is a small set of changes specific to x64 (specifically to support null and primitives constants, as prior to this patch null was used as a sentinel for resolution and certain primitives types would never have been encountered, such as say byte).
We will need to follow up with the SPARC platform and it is hoped/anticipated that OpenJDK members responsible for other platforms (namely ARM and PPC) will separately provide patches.
—
Many of tests rely on an experimental byte code API that supports the generation of byte code with dynamic constants.
One test uses class file bytes produced from a modified version of asmtools. The modifications have now been pushed but a new version of asmtools need to be rolled into jtreg before the test can operate directly on asmtools information rather than embedding class file bytes directly in the test.
—
Paul.
Lois identified and fixed a bug found when running the JCK VM tests. I merged the changes below into the current webrev. Paul. --- old/src/hotspot/share/interpreter/linkResolver.cpp 2017-10-31 11:56:30.541287505 -0400 +++ new/src/hotspot/share/interpreter/linkResolver.cpp 2017-10-31 11:56:29.215676272 -0400 @@ -301,14 +301,14 @@ if (vca_result != Reflection::ACCESS_OK) { ResourceMark rm(THREAD); char* msg = Reflection::verify_class_access_msg(ref_klass, - InstanceKlass::cast(sel_klass), + InstanceKlass::cast(base_klass), vca_result); if (msg == NULL) { Exceptions::fthrow( THREAD_AND_LOCATION, vmSymbols::java_lang_IllegalAccessError(), "failed to access class %s from class %s", - sel_klass->external_name(), + base_klass->external_name(), ref_klass->external_name()); } else { // Use module specific message returned by verify_class_access_msg().
On 26 Oct 2017, at 10:03, Paul Sandoz <Paul.Sandoz@oracle.com> wrote:
Hi,
Please review the following patch for minimal dynamic constant support:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-hs/webrev/>
https://bugs.openjdk.java.net/browse/JDK-8186046 <https://bugs.openjdk.java.net/browse/JDK-8186046> https://bugs.openjdk.java.net/browse/JDK-8186209 <https://bugs.openjdk.java.net/browse/JDK-8186209>
This patch is based on the JDK 10 unified HotSpot repository. Testing so far looks good.
By minimal i mean just the support in the runtime for a dynamic constant pool entry to be referenced by a LDC instruction or a bootstrap method argument. Much of the work leverages the foundations built by invoke dynamic but is arguably simpler since resolution is less complex.
A small set of bootstrap methods will be proposed as a follow on issue for 10 (these are currently being refined in the amber repository).
Bootstrap method invocation has not changed (and the rules are the same for dynamic constants and indy). It is planned to enhance this in a further major release to support lazy resolution of bootstrap method arguments.
The CSR for the VM specification is here:
https://bugs.openjdk.java.net/browse/JDK-8189199 <https://bugs.openjdk.java.net/browse/JDK-8189199>
the j.l.invoke package documentation was also updated but please consider the VM specification as the definitive "source of truth" (we may clean up this area further later on so it becomes more informative, and that may also apply to duplicative text on MethodHandles/VarHandles).
Any AoT-related work will be deferred to a future release.
—
This patch only supports x64 platforms. There is a small set of changes specific to x64 (specifically to support null and primitives constants, as prior to this patch null was used as a sentinel for resolution and certain primitives types would never have been encountered, such as say byte).
We will need to follow up with the SPARC platform and it is hoped/anticipated that OpenJDK members responsible for other platforms (namely ARM and PPC) will separately provide patches.
—
Many of tests rely on an experimental byte code API that supports the generation of byte code with dynamic constants.
One test uses class file bytes produced from a modified version of asmtools. The modifications have now been pushed but a new version of asmtools need to be rolled into jtreg before the test can operate directly on asmtools information rather than embedding class file bytes directly in the test.
—
Paul.
On 10/26/17 10:03 AM, Paul Sandoz wrote:
Hi,
Please review the following patch for minimal dynamic constant support:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-hs/webrev/>
I reviewed the non-hotspot change as a learning exercise (I am not close to j.l.invoke implementation). I assume DynamicConstant intends to be non-public in this patch, right? 30 public final class DynamicConstant Mandy
On 31 Oct 2017, at 14:43, mandy chung <mandy.chung@oracle.com> wrote:
On 10/26/17 10:03 AM, Paul Sandoz wrote:
Hi,
Please review the following patch for minimal dynamic constant support:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-hs/webrev/>
I reviewed the non-hotspot change as a learning exercise (I am not close to j.l.invoke implementation). I assume DynamicConstant intends to be non-public in this patch, right? 30 public final class DynamicConstant
Well spotted. More likely to be renamed to ConstantBootstraps when a minimal set of dynamic constant bootstraps will be proposed (likely this week) as a follow on patch. I’ll made it non-public in the updated webrev so as to keep this patch self-contained. Paul.
Thank you so much for doing this jointly. Couple of questions/comments: 1. thank you for making UseBootstrapCallInfo diagnostic 2. org/objectweb/asmClassReader.java why hardcoded 17 instead of ClassWriter.CONDY? 3. java/lang/invoke/package-info.java 128-134 Error handling could be clearer. My understanding is that if a LinkageError or subclass is thrown, this will be rethrown for all subsequent attempts. Other errors, e.g. VMError may retry resolution Also: after line 165: rules do not enable the JVM to share call sites. Is it worth noting anywhere that the Constant_Dynamic resolution is shared? 4. SD::find_java_mirror_for_type lines 2679+ ConDy should not be supporting CONSTANT_Class(“;” + FD) IIRC that is temporary for MVT but not part of ConDy’s spec, nor part of what should be checked in for 18.3 also line 2731 - remove comment about “Foo;” 5. constantTag.hpp ofBasicType: case T_OBJECT return constantTag(JVM_CONSTANT_String) ? why not JVM_CONSTANT_CLASS? 6. SD::find_java_mirror_for_type You have resolve_or_null/fail doing CHECK_(empty) which should check for a NULL constant_type_klass. This is followed by an explicit if (constant_type_klass == NULL) — is that needed? 7. reflection.cpp get_mirror_from_signature Looks like potential for side effects. Odd to set mirror_oop inside if (log_is_enabled) Is the intent to assert that k->java_mirror() == mirror_oop? Or is the issue that we have a make drop here and the potential for a safe point? If so, make a handle and extract it on return? — Or better yet - don’t make any changes to reflection.cpp - not necessary 8. BootstrapMethodInvoker Could you possibly add a comment that the default today is vmIsPushing and IsPullModeBSM is false? Or find a slightly different naming to make that clearer? 9. test/jdk/java/lang/invoke/common/test/java/lang/invoke/lib/InstructionHelper.java How would I write an ldc CONSTANT_Dynamic which referred to a bootstrap method that was not ACC_STATIC? Or was not ACC_PUBLIC? Or was Or did I read the invoke dynamic method incorrectly? thanks, Karen
On Oct 26, 2017, at 1:03 PM, Paul Sandoz <paul.sandoz@oracle.com> wrote:
Hi,
Please review the following patch for minimal dynamic constant support:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-hs/webrev/>
https://bugs.openjdk.java.net/browse/JDK-8186046 <https://bugs.openjdk.java.net/browse/JDK-8186046> https://bugs.openjdk.java.net/browse/JDK-8186209 <https://bugs.openjdk.java.net/browse/JDK-8186209>
This patch is based on the JDK 10 unified HotSpot repository. Testing so far looks good.
By minimal i mean just the support in the runtime for a dynamic constant pool entry to be referenced by a LDC instruction or a bootstrap method argument. Much of the work leverages the foundations built by invoke dynamic but is arguably simpler since resolution is less complex.
A small set of bootstrap methods will be proposed as a follow on issue for 10 (these are currently being refined in the amber repository).
Bootstrap method invocation has not changed (and the rules are the same for dynamic constants and indy). It is planned to enhance this in a further major release to support lazy resolution of bootstrap method arguments.
The CSR for the VM specification is here:
https://bugs.openjdk.java.net/browse/JDK-8189199 <https://bugs.openjdk.java.net/browse/JDK-8189199>
the j.l.invoke package documentation was also updated but please consider the VM specification as the definitive "source of truth" (we may clean up this area further later on so it becomes more informative, and that may also apply to duplicative text on MethodHandles/VarHandles).
Any AoT-related work will be deferred to a future release.
—
This patch only supports x64 platforms. There is a small set of changes specific to x64 (specifically to support null and primitives constants, as prior to this patch null was used as a sentinel for resolution and certain primitives types would never have been encountered, such as say byte).
We will need to follow up with the SPARC platform and it is hoped/anticipated that OpenJDK members responsible for other platforms (namely ARM and PPC) will separately provide patches.
—
Many of tests rely on an experimental byte code API that supports the generation of byte code with dynamic constants.
One test uses class file bytes produced from a modified version of asmtools. The modifications have now been pushed but a new version of asmtools need to be rolled into jtreg before the test can operate directly on asmtools information rather than embedding class file bytes directly in the test.
—
Paul.
On 3 Nov 2017, at 11:14, Karen Kinnear <karen.kinnear@oracle.com> wrote:
Thank you so much for doing this jointly.
Couple of questions/comments: 1. thank you for making UseBootstrapCallInfo diagnostic
2. org/objectweb/asmClassReader.java why hardcoded 17 instead of ClassWriter.CONDY?
I chose to make the absolute minimal amount of changes to our internal ASM to reduce possible conflicts as i anticipate that a new version of ASM with condy support will eventually be rolled in.
3. java/lang/invoke/package-info.java 128-134 Error handling could be clearer. My understanding is that if a LinkageError or subclass is thrown, this will be rethrown for all subsequent attempts. Other errors, e.g. VMError may retry resolution
Also: after line 165: rules do not enable the JVM to share call sites. Is it worth noting anywhere that the Constant_Dynamic resolution is shared?
4. SD::find_java_mirror_for_type lines 2679+ ConDy should not be supporting CONSTANT_Class(“;” + FD) IIRC that is temporary for MVT but not part of ConDy’s spec, nor part of what should be checked in for 18.3
Yes, i deleted lines 2678 to 2687 TempNewSymbol type_buf; if (type->utf8_length() > 1 && type->byte_at(0) == ';') { // Strip the quote, which may have come from CONSTANT_Class[";"+FD]. // (This logic corresponds to the use of "FieldType::is_obj" // in resolve_or_null. A field type can be unwrapped to a class // type and vice versa.) type = SymbolTable::new_symbol(type->as_C_string() + 1, type->utf8_length() - 1, CHECK_(empty)); type_buf = type; // will free later }
also line 2731 - remove comment about “Foo;”
Removed.
5. constantTag.hpp ofBasicType: case T_OBJECT return constantTag(JVM_CONSTANT_String) ? why not JVM_CONSTANT_CLASS?
We would have to ask John on that, i am guessing it does not matter much since round tripping is ok and it is the more benign of the mapping. I do find this sits awkwardly though, perhaps there is a more general cleanup that can follow in this regard?
6. SD::find_java_mirror_for_type You have resolve_or_null/fail doing CHECK_(empty) which should check for a NULL constant_type_klass. This is followed by an explicit if (constant_type_klass == NULL) — is that needed?
Can SD:resolve_or_null return a null value when HAS_PENDING_EXCEPTION=false? I see other usages that suggest this to be the case. Where as for resolve_or_fail: Klass* SystemDictionary::resolve_or_fail(Symbol* class_name, Handle class_loader, Handle protection_domain, bool throw_error, TRAPS) { Klass* klass = resolve_or_null(class_name, class_loader, protection_domain, THREAD); if (HAS_PENDING_EXCEPTION || klass == NULL) { // can return a null klass klass = handle_resolution_exception(class_name, throw_error, klass, THREAD); } return klass; } It suggests we can change the code from: if (failure_mode == SignatureStream::ReturnNull) { constant_type_klass = resolve_or_null(type, class_loader, protection_domain, CHECK_(empty)); } else { bool throw_error = (failure_mode == SignatureStream::NCDFError); constant_type_klass = resolve_or_fail(type, class_loader, protection_domain, throw_error, CHECK_(empty)); } if (constant_type_klass == NULL) { return Handle(); // report failure this way } to if (failure_mode == SignatureStream::ReturnNull) { constant_type_klass = resolve_or_null(type, class_loader, protection_domain, CHECK_(empty)); if (constant_type_klass == NULL) { return Handle(); // report failure this way } } else { bool throw_error = (failure_mode == SignatureStream::NCDFError); constant_type_klass = resolve_or_fail(type, class_loader, protection_domain, throw_error, CHECK_(empty)); } ?
7. reflection.cpp get_mirror_from_signature Looks like potential for side effects. Odd to set mirror_oop inside if (log_is_enabled) Is the intent to assert that k->java_mirror() == mirror_oop? Or is the issue that we have a make drop here and the potential for a safe point? If so, make a handle and extract it on return?
— Or better yet - don’t make any changes to reflection.cpp - not necessary
Lois, John? My recollection was John was attempting to tunnel things through a single method find_java_mirror_for_type, but i agree the setting of mirror_oop is odd.
8. BootstrapMethodInvoker Could you possibly add a comment that the default today is vmIsPushing and IsPullModeBSM is false? Or find a slightly different naming to make that clearer?
boolean pullMode = isPullModeBSM(bootstrapMethod); // default value is false boolean vmIsPushing = !staticArgumentsPulled(info); // default value is true ?
9. test/jdk/java/lang/invoke/common/test/java/lang/invoke/lib/InstructionHelper.java How would I write an ldc CONSTANT_Dynamic which referred to a bootstrap method that was not ACC_STATIC?
https://docs.oracle.com/javase/specs/jvms/se9/html/jvms-4.html#jvms-4.7.23 See the note under bootstrap_method_ref. The kind of method handle is constrained because of the arguments that are pushed on the stack before invocation. I believe it’s possible to have a constructor, but the declaring class would need to be a subtype of CallSite in the case of indy, and a subtype of the constant value type in the case of condy.
Or was not ACC_PUBLIC?
That’s dependent on the accessibility between the lookup class the the BSM declaring class.
Or was Or did I read the invoke dynamic method incorrectly?
By default, and for convenience, the InstructionHelper assumes the BSM is declared by the lookup class. I recently modified that to support the BSM being declared on another class, to test the minimal set of BSMs for condy (separate issue [1]). Note it’s always possible to use the bytecode API more directly. So we can easily add more -ve tests for non-accessible or non-static BSMs. Paul. [1] http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webre...
On 4/11/2017 7:28 AM, Paul Sandoz wrote:
On 3 Nov 2017, at 11:14, Karen Kinnear <karen.kinnear@oracle.com> wrote: 6. SD::find_java_mirror_for_type You have resolve_or_null/fail doing CHECK_(empty) which should check for a NULL constant_type_klass. This is followed by an explicit if (constant_type_klass == NULL) — is that needed?
Can SD:resolve_or_null return a null value when HAS_PENDING_EXCEPTION=false?
I don't believe it actually can - the only reason you would get NULL is if something went wrong, for which an exception must be pending. However, even the internal implementation underlying this seems unclear on that point e.g in SystemDictionary::resolve_instance_class_or_null we have: if (HAS_PENDING_EXCEPTION || k == NULL) { return NULL; } David -----
I see other usages that suggest this to be the case. Where as for resolve_or_fail:
Klass* SystemDictionary::resolve_or_fail(Symbol* class_name, Handle class_loader, Handle protection_domain, bool throw_error, TRAPS) { Klass* klass = resolve_or_null(class_name, class_loader, protection_domain, THREAD); if (HAS_PENDING_EXCEPTION || klass == NULL) { // can return a null klass klass = handle_resolution_exception(class_name, throw_error, klass, THREAD); } return klass; }
It suggests we can change the code from:
if (failure_mode == SignatureStream::ReturnNull) { constant_type_klass = resolve_or_null(type, class_loader, protection_domain, CHECK_(empty)); } else { bool throw_error = (failure_mode == SignatureStream::NCDFError); constant_type_klass = resolve_or_fail(type, class_loader, protection_domain, throw_error, CHECK_(empty)); } if (constant_type_klass == NULL) { return Handle(); // report failure this way }
to
if (failure_mode == SignatureStream::ReturnNull) { constant_type_klass = resolve_or_null(type, class_loader, protection_domain, CHECK_(empty)); if (constant_type_klass == NULL) { return Handle(); // report failure this way } } else { bool throw_error = (failure_mode == SignatureStream::NCDFError); constant_type_klass = resolve_or_fail(type, class_loader, protection_domain, throw_error, CHECK_(empty)); }
?
7. reflection.cpp get_mirror_from_signature Looks like potential for side effects. Odd to set mirror_oop inside if (log_is_enabled) Is the intent to assert that k->java_mirror() == mirror_oop? Or is the issue that we have a make drop here and the potential for a safe point? If so, make a handle and extract it on return?
— Or better yet - don’t make any changes to reflection.cpp - not necessary
Lois, John?
My recollection was John was attempting to tunnel things through a single method find_java_mirror_for_type, but i agree the setting of mirror_oop is odd.
8. BootstrapMethodInvoker Could you possibly add a comment that the default today is vmIsPushing and IsPullModeBSM is false? Or find a slightly different naming to make that clearer?
boolean pullMode = isPullModeBSM(bootstrapMethod); // default value is false boolean vmIsPushing = !staticArgumentsPulled(info); // default value is true
?
9. test/jdk/java/lang/invoke/common/test/java/lang/invoke/lib/InstructionHelper.java How would I write an ldc CONSTANT_Dynamic which referred to a bootstrap method that was not ACC_STATIC?
https://docs.oracle.com/javase/specs/jvms/se9/html/jvms-4.html#jvms-4.7.23
See the note under bootstrap_method_ref. The kind of method handle is constrained because of the arguments that are pushed on the stack before invocation. I believe it’s possible to have a constructor, but the declaring class would need to be a subtype of CallSite in the case of indy, and a subtype of the constant value type in the case of condy.
Or was not ACC_PUBLIC?
That’s dependent on the accessibility between the lookup class the the BSM declaring class.
Or was Or did I read the invoke dynamic method incorrectly?
By default, and for convenience, the InstructionHelper assumes the BSM is declared by the lookup class. I recently modified that to support the BSM being declared on another class, to test the minimal set of BSMs for condy (separate issue [1]). Note it’s always possible to use the bytecode API more directly.
So we can easily add more -ve tests for non-accessible or non-static BSMs.
Paul.
[1] http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webre...
On 5 Nov 2017, at 16:55, David Holmes <david.holmes@oracle.com> wrote:
On 4/11/2017 7:28 AM, Paul Sandoz wrote:
On 3 Nov 2017, at 11:14, Karen Kinnear <karen.kinnear@oracle.com> wrote: 6. SD::find_java_mirror_for_type You have resolve_or_null/fail doing CHECK_(empty) which should check for a NULL constant_type_klass. This is followed by an explicit if (constant_type_klass == NULL) — is that needed?
Can SD:resolve_or_null return a null value when HAS_PENDING_EXCEPTION=false?
I don't believe it actually can - the only reason you would get NULL is if something went wrong, for which an exception must be pending. However, even the internal implementation underlying this seems unclear on that point
Right, i am gonna leave things as they are for now unless we come up with a more definitive answer. Paul.
e.g in SystemDictionary::resolve_instance_class_or_null we have:
if (HAS_PENDING_EXCEPTION || k == NULL) { return NULL; }
David ——
Paul, Thank you for the explanations. Asking for your help in some test case construction at the end here:
3. java/lang/invoke/package-info.java 128-134 Error handling could be clearer. My understanding is that if a LinkageError or subclass is thrown, this will be rethrown for all subsequent attempts. Other errors, e.g. VMError may retry resolution I was WRONG here - this does match the JVMS. Apologies for the confusion.
9. test/jdk/java/lang/invoke/common/test/java/lang/invoke/lib/InstructionHelper.java How would I write an ldc CONSTANT_Dynamic which referred to a bootstrap method that was not ACC_STATIC?
https://docs.oracle.com/javase/specs/jvms/se9/html/jvms-4.html#jvms-4.7.23 <https://docs.oracle.com/javase/specs/jvms/se9/html/jvms-4.html#jvms-4.7.23>
See the note under bootstrap_method_ref. The kind of method handle is constrained because of the arguments that are pushed on the stack before invocation. I believe it’s possible to have a constructor, but the declaring class would need to be a subtype of CallSite in the case of indy, and a subtype of the constant value type in the case of condy.
Or was not ACC_PUBLIC?
That’s dependent on the accessibility between the lookup class the the BSM declaring class.
Or was Or did I read the invoke dynamic method incorrectly?
By default, and for convenience, the InstructionHelper assumes the BSM is declared by the lookup class. I recently modified that to support the BSM being declared on another class, to test the minimal set of BSMs for condy (separate issue [1]). Note it’s always possible to use the bytecode API more directly.
So we can easily add more -ve tests for non-accessible or non-static BSMs. Thank you - that is what we were trying to do - test BSM declared in another class, test in-accessible BSM and test non static method.
Could you help us figure out how to 1) invoke a constructor? 2) invoke a virtual method? How do you pass the receiver? thanks, Karen
Paul.
[1] http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webre... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8187742-condy-bootstraps/webrev/>
On 7 Nov 2017, at 12:04, Karen Kinnear <karen.kinnear@oracle.com> wrote:
Paul,
Thank you for the explanations. Asking for your help in some test case construction at the end here:
3. java/lang/invoke/package-info.java 128-134 Error handling could be clearer. My understanding is that if a LinkageError or subclass is thrown, this will be rethrown for all subsequent attempts. Other errors, e.g. VMError may retry resolution I was WRONG here - this does match the JVMS. Apologies for the confusion.
9. test/jdk/java/lang/invoke/common/test/java/lang/invoke/lib/InstructionHelper.java How would I write an ldc CONSTANT_Dynamic which referred to a bootstrap method that was not ACC_STATIC?
https://docs.oracle.com/javase/specs/jvms/se9/html/jvms-4.html#jvms-4.7.23
See the note under bootstrap_method_ref. The kind of method handle is constrained because of the arguments that are pushed on the stack before invocation. I believe it’s possible to have a constructor, but the declaring class would need to be a subtype of CallSite in the case of indy, and a subtype of the constant value type in the case of condy.
Or was not ACC_PUBLIC?
That’s dependent on the accessibility between the lookup class the the BSM declaring class.
Or was Or did I read the invoke dynamic method incorrectly?
By default, and for convenience, the InstructionHelper assumes the BSM is declared by the lookup class. I recently modified that to support the BSM being declared on another class, to test the minimal set of BSMs for condy (separate issue [1]). Note it’s always possible to use the bytecode API more directly.
So we can easily add more -ve tests for non-accessible or non-static BSMs.
I am wrong, i forgot i updated the API to remove the declaration of the BSM kind, and it only supports static BSMs.
Thank you - that is what we were trying to do - test BSM declared in another class, test in-accessible BSM and test non static method.
The latter (access to a BSM in another class) is easy now using the high-level construct of InstructionHelper.ldcDynamicConstant (see patch for the condy BSMs, which is also in the amber repo in the condy branch).
Could you help us figure out how to 1) invoke a constructor? 2) invoke a virtual method? How do you pass the receiver?
I think for these you will need to use ASM tools to massage the MH for the BSM e.g. develop for a class with a static BSM then adjust the constant pool entry for the associated MH. Paul.
Hi, The schedule of dynamic constants has been adjusted, the JEP is currently proposed to target for 11. There is risk to integrating a change such as constant dynamic this late in the 10 release schedule. Instead it's better, and more aligned with the new release process, to integrate early on into the main repository so it has time to bake. I don’t anticipate there will be significant changes to the existing reviews in progress (code, CSRs, VM specification), so we can keep the momentum, integrate, then iterate. Thanks, Paul.
On 26 Oct 2017, at 10:03, Paul Sandoz <Paul.Sandoz@oracle.com> wrote:
Hi,
Please review the following patch for minimal dynamic constant support:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-... <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-hs/webrev/>
https://bugs.openjdk.java.net/browse/JDK-8186046 <https://bugs.openjdk.java.net/browse/JDK-8186046> https://bugs.openjdk.java.net/browse/JDK-8186209 <https://bugs.openjdk.java.net/browse/JDK-8186209>
This patch is based on the JDK 10 unified HotSpot repository. Testing so far looks good.
By minimal i mean just the support in the runtime for a dynamic constant pool entry to be referenced by a LDC instruction or a bootstrap method argument. Much of the work leverages the foundations built by invoke dynamic but is arguably simpler since resolution is less complex.
A small set of bootstrap methods will be proposed as a follow on issue for 10 (these are currently being refined in the amber repository).
Bootstrap method invocation has not changed (and the rules are the same for dynamic constants and indy). It is planned to enhance this in a further major release to support lazy resolution of bootstrap method arguments.
The CSR for the VM specification is here:
https://bugs.openjdk.java.net/browse/JDK-8189199 <https://bugs.openjdk.java.net/browse/JDK-8189199>
the j.l.invoke package documentation was also updated but please consider the VM specification as the definitive "source of truth" (we may clean up this area further later on so it becomes more informative, and that may also apply to duplicative text on MethodHandles/VarHandles).
Any AoT-related work will be deferred to a future release.
—
This patch only supports x64 platforms. There is a small set of changes specific to x64 (specifically to support null and primitives constants, as prior to this patch null was used as a sentinel for resolution and certain primitives types would never have been encountered, such as say byte).
We will need to follow up with the SPARC platform and it is hoped/anticipated that OpenJDK members responsible for other platforms (namely ARM and PPC) will separately provide patches.
—
Many of tests rely on an experimental byte code API that supports the generation of byte code with dynamic constants.
One test uses class file bytes produced from a modified version of asmtools. The modifications have now been pushed but a new version of asmtools need to be rolled into jtreg before the test can operate directly on asmtools information rather than embedding class file bytes directly in the test.
—
Paul.
2017/11/20 11:56:42 -0800, paul.sandoz@oracle.com:
The schedule of dynamic constants has been adjusted, the JEP is currently proposed to target for 11.
There is risk to integrating a change such as constant dynamic this late in the 10 release schedule. Instead it's better, and more aligned with the new release process, to integrate early on into the main repository so it has time to bake.
Thanks for the update. It's too bad that JEP 309 won't make JDK 10, but this sounds like the right decision. - Mark
participants (7)
-
David Holmes
-
Dmitry Samersoff
-
Frederic Parain
-
Karen Kinnear
-
mandy chung
-
mark.reinhold@oracle.com
-
Paul Sandoz