RFR 8047737 Move array component mirror to instance of java/lang/Class
Summary: Add field in java.lang.Class for componentType to simplify oop processing and intrinsics in JVM This is part of ongoing work to clean up oop pointers in the metadata and simplify the interface between the JDK j.l.C and the JVM. There's a performance benefit at the end of all of this if we can remove all oop pointers from metadata. mirror in Klass is the only one left after this full change. See bug https://bugs.openjdk.java.net/browse/JDK-8047737 There are a couple steps to this change because Hotspot testing is done with promoted JDKs. The first step is this webrev: http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_jdk/ http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot/ When the JDK is promoted, the code to remove ArrayKlass::_component_mirror will be changed under a new bug id. http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot_full Finally, a compatibility request and licensee notification will occur to remove the function JVM_GetComponentType. Performance testing was done that shows no difference in performance. The isArray() call is a compiler intrinsic which is now called instead of getComponentType, which was recognized as a compiler intrinsic. JDK jtreg testing, hotspot jtreg testing, hotspot NSK testing and jck8 tests were performed on both the change requested (1st one) and the full change. hotspot NSK tests were run on the hotspot-only change with a promoted JDK. Please send your comments. Thanks, Coleen
Hi Coleen, Your webrev links are to internal locations. David On 28/06/2014 5:24 AM, Coleen Phillimore wrote:
Summary: Add field in java.lang.Class for componentType to simplify oop processing and intrinsics in JVM
This is part of ongoing work to clean up oop pointers in the metadata and simplify the interface between the JDK j.l.C and the JVM. There's a performance benefit at the end of all of this if we can remove all oop pointers from metadata. mirror in Klass is the only one left after this full change.
See bug https://bugs.openjdk.java.net/browse/JDK-8047737
There are a couple steps to this change because Hotspot testing is done with promoted JDKs. The first step is this webrev:
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_jdk/ http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot/
When the JDK is promoted, the code to remove ArrayKlass::_component_mirror will be changed under a new bug id.
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot_full
Finally, a compatibility request and licensee notification will occur to remove the function JVM_GetComponentType.
Performance testing was done that shows no difference in performance. The isArray() call is a compiler intrinsic which is now called instead of getComponentType, which was recognized as a compiler intrinsic.
JDK jtreg testing, hotspot jtreg testing, hotspot NSK testing and jck8 tests were performed on both the change requested (1st one) and the full change.
hotspot NSK tests were run on the hotspot-only change with a promoted JDK.
Please send your comments.
Thanks, Coleen
On 6/30/14, 1:55 AM, David Holmes wrote:
Hi Coleen,
Your webrev links are to internal locations.
Sorry, I cut/pasted the wrong links. They are: http://cr.openjdk.java.net/~coleenp/8047737_jdk/ http://cr.openjdk.java.net/~coleenp/8047737_hotspot/ and the full version http://cr.openjdk.java.net/~coleenp/8047737_hotspot/ Thank you for pointing this out David. Coleen
David
On 28/06/2014 5:24 AM, Coleen Phillimore wrote:
Summary: Add field in java.lang.Class for componentType to simplify oop processing and intrinsics in JVM
This is part of ongoing work to clean up oop pointers in the metadata and simplify the interface between the JDK j.l.C and the JVM. There's a performance benefit at the end of all of this if we can remove all oop pointers from metadata. mirror in Klass is the only one left after this full change.
See bug https://bugs.openjdk.java.net/browse/JDK-8047737
There are a couple steps to this change because Hotspot testing is done with promoted JDKs. The first step is this webrev:
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_jdk/ http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot/
When the JDK is promoted, the code to remove ArrayKlass::_component_mirror will be changed under a new bug id.
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot_full
Finally, a compatibility request and licensee notification will occur to remove the function JVM_GetComponentType.
Performance testing was done that shows no difference in performance. The isArray() call is a compiler intrinsic which is now called instead of getComponentType, which was recognized as a compiler intrinsic.
JDK jtreg testing, hotspot jtreg testing, hotspot NSK testing and jck8 tests were performed on both the change requested (1st one) and the full change.
hotspot NSK tests were run on the hotspot-only change with a promoted JDK.
Please send your comments.
Thanks, Coleen
On 30/06/2014 14:42, Coleen Phillimore wrote:
On 6/30/14, 1:55 AM, David Holmes wrote:
Hi Coleen,
Your webrev links are to internal locations.
Sorry, I cut/pasted the wrong links. They are:
http://cr.openjdk.java.net/~coleenp/8047737_jdk/ http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
First step looks good.
and the full version
The correct link to the full version seems to be: http://cr.openjdk.java.net/~coleenp/8047737_hotspot_full/ I haven't reviewed the full version yet. Fred
Thank you for pointing this out David.
Coleen
David
On 28/06/2014 5:24 AM, Coleen Phillimore wrote:
Summary: Add field in java.lang.Class for componentType to simplify oop processing and intrinsics in JVM
This is part of ongoing work to clean up oop pointers in the metadata and simplify the interface between the JDK j.l.C and the JVM. There's a performance benefit at the end of all of this if we can remove all oop pointers from metadata. mirror in Klass is the only one left after this full change.
See bug https://bugs.openjdk.java.net/browse/JDK-8047737
There are a couple steps to this change because Hotspot testing is done with promoted JDKs. The first step is this webrev:
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_jdk/ http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot/
When the JDK is promoted, the code to remove ArrayKlass::_component_mirror will be changed under a new bug id.
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot_full
Finally, a compatibility request and licensee notification will occur to remove the function JVM_GetComponentType.
Performance testing was done that shows no difference in performance. The isArray() call is a compiler intrinsic which is now called instead of getComponentType, which was recognized as a compiler intrinsic.
JDK jtreg testing, hotspot jtreg testing, hotspot NSK testing and jck8 tests were performed on both the change requested (1st one) and the full change.
hotspot NSK tests were run on the hotspot-only change with a promoted JDK.
Please send your comments.
Thanks, Coleen
-- Frederic Parain - Oracle Grenoble Engineering Center - France Phone: +33 4 76 18 81 17 Email: Frederic.Parain@oracle.com
Thank you, Fred, and thanks for correcting the link below. Coleen On 6/30/14, 10:27 AM, Frederic Parain wrote:
On 30/06/2014 14:42, Coleen Phillimore wrote:
On 6/30/14, 1:55 AM, David Holmes wrote:
Hi Coleen,
Your webrev links are to internal locations.
Sorry, I cut/pasted the wrong links. They are:
http://cr.openjdk.java.net/~coleenp/8047737_jdk/ http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
First step looks good.
and the full version
The correct link to the full version seems to be: http://cr.openjdk.java.net/~coleenp/8047737_hotspot_full/
I haven't reviewed the full version yet.
Fred
Thank you for pointing this out David.
Coleen
David
On 28/06/2014 5:24 AM, Coleen Phillimore wrote:
Summary: Add field in java.lang.Class for componentType to simplify oop processing and intrinsics in JVM
This is part of ongoing work to clean up oop pointers in the metadata and simplify the interface between the JDK j.l.C and the JVM. There's a performance benefit at the end of all of this if we can remove all oop pointers from metadata. mirror in Klass is the only one left after this full change.
See bug https://bugs.openjdk.java.net/browse/JDK-8047737
There are a couple steps to this change because Hotspot testing is done with promoted JDKs. The first step is this webrev:
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_jdk/ http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot/
When the JDK is promoted, the code to remove ArrayKlass::_component_mirror will be changed under a new bug id.
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot_full
Finally, a compatibility request and licensee notification will occur to remove the function JVM_GetComponentType.
Performance testing was done that shows no difference in performance. The isArray() call is a compiler intrinsic which is now called instead of getComponentType, which was recognized as a compiler intrinsic.
JDK jtreg testing, hotspot jtreg testing, hotspot NSK testing and jck8 tests were performed on both the change requested (1st one) and the full change.
hotspot NSK tests were run on the hotspot-only change with a promoted JDK.
Please send your comments.
Thanks, Coleen
private Class(ClassLoader loader) { // Initialize final field for classLoader. The initialization value of non-null // prevents future JIT optimizations from assuming this final field is null. classLoader = loader; + componentType = null; } Are we worried about the same optimization? + compute_optional_offset(_component_mirror_offset, + klass_oop, vmSymbols::componentType_name(), + vmSymbols::class_signature()); Is there a followup cleanup to make it non-optional? Or, are you waiting for JPRT to be able to push hotspot and jdk changes together? On Jun 30, 2014, at 5:42 AM, Coleen Phillimore <coleen.phillimore@oracle.com> wrote:
On 6/30/14, 1:55 AM, David Holmes wrote:
Hi Coleen,
Your webrev links are to internal locations.
Sorry, I cut/pasted the wrong links. They are:
http://cr.openjdk.java.net/~coleenp/8047737_jdk/ http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
and the full version
http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
Thank you for pointing this out David.
Coleen
David
On 28/06/2014 5:24 AM, Coleen Phillimore wrote:
Summary: Add field in java.lang.Class for componentType to simplify oop processing and intrinsics in JVM
This is part of ongoing work to clean up oop pointers in the metadata and simplify the interface between the JDK j.l.C and the JVM. There's a performance benefit at the end of all of this if we can remove all oop pointers from metadata. mirror in Klass is the only one left after this full change.
See bug https://bugs.openjdk.java.net/browse/JDK-8047737
There are a couple steps to this change because Hotspot testing is done with promoted JDKs. The first step is this webrev:
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_jdk/ http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot/
When the JDK is promoted, the code to remove ArrayKlass::_component_mirror will be changed under a new bug id.
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot_full
Finally, a compatibility request and licensee notification will occur to remove the function JVM_GetComponentType.
Performance testing was done that shows no difference in performance. The isArray() call is a compiler intrinsic which is now called instead of getComponentType, which was recognized as a compiler intrinsic.
JDK jtreg testing, hotspot jtreg testing, hotspot NSK testing and jck8 tests were performed on both the change requested (1st one) and the full change.
hotspot NSK tests were run on the hotspot-only change with a promoted JDK.
Please send your comments.
Thanks, Coleen
On 6/30/14, 3:50 PM, Christian Thalinger wrote:
private Class(ClassLoader loader) { // Initialize final field for classLoader. The initialization value of non-null // prevents future JIT optimizations from assuming this final field is null. classLoader = loader; + componentType = null; }
Are we worried about the same optimization?
I don't know if I was justified in worrying about the optimization in the first place. Since getComponentType() is conditional, I wasn't worried. But it should be consistent. Maybe I should revert the classLoader constructor change, now that I fixed all the tests not to care. What do people think?
+ compute_optional_offset(_component_mirror_offset, + klass_oop, vmSymbols::componentType_name(), + vmSymbols::class_signature());
Is there a followup cleanup to make it non-optional? Or, are you waiting for JPRT to be able to push hotspot and jdk changes together?
Yes, please look at the _full webrev. That has the non-optional changes in it and the follow on changes to remove getComponentType as an intrinsic in C2 (will file new RFE). I really would like a compiler person to comment on it. Thanks so much, Coleen
On Jun 30, 2014, at 5:42 AM, Coleen Phillimore <coleen.phillimore@oracle.com <mailto:coleen.phillimore@oracle.com>> wrote:
On 6/30/14, 1:55 AM, David Holmes wrote:
Hi Coleen,
Your webrev links are to internal locations.
Sorry, I cut/pasted the wrong links. They are:
http://cr.openjdk.java.net/~coleenp/8047737_jdk/ <http://cr.openjdk.java.net/%7Ecoleenp/8047737_jdk/> http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
and the full version
http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
Thank you for pointing this out David.
Coleen
David
On 28/06/2014 5:24 AM, Coleen Phillimore wrote:
Summary: Add field in java.lang.Class for componentType to simplify oop processing and intrinsics in JVM
This is part of ongoing work to clean up oop pointers in the metadata and simplify the interface between the JDK j.l.C and the JVM. There's a performance benefit at the end of all of this if we can remove all oop pointers from metadata. mirror in Klass is the only one left after this full change.
See bug https://bugs.openjdk.java.net/browse/JDK-8047737
There are a couple steps to this change because Hotspot testing is done with promoted JDKs. The first step is this webrev:
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_jdk/ http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot/
When the JDK is promoted, the code to remove ArrayKlass::_component_mirror will be changed under a new bug id.
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot_full
Finally, a compatibility request and licensee notification will occur to remove the function JVM_GetComponentType.
Performance testing was done that shows no difference in performance. The isArray() call is a compiler intrinsic which is now called instead of getComponentType, which was recognized as a compiler intrinsic.
JDK jtreg testing, hotspot jtreg testing, hotspot NSK testing and jck8 tests were performed on both the change requested (1st one) and the full change.
hotspot NSK tests were run on the hotspot-only change with a promoted JDK.
Please send your comments.
Thanks, Coleen
On Jun 30, 2014, at 1:06 PM, Coleen Phillimore <coleen.phillimore@oracle.com> wrote:
On 6/30/14, 3:50 PM, Christian Thalinger wrote:
private Class(ClassLoader loader) { // Initialize final field for classLoader. The initialization value of non-null // prevents future JIT optimizations from assuming this final field is null. classLoader = loader; + componentType = null; }
Are we worried about the same optimization?
I don't know if I was justified in worrying about the optimization in the first place. Since getComponentType() is conditional, I wasn't worried.
But it should be consistent. Maybe I should revert the classLoader constructor change, now that I fixed all the tests not to care. What do people think?
+ compute_optional_offset(_component_mirror_offset, + klass_oop, vmSymbols::componentType_name(), + vmSymbols::class_signature());
Is there a followup cleanup to make it non-optional? Or, are you waiting for JPRT to be able to push hotspot and jdk changes together?
Yes, please look at the _full webrev. That has the non-optional changes in it and the follow on changes to remove getComponentType as an intrinsic in C2 (will file new RFE). I really would like a compiler person to comment on it.
Sorry, I missed that. Yes, the compiler changes look good.
Thanks so much, Coleen
On Jun 30, 2014, at 5:42 AM, Coleen Phillimore <coleen.phillimore@oracle.com> wrote:
On 6/30/14, 1:55 AM, David Holmes wrote:
Hi Coleen,
Your webrev links are to internal locations.
Sorry, I cut/pasted the wrong links. They are:
http://cr.openjdk.java.net/~coleenp/8047737_jdk/ http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
and the full version
http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
Thank you for pointing this out David.
Coleen
David
On 28/06/2014 5:24 AM, Coleen Phillimore wrote:
Summary: Add field in java.lang.Class for componentType to simplify oop processing and intrinsics in JVM
This is part of ongoing work to clean up oop pointers in the metadata and simplify the interface between the JDK j.l.C and the JVM. There's a performance benefit at the end of all of this if we can remove all oop pointers from metadata. mirror in Klass is the only one left after this full change.
See bug https://bugs.openjdk.java.net/browse/JDK-8047737
There are a couple steps to this change because Hotspot testing is done with promoted JDKs. The first step is this webrev:
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_jdk/ http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot/
When the JDK is promoted, the code to remove ArrayKlass::_component_mirror will be changed under a new bug id.
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot_full
Finally, a compatibility request and licensee notification will occur to remove the function JVM_GetComponentType.
Performance testing was done that shows no difference in performance. The isArray() call is a compiler intrinsic which is now called instead of getComponentType, which was recognized as a compiler intrinsic.
JDK jtreg testing, hotspot jtreg testing, hotspot NSK testing and jck8 tests were performed on both the change requested (1st one) and the full change.
hotspot NSK tests were run on the hotspot-only change with a promoted JDK.
Please send your comments.
Thanks, Coleen
On 6/30/14, 3:50 PM, Christian Thalinger wrote:
private Class(ClassLoader loader) { // Initialize final field for classLoader. The initialization value of non-null // prevents future JIT optimizations from assuming this final field is null. classLoader = loader; + componentType = null; }
Are we worried about the same optimization?
Hi, I've decided to make them consistent and add another parameter to the Class constructor. http://cr.openjdk.java.net/~coleenp/8047737_jdk_2/ Thanks, Coleen
+ compute_optional_offset(_component_mirror_offset, + klass_oop, vmSymbols::componentType_name(), + vmSymbols::class_signature());
Is there a followup cleanup to make it non-optional? Or, are you waiting for JPRT to be able to push hotspot and jdk changes together?
On Jun 30, 2014, at 5:42 AM, Coleen Phillimore <coleen.phillimore@oracle.com <mailto:coleen.phillimore@oracle.com>> wrote:
On 6/30/14, 1:55 AM, David Holmes wrote:
Hi Coleen,
Your webrev links are to internal locations.
Sorry, I cut/pasted the wrong links. They are:
http://cr.openjdk.java.net/~coleenp/8047737_jdk/ <http://cr.openjdk.java.net/%7Ecoleenp/8047737_jdk/> http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
and the full version
http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
Thank you for pointing this out David.
Coleen
David
On 28/06/2014 5:24 AM, Coleen Phillimore wrote:
Summary: Add field in java.lang.Class for componentType to simplify oop processing and intrinsics in JVM
This is part of ongoing work to clean up oop pointers in the metadata and simplify the interface between the JDK j.l.C and the JVM. There's a performance benefit at the end of all of this if we can remove all oop pointers from metadata. mirror in Klass is the only one left after this full change.
See bug https://bugs.openjdk.java.net/browse/JDK-8047737
There are a couple steps to this change because Hotspot testing is done with promoted JDKs. The first step is this webrev:
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_jdk/ http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot/
When the JDK is promoted, the code to remove ArrayKlass::_component_mirror will be changed under a new bug id.
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot_full
Finally, a compatibility request and licensee notification will occur to remove the function JVM_GetComponentType.
Performance testing was done that shows no difference in performance. The isArray() call is a compiler intrinsic which is now called instead of getComponentType, which was recognized as a compiler intrinsic.
JDK jtreg testing, hotspot jtreg testing, hotspot NSK testing and jck8 tests were performed on both the change requested (1st one) and the full change.
hotspot NSK tests were run on the hotspot-only change with a promoted JDK.
Please send your comments.
Thanks, Coleen
On Jun 30, 2014, at 5:50 PM, Coleen Phillimore <coleen.phillimore@oracle.com> wrote:
On 6/30/14, 3:50 PM, Christian Thalinger wrote:
private Class(ClassLoader loader) { // Initialize final field for classLoader. The initialization value of non-null // prevents future JIT optimizations from assuming this final field is null. classLoader = loader; + componentType = null; }
Are we worried about the same optimization?
Hi, I've decided to make them consistent and add another parameter to the Class constructor.
Thanks.
Thanks, Coleen
+ compute_optional_offset(_component_mirror_offset, + klass_oop, vmSymbols::componentType_name(), + vmSymbols::class_signature());
Is there a followup cleanup to make it non-optional? Or, are you waiting for JPRT to be able to push hotspot and jdk changes together?
On Jun 30, 2014, at 5:42 AM, Coleen Phillimore <coleen.phillimore@oracle.com <mailto:coleen.phillimore@oracle.com>> wrote:
On 6/30/14, 1:55 AM, David Holmes wrote:
Hi Coleen,
Your webrev links are to internal locations.
Sorry, I cut/pasted the wrong links. They are:
http://cr.openjdk.java.net/~coleenp/8047737_jdk/ <http://cr.openjdk.java.net/%7Ecoleenp/8047737_jdk/> http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
and the full version
http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
Thank you for pointing this out David.
Coleen
David
On 28/06/2014 5:24 AM, Coleen Phillimore wrote:
Summary: Add field in java.lang.Class for componentType to simplify oop processing and intrinsics in JVM
This is part of ongoing work to clean up oop pointers in the metadata and simplify the interface between the JDK j.l.C and the JVM. There's a performance benefit at the end of all of this if we can remove all oop pointers from metadata. mirror in Klass is the only one left after this full change.
See bug https://bugs.openjdk.java.net/browse/JDK-8047737
There are a couple steps to this change because Hotspot testing is done with promoted JDKs. The first step is this webrev:
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_jdk/ http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot/
When the JDK is promoted, the code to remove ArrayKlass::_component_mirror will be changed under a new bug id.
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot_full
Finally, a compatibility request and licensee notification will occur to remove the function JVM_GetComponentType.
Performance testing was done that shows no difference in performance. The isArray() call is a compiler intrinsic which is now called instead of getComponentType, which was recognized as a compiler intrinsic.
JDK jtreg testing, hotspot jtreg testing, hotspot NSK testing and jck8 tests were performed on both the change requested (1st one) and the full change.
hotspot NSK tests were run on the hotspot-only change with a promoted JDK.
Please send your comments.
Thanks, Coleen
Thank you! Coleen On 7/1/14, 12:51 AM, Christian Thalinger wrote:
On Jun 30, 2014, at 5:50 PM, Coleen Phillimore <coleen.phillimore@oracle.com> wrote:
On 6/30/14, 3:50 PM, Christian Thalinger wrote:
private Class(ClassLoader loader) { // Initialize final field for classLoader. The initialization value of non-null // prevents future JIT optimizations from assuming this final field is null. classLoader = loader; + componentType = null; }
Are we worried about the same optimization?
Hi, I've decided to make them consistent and add another parameter to the Class constructor.
Thanks, Coleen
+ compute_optional_offset(_component_mirror_offset, + klass_oop, vmSymbols::componentType_name(), + vmSymbols::class_signature());
Is there a followup cleanup to make it non-optional? Or, are you waiting for JPRT to be able to push hotspot and jdk changes together?
On Jun 30, 2014, at 5:42 AM, Coleen Phillimore <coleen.phillimore@oracle.com <mailto:coleen.phillimore@oracle.com>> wrote:
On 6/30/14, 1:55 AM, David Holmes wrote:
Hi Coleen,
Your webrev links are to internal locations. Sorry, I cut/pasted the wrong links. They are:
http://cr.openjdk.java.net/~coleenp/8047737_jdk/ <http://cr.openjdk.java.net/%7Ecoleenp/8047737_jdk/> http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
and the full version
http://cr.openjdk.java.net/~coleenp/8047737_hotspot/
Thank you for pointing this out David.
Coleen
David
On 28/06/2014 5:24 AM, Coleen Phillimore wrote:
Summary: Add field in java.lang.Class for componentType to simplify oop processing and intrinsics in JVM
This is part of ongoing work to clean up oop pointers in the metadata and simplify the interface between the JDK j.l.C and the JVM. There's a performance benefit at the end of all of this if we can remove all oop pointers from metadata. mirror in Klass is the only one left after this full change.
See bug https://bugs.openjdk.java.net/browse/JDK-8047737
There are a couple steps to this change because Hotspot testing is done with promoted JDKs. The first step is this webrev:
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_jdk/ http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot/
When the JDK is promoted, the code to remove ArrayKlass::_component_mirror will be changed under a new bug id.
http://oklahoma.us.oracle.com/~cphillim/webrev/8047737_hotspot_full
Finally, a compatibility request and licensee notification will occur to remove the function JVM_GetComponentType.
Performance testing was done that shows no difference in performance. The isArray() call is a compiler intrinsic which is now called instead of getComponentType, which was recognized as a compiler intrinsic.
JDK jtreg testing, hotspot jtreg testing, hotspot NSK testing and jck8 tests were performed on both the change requested (1st one) and the full change.
hotspot NSK tests were run on the hotspot-only change with a promoted JDK.
Please send your comments.
Thanks, Coleen
On 6/30/2014 9:51 PM, Christian Thalinger wrote:
On Jun 30, 2014, at 5:50 PM, Coleen Phillimore <coleen.phillimore@oracle.com> wrote:
On 6/30/14, 3:50 PM, Christian Thalinger wrote:
private Class(ClassLoader loader) { // Initialize final field for classLoader. The initialization value of non-null // prevents future JIT optimizations from assuming this final field is null. classLoader = loader; + componentType = null; }
Are we worried about the same optimization?
Hi, I've decided to make them consistent and add another parameter to the Class constructor.
The jdk change looks okay while I am beginning to think whether we really want to keep expanding this constructor to deal with this future JIT optimization (you will be moving more fields out from the VM to java.lang.Class). There are places in JDK initializing the final fields to null while the final field value is overridden via native/VM - e.g. System.in, System.out, etc. I would prefer reverting the classLoader constructor change to expanding the constructor for any new field being added. Handle it (and other places in JDK) when such JIT optimization comes. Mandy
On 07/02/2014 08:26 AM, Mandy Chung wrote:
On 6/30/2014 9:51 PM, Christian Thalinger wrote:
On Jun 30, 2014, at 5:50 PM, Coleen Phillimore <coleen.phillimore@oracle.com> wrote:
On 6/30/14, 3:50 PM, Christian Thalinger wrote:
private Class(ClassLoader loader) { // Initialize final field for classLoader. The initialization value of non-null // prevents future JIT optimizations from assuming this final field is null. classLoader = loader; + componentType = null; }
Are we worried about the same optimization?
Hi, I've decided to make them consistent and add another parameter to the Class constructor.
The jdk change looks okay while I am beginning to think whether we really want to keep expanding this constructor to deal with this future JIT optimization (you will be moving more fields out from the VM to java.lang.Class).
There are places in JDK initializing the final fields to null while the final field value is overridden via native/VM - e.g. System.in, System.out, etc. I would prefer reverting the classLoader constructor change to expanding the constructor for any new field being added. Handle it (and other places in JDK) when such JIT optimization comes.
Mandy
What about: private Class() { classLoader = none(); componentType = none(); ... } private <T> T none() { throw new Error(); } I think this should be resistant to future optimizations. Regards, Peter
On 07/02/2014 02:22 PM, Peter Levart wrote:
On 07/02/2014 08:26 AM, Mandy Chung wrote:
On 6/30/2014 9:51 PM, Christian Thalinger wrote:
On Jun 30, 2014, at 5:50 PM, Coleen Phillimore <coleen.phillimore@oracle.com> wrote:
On 6/30/14, 3:50 PM, Christian Thalinger wrote:
private Class(ClassLoader loader) { // Initialize final field for classLoader. The initialization value of non-null // prevents future JIT optimizations from assuming this final field is null. classLoader = loader; + componentType = null; }
Are we worried about the same optimization?
Hi, I've decided to make them consistent and add another parameter to the Class constructor.
The jdk change looks okay while I am beginning to think whether we really want to keep expanding this constructor to deal with this future JIT optimization (you will be moving more fields out from the VM to java.lang.Class).
There are places in JDK initializing the final fields to null while the final field value is overridden via native/VM - e.g. System.in, System.out, etc. I would prefer reverting the classLoader constructor change to expanding the constructor for any new field being added. Handle it (and other places in JDK) when such JIT optimization comes.
Mandy
What about:
private Class() { classLoader = none(); componentType = none(); ... }
private <T> T none() { throw new Error(); }
I think this should be resistant to future optimizations.
And you could even remove the special-casing in AccessibleObject.setAccessible0() then. Regards, Peter
Regards, Peter
On 07/02/2014 02:38 PM, Peter Levart wrote:
On 07/02/2014 02:22 PM, Peter Levart wrote:
On 07/02/2014 08:26 AM, Mandy Chung wrote:
On 6/30/2014 9:51 PM, Christian Thalinger wrote:
On Jun 30, 2014, at 5:50 PM, Coleen Phillimore <coleen.phillimore@oracle.com> wrote:
On 6/30/14, 3:50 PM, Christian Thalinger wrote:
private Class(ClassLoader loader) { // Initialize final field for classLoader. The initialization value of non-null // prevents future JIT optimizations from assuming this final field is null. classLoader = loader; + componentType = null; }
Are we worried about the same optimization?
Hi, I've decided to make them consistent and add another parameter to the Class constructor.
The jdk change looks okay while I am beginning to think whether we really want to keep expanding this constructor to deal with this future JIT optimization (you will be moving more fields out from the VM to java.lang.Class).
There are places in JDK initializing the final fields to null while the final field value is overridden via native/VM - e.g. System.in, System.out, etc. I would prefer reverting the classLoader constructor change to expanding the constructor for any new field being added. Handle it (and other places in JDK) when such JIT optimization comes.
Mandy
What about:
private Class() { classLoader = none(); componentType = none(); ... }
private <T> T none() { throw new Error(); }
I think this should be resistant to future optimizations.
And you could even remove the special-casing in AccessibleObject.setAccessible0() then.
Regards, Peter
I take it back. Such java.lang.Class instance would still be constructed and GC will see it.
Regards, Peter
On 7/2/14, 8:41 AM, Peter Levart wrote:
On 07/02/2014 02:38 PM, Peter Levart wrote:
On 07/02/2014 02:22 PM, Peter Levart wrote:
On 07/02/2014 08:26 AM, Mandy Chung wrote:
On 6/30/2014 9:51 PM, Christian Thalinger wrote:
On Jun 30, 2014, at 5:50 PM, Coleen Phillimore <coleen.phillimore@oracle.com> wrote:
On 6/30/14, 3:50 PM, Christian Thalinger wrote:
> private Class(ClassLoader loader) { > // Initialize final field for classLoader. The > initialization value of non-null > // prevents future JIT optimizations from assuming > this final field is null. > classLoader = loader; > + componentType = null; > } > > Are we worried about the same optimization? Hi, I've decided to make them consistent and add another parameter to the Class constructor.
The jdk change looks okay while I am beginning to think whether we really want to keep expanding this constructor to deal with this future JIT optimization (you will be moving more fields out from the VM to java.lang.Class).
There are places in JDK initializing the final fields to null while the final field value is overridden via native/VM - e.g. System.in, System.out, etc. I would prefer reverting the classLoader constructor change to expanding the constructor for any new field being added. Handle it (and other places in JDK) when such JIT optimization comes.
Mandy
What about:
private Class() { classLoader = none(); componentType = none(); ... }
private <T> T none() { throw new Error(); }
I think this should be resistant to future optimizations.
And you could even remove the special-casing in AccessibleObject.setAccessible0() then.
Regards, Peter
I take it back. Such java.lang.Class instance would still be constructed and GC will see it.
The setAccessible0 check is still needed because we do other things to the mirror inside the jvm. Coleen
Regards, Peter
Hi Mandy, The componentType field is the last one that I'm planning on moving out for now, so I'd like to keep the code as is. If more are added because of more performance opportunities, I think we can revisit this. I agree with Doug that we don't want any more special code like this in the JVM to disable these optimizations if they are ever implemented. Thank you for reviewing the code. Coleen On 7/2/14, 2:26 AM, Mandy Chung wrote:
On 6/30/2014 9:51 PM, Christian Thalinger wrote:
On Jun 30, 2014, at 5:50 PM, Coleen Phillimore <coleen.phillimore@oracle.com> wrote:
On 6/30/14, 3:50 PM, Christian Thalinger wrote:
private Class(ClassLoader loader) { // Initialize final field for classLoader. The initialization value of non-null // prevents future JIT optimizations from assuming this final field is null. classLoader = loader; + componentType = null; }
Are we worried about the same optimization?
Hi, I've decided to make them consistent and add another parameter to the Class constructor.
The jdk change looks okay while I am beginning to think whether we really want to keep expanding this constructor to deal with this future JIT optimization (you will be moving more fields out from the VM to java.lang.Class).
There are places in JDK initializing the final fields to null while the final field value is overridden via native/VM - e.g. System.in, System.out, etc. I would prefer reverting the classLoader constructor change to expanding the constructor for any new field being added. Handle it (and other places in JDK) when such JIT optimization comes.
Mandy
I wasn't aware of the VM special case of System.{in,out,err} fields that Doug pointed out (thanks. I should have checked). That's okay with me. Mandy On 7/2/2014 10:05 AM, Coleen Phillimore wrote:
Hi Mandy,
The componentType field is the last one that I'm planning on moving out for now, so I'd like to keep the code as is. If more are added because of more performance opportunities, I think we can revisit this.
I agree with Doug that we don't want any more special code like this in the JVM to disable these optimizations if they are ever implemented.
Thank you for reviewing the code. Coleen
On 7/2/14, 2:26 AM, Mandy Chung wrote:
On 6/30/2014 9:51 PM, Christian Thalinger wrote:
On Jun 30, 2014, at 5:50 PM, Coleen Phillimore <coleen.phillimore@oracle.com> wrote:
On 6/30/14, 3:50 PM, Christian Thalinger wrote:
private Class(ClassLoader loader) { // Initialize final field for classLoader. The initialization value of non-null // prevents future JIT optimizations from assuming this final field is null. classLoader = loader; + componentType = null; }
Are we worried about the same optimization?
Hi, I've decided to make them consistent and add another parameter to the Class constructor.
The jdk change looks okay while I am beginning to think whether we really want to keep expanding this constructor to deal with this future JIT optimization (you will be moving more fields out from the VM to java.lang.Class).
There are places in JDK initializing the final fields to null while the final field value is overridden via native/VM - e.g. System.in, System.out, etc. I would prefer reverting the classLoader constructor change to expanding the constructor for any new field being added. Handle it (and other places in JDK) when such JIT optimization comes.
Mandy
participants (6)
-
Christian Thalinger
-
Coleen Phillimore
-
David Holmes
-
Frederic Parain
-
Mandy Chung
-
Peter Levart