RFR(L): JDK-8057777 Cleanup of old and unused VM interfaces

[Frederic Parain]

Hi all,

Please review changes for bug JDK-8057777 "Cleanup of old
and unused VM interfaces"

CR:
https://bugs.openjdk.java.net/browse/JDK-8057777

This is basically a big cleanup of VM interfaces that are
not used anymore by the JDK but have been kept in our code
base for historical reasons (HotSpot Express for instance).
These changesets remove these interfaces from both the
JDK and the HotSpot side, and also perform some cleanup
on code that directly referenced the removed interfaces.

These changes do not modify the behavior of the Java
classes impacted by the cleanup.

VM interfaces removal has been approved by CCC and
a Release Note has been prepared that explicitly list
all the removed interfaces.

Testing: JPRT hotspot + core, vm.quick.testlist, jdk_core

Webrevs:
http://cr.openjdk.java.net/~fparain/8057777/

Thank you,

Fred

[Coleen Phillimore]

Fred,

I reviewed this change. It looks great. Some of the functions removed
seem to be not only unused but dangerous. Some of these I have made
changes to that I didn't realize that the JVM didn't use these
functions. Thank you for doing this!

Coleen

Post by Frederic Parain
Hi all,
Please review changes for bug JDK-8057777 "Cleanup of old
and unused VM interfaces"
https://bugs.openjdk.java.net/browse/JDK-8057777
This is basically a big cleanup of VM interfaces that are
not used anymore by the JDK but have been kept in our code
base for historical reasons (HotSpot Express for instance).
These changesets remove these interfaces from both the
JDK and the HotSpot side, and also perform some cleanup
on code that directly referenced the removed interfaces.
These changes do not modify the behavior of the Java
classes impacted by the cleanup.
VM interfaces removal has been approved by CCC and
a Release Note has been prepared that explicitly list
all the removed interfaces.
Testing: JPRT hotspot + core, vm.quick.testlist, jdk_core
http://cr.openjdk.java.net/~fparain/8057777/
Thank you,
Fred

[Frederic Parain]

Thanks Coleen,

Fred

Post by Coleen Phillimore
Fred,
I reviewed this change. It looks great. Some of the functions removed
seem to be not only unused but dangerous. Some of these I have made
changes to that I didn't realize that the JVM didn't use these
functions. Thank you for doing this!
Coleen

Post by Frederic Parain
Hi all,
Please review changes for bug JDK-8057777 "Cleanup of old
and unused VM interfaces"
https://bugs.openjdk.java.net/browse/JDK-8057777
This is basically a big cleanup of VM interfaces that are
not used anymore by the JDK but have been kept in our code
base for historical reasons (HotSpot Express for instance).
These changesets remove these interfaces from both the
JDK and the HotSpot side, and also perform some cleanup
on code that directly referenced the removed interfaces.
These changes do not modify the behavior of the Java
classes impacted by the cleanup.
VM interfaces removal has been approved by CCC and
a Release Note has been prepared that explicitly list
all the removed interfaces.
Testing: JPRT hotspot + core, vm.quick.testlist, jdk_core
http://cr.openjdk.java.net/~fparain/8057777/
Thank you,
Fred

[harold seigel]

Hi Fred,

It's great to see all this code go away.

Since JVM_AllocateNewObject() is getting removed, can
JavaCalls::call_default_constructor() also be removed?

Also, why were JVM_DesiredAssertionStatus and JVM_DoPrivileged moved in
make/solaris/makefiles/mapfile-vers ? They are no longer in
alphabetical order.

Thanks, Harold

Post by Frederic Parain
Thanks Coleen,
Fred

Post by Coleen Phillimore
Fred,
I reviewed this change. It looks great. Some of the functions removed
seem to be not only unused but dangerous. Some of these I have made
changes to that I didn't realize that the JVM didn't use these
functions. Thank you for doing this!
Coleen

Post by Frederic Parain
Hi all,
Please review changes for bug JDK-8057777 "Cleanup of old
and unused VM interfaces"
https://bugs.openjdk.java.net/browse/JDK-8057777
This is basically a big cleanup of VM interfaces that are
not used anymore by the JDK but have been kept in our code
base for historical reasons (HotSpot Express for instance).
These changesets remove these interfaces from both the
JDK and the HotSpot side, and also perform some cleanup
on code that directly referenced the removed interfaces.
These changes do not modify the behavior of the Java
classes impacted by the cleanup.
VM interfaces removal has been approved by CCC and
a Release Note has been prepared that explicitly list
all the removed interfaces.
Testing: JPRT hotspot + core, vm.quick.testlist, jdk_core
http://cr.openjdk.java.net/~fparain/8057777/
Thank you,
Fred

[Frederic Parain]

Harold,

Thank you for the review and your feedback.

I didn't try to remove all dead code from JVM entries
removal, just the most obvious dead code pieces.
Thank you for spotting that JavaCalls::call_default_constructor()
could be safely removed. I removed it in the new webrev.

Regarding mapfiles, I played with JVM_DefineClassWithSources
and DefineClassWithSourcesCond, and it seems that I didn't
restore the mapfile correctly. This is now fixed.

New HoSpot webrev:
http://cr.openjdk.java.net/~fparain/8057777/hotspot_v02/

Thank you,

Fred

Post by harold seigel
Hi Fred,
It's great to see all this code go away.
Since JVM_AllocateNewObject() is getting removed, can
JavaCalls::call_default_constructor() also be removed?
Also, why were JVM_DesiredAssertionStatus and JVM_DoPrivileged moved in
make/solaris/makefiles/mapfile-vers ? They are no longer in
alphabetical order.
Thanks, Harold

Post by Frederic Parain
Thanks Coleen,
Fred

Post by Coleen Phillimore
Fred,
I reviewed this change. It looks great. Some of the functions removed
seem to be not only unused but dangerous. Some of these I have made
changes to that I didn't realize that the JVM didn't use these
functions. Thank you for doing this!
Coleen

Post by Frederic Parain
Hi all,
Please review changes for bug JDK-8057777 "Cleanup of old
and unused VM interfaces"
https://bugs.openjdk.java.net/browse/JDK-8057777
This is basically a big cleanup of VM interfaces that are
not used anymore by the JDK but have been kept in our code
base for historical reasons (HotSpot Express for instance).
These changesets remove these interfaces from both the
JDK and the HotSpot side, and also perform some cleanup
on code that directly referenced the removed interfaces.
These changes do not modify the behavior of the Java
classes impacted by the cleanup.
VM interfaces removal has been approved by CCC and
a Release Note has been prepared that explicitly list
all the removed interfaces.
Testing: JPRT hotspot + core, vm.quick.testlist, jdk_core
http://cr.openjdk.java.net/~fparain/8057777/
Thank you,
Fred

[harold seigel]

Hi Fred,

It looks good! Thanks for making those changes.

Harold

Post by Frederic Parain
Harold,
Thank you for the review and your feedback.
I didn't try to remove all dead code from JVM entries
removal, just the most obvious dead code pieces.
Thank you for spotting that JavaCalls::call_default_constructor()
could be safely removed. I removed it in the new webrev.
Regarding mapfiles, I played with JVM_DefineClassWithSources
and DefineClassWithSourcesCond, and it seems that I didn't
restore the mapfile correctly. This is now fixed.
http://cr.openjdk.java.net/~fparain/8057777/hotspot_v02/
Thank you,
Fred

Post by harold seigel
Hi Fred,
It's great to see all this code go away.
Since JVM_AllocateNewObject() is getting removed, can
JavaCalls::call_default_constructor() also be removed?
Also, why were JVM_DesiredAssertionStatus and JVM_DoPrivileged moved in
make/solaris/makefiles/mapfile-vers ? They are no longer in
alphabetical order.
Thanks, Harold

Post by Frederic Parain
Thanks Coleen,
Fred

Post by Coleen Phillimore
Fred,
I reviewed this change. It looks great. Some of the functions removed
seem to be not only unused but dangerous. Some of these I have made
changes to that I didn't realize that the JVM didn't use these
functions. Thank you for doing this!
Coleen

Post by Frederic Parain
Hi all,
Please review changes for bug JDK-8057777 "Cleanup of old
and unused VM interfaces"
https://bugs.openjdk.java.net/browse/JDK-8057777
This is basically a big cleanup of VM interfaces that are
not used anymore by the JDK but have been kept in our code
base for historical reasons (HotSpot Express for instance).
These changesets remove these interfaces from both the
JDK and the HotSpot side, and also perform some cleanup
on code that directly referenced the removed interfaces.
These changes do not modify the behavior of the Java
classes impacted by the cleanup.
VM interfaces removal has been approved by CCC and
a Release Note has been prepared that explicitly list
all the removed interfaces.
Testing: JPRT hotspot + core, vm.quick.testlist, jdk_core
http://cr.openjdk.java.net/~fparain/8057777/
Thank you,
Fred

[Frederic Parain]

Thank you Harold,

Fred

Post by harold seigel
Hi Fred,
It looks good! Thanks for making those changes.
Harold

Post by Frederic Parain
Harold,
Thank you for the review and your feedback.
I didn't try to remove all dead code from JVM entries
removal, just the most obvious dead code pieces.
Thank you for spotting that JavaCalls::call_default_constructor()
could be safely removed. I removed it in the new webrev.
Regarding mapfiles, I played with JVM_DefineClassWithSources
and DefineClassWithSourcesCond, and it seems that I didn't
restore the mapfile correctly. This is now fixed.
http://cr.openjdk.java.net/~fparain/8057777/hotspot_v02/
Thank you,
Fred

Post by harold seigel
Hi Fred,
It's great to see all this code go away.
Since JVM_AllocateNewObject() is getting removed, can
JavaCalls::call_default_constructor() also be removed?
Also, why were JVM_DesiredAssertionStatus and JVM_DoPrivileged moved in
make/solaris/makefiles/mapfile-vers ? They are no longer in
alphabetical order.
Thanks, Harold

Post by Frederic Parain
Thanks Coleen,
Fred

Post by Coleen Phillimore
Fred,
I reviewed this change. It looks great. Some of the functions removed
seem to be not only unused but dangerous. Some of these I have made
changes to that I didn't realize that the JVM didn't use these
functions. Thank you for doing this!
Coleen

Post by Frederic Parain
Hi all,
Please review changes for bug JDK-8057777 "Cleanup of old
and unused VM interfaces"
https://bugs.openjdk.java.net/browse/JDK-8057777
This is basically a big cleanup of VM interfaces that are
not used anymore by the JDK but have been kept in our code
base for historical reasons (HotSpot Express for instance).
These changesets remove these interfaces from both the
JDK and the HotSpot side, and also perform some cleanup
on code that directly referenced the removed interfaces.
These changes do not modify the behavior of the Java
classes impacted by the cleanup.
VM interfaces removal has been approved by CCC and
a Release Note has been prepared that explicitly list
all the removed interfaces.
Testing: JPRT hotspot + core, vm.quick.testlist, jdk_core
http://cr.openjdk.java.net/~fparain/8057777/
Thank you,
Fred

[Alan Bateman]

Post by Frederic Parain
Hi all,
Please review changes for bug JDK-8057777 "Cleanup of old
and unused VM interfaces"
https://bugs.openjdk.java.net/browse/JDK-8057777
This is basically a big cleanup of VM interfaces that are
not used anymore by the JDK but have been kept in our code
base for historical reasons (HotSpot Express for instance).
These changesets remove these interfaces from both the
JDK and the HotSpot side, and also perform some cleanup
on code that directly referenced the removed interfaces.
These changes do not modify the behavior of the Java
classes impacted by the cleanup.
VM interfaces removal has been approved by CCC and
a Release Note has been prepared that explicitly list
all the removed interfaces.
Testing: JPRT hotspot + core, vm.quick.testlist, jdk_core
http://cr.openjdk.java.net/~fparain/8057777/

cc'ing core-libs-dev as part of this is clean-up in the library code too.

I think we should deprecate java.lang.Compiler and the Runtime.traceXXX
methods. They've been non-functional for a long time and having them in
the API is a bit mis-leading to anyone reading the javadoc. I realize
you are focused on the removing the old JVM_* functions so we can
follow-up on that via other issues of course.

Can ClassLoader#resolveClass0 can be removed completely? The null check
can be done in ClassLoader#resolveClass.

In the mapfile for libjava then the comment at line 281 says "ZipFile.c
needs this one". As getLastErrorString is now exported for use by libzip
then the comment should probably be updated.

Otherwise this clean-up looks good to me and the jdk_core group of tests
is the right group to exercise this area.

-Alan

[Frederic Parain]

Alan,

Thank you for your feedback, see my comments below.

Post by Alan Bateman

Post by Frederic Parain
Hi all,
Please review changes for bug JDK-8057777 "Cleanup of old
and unused VM interfaces"
https://bugs.openjdk.java.net/browse/JDK-8057777
This is basically a big cleanup of VM interfaces that are
not used anymore by the JDK but have been kept in our code
base for historical reasons (HotSpot Express for instance).
These changesets remove these interfaces from both the
JDK and the HotSpot side, and also perform some cleanup
on code that directly referenced the removed interfaces.
These changes do not modify the behavior of the Java
classes impacted by the cleanup.
VM interfaces removal has been approved by CCC and
a Release Note has been prepared that explicitly list
all the removed interfaces.
Testing: JPRT hotspot + core, vm.quick.testlist, jdk_core
http://cr.openjdk.java.net/~fparain/8057777/

cc'ing core-libs-dev as part of this is clean-up in the library code too.
I think we should deprecate java.lang.Compiler and the Runtime.traceXXX
methods. They've been non-functional for a long time and having them in
the API is a bit mis-leading to anyone reading the javadoc. I realize
you are focused on the removing the old JVM_* functions so we can
follow-up on that via other issues of course.

Deprecating class and methods would require another CCC approval, so
I'd prefer to post-pone this change to another changeset.

Post by Alan Bateman
Can ClassLoader#resolveClass0 can be removed completely? The null check
can be done in ClassLoader#resolveClass.

Done.

Post by Alan Bateman
In the mapfile for libjava then the comment at line 281 says "ZipFile.c
needs this one". As getLastErrorString is now exported for use by libzip
then the comment should probably be updated.

Done.

Post by Alan Bateman
Otherwise this clean-up looks good to me and the jdk_core group of tests
is the right group to exercise this area.

Thank you, the new jdk webrev is here:
http://cr.openjdk.java.net/~fparain/8057777/jdk_v02/

Regards,

Fred

[Xueming Shen]

Post by Alan Bateman

Post by Frederic Parain
Hi all,
Please review changes for bug JDK-8057777 "Cleanup of old
and unused VM interfaces"
https://bugs.openjdk.java.net/browse/JDK-8057777
This is basically a big cleanup of VM interfaces that are
not used anymore by the JDK but have been kept in our code
base for historical reasons (HotSpot Express for instance).
These changesets remove these interfaces from both the
JDK and the HotSpot side, and also perform some cleanup
on code that directly referenced the removed interfaces.
These changes do not modify the behavior of the Java
classes impacted by the cleanup.
VM interfaces removal has been approved by CCC and
a Release Note has been prepared that explicitly list
all the removed interfaces.
Testing: JPRT hotspot + core, vm.quick.testlist, jdk_core
http://cr.openjdk.java.net/~fparain/8057777/

cc'ing core-libs-dev as part of this is clean-up in the library code too.
I think we should deprecate java.lang.Compiler and the
Runtime.traceXXX methods. They've been non-functional for a long time
and having them in the API is a bit mis-leading to anyone reading the
javadoc. I realize you are focused on the removing the old JVM_*
functions so we can follow-up on that via other issues of course.
Can ClassLoader#resolveClass0 can be removed completely? The null
check can be done in ClassLoader#resolveClass.
In the mapfile for libjava then the comment at line 281 says
"ZipFile.c needs this one". As getLastErrorString is now exported for
use by libzip then the comment should probably be updated.
Otherwise this clean-up looks good to me and the jdk_core group of
tests is the right group to exercise this area.
-Alan

Hi,

ZipFile.c expects the JVM_Open() to handle the JVM_O_DELETE, with
explicit unlink on linux/solaris for example.
I would assume the open from the c library does not handle it and we
need to do it explicitly by ZipFile.c now?

-Sherman

[Frederic Parain]

Thank you very much for catching the JVM_O_DELETE issue.
I moved the O_DELETE support into the ZipFile.c file
(code was the same for all non-Windows platforms) and
cleaned up the original code in HotSpot.

The new webrevs:

http://cr.openjdk.java.net/~fparain/8057777/jdk_v03/
http://cr.openjdk.java.net/~fparain/8057777/hotspot_v03/

Regards,

Fred

Post by Xueming Shen

Post by Alan Bateman

Post by Frederic Parain
Hi all,
Please review changes for bug JDK-8057777 "Cleanup of old
and unused VM interfaces"
https://bugs.openjdk.java.net/browse/JDK-8057777
This is basically a big cleanup of VM interfaces that are
not used anymore by the JDK but have been kept in our code
base for historical reasons (HotSpot Express for instance).
These changesets remove these interfaces from both the
JDK and the HotSpot side, and also perform some cleanup
on code that directly referenced the removed interfaces.
These changes do not modify the behavior of the Java
classes impacted by the cleanup.
VM interfaces removal has been approved by CCC and
a Release Note has been prepared that explicitly list
all the removed interfaces.
Testing: JPRT hotspot + core, vm.quick.testlist, jdk_core
http://cr.openjdk.java.net/~fparain/8057777/

cc'ing core-libs-dev as part of this is clean-up in the library code too.
I think we should deprecate java.lang.Compiler and the
Runtime.traceXXX methods. They've been non-functional for a long time
and having them in the API is a bit mis-leading to anyone reading the
javadoc. I realize you are focused on the removing the old JVM_*
functions so we can follow-up on that via other issues of course.
Can ClassLoader#resolveClass0 can be removed completely? The null
check can be done in ClassLoader#resolveClass.
In the mapfile for libjava then the comment at line 281 says
"ZipFile.c needs this one". As getLastErrorString is now exported for
use by libzip then the comment should probably be updated.
Otherwise this clean-up looks good to me and the jdk_core group of
tests is the right group to exercise this area.
-Alan

Hi,
ZipFile.c expects the JVM_Open() to handle the JVM_O_DELETE, with
explicit unlink on linux/solaris for example.
I would assume the open from the c library does not handle it and we
need to do it explicitly by ZipFile.c now?
-Sherman

[David Holmes]

Hi Fred,

Looks okay to me too. But I wouldn't be surprised if there is some test
somewhere that checks for things when PrintJVMWarnings is set :)

David

Post by Frederic Parain
Thank you very much for catching the JVM_O_DELETE issue.
I moved the O_DELETE support into the ZipFile.c file
(code was the same for all non-Windows platforms) and
cleaned up the original code in HotSpot.
http://cr.openjdk.java.net/~fparain/8057777/jdk_v03/
http://cr.openjdk.java.net/~fparain/8057777/hotspot_v03/
Regards,
Fred

Post by Xueming Shen

Post by Alan Bateman

Post by Frederic Parain
Hi all,
Please review changes for bug JDK-8057777 "Cleanup of old
and unused VM interfaces"
https://bugs.openjdk.java.net/browse/JDK-8057777
This is basically a big cleanup of VM interfaces that are
not used anymore by the JDK but have been kept in our code
base for historical reasons (HotSpot Express for instance).
These changesets remove these interfaces from both the
JDK and the HotSpot side, and also perform some cleanup
on code that directly referenced the removed interfaces.
These changes do not modify the behavior of the Java
classes impacted by the cleanup.
VM interfaces removal has been approved by CCC and
a Release Note has been prepared that explicitly list
all the removed interfaces.
Testing: JPRT hotspot + core, vm.quick.testlist, jdk_core
http://cr.openjdk.java.net/~fparain/8057777/

cc'ing core-libs-dev as part of this is clean-up in the library code too.
I think we should deprecate java.lang.Compiler and the
Runtime.traceXXX methods. They've been non-functional for a long time
and having them in the API is a bit mis-leading to anyone reading the
javadoc. I realize you are focused on the removing the old JVM_*
functions so we can follow-up on that via other issues of course.
Can ClassLoader#resolveClass0 can be removed completely? The null
check can be done in ClassLoader#resolveClass.
In the mapfile for libjava then the comment at line 281 says
"ZipFile.c needs this one". As getLastErrorString is now exported for
use by libzip then the comment should probably be updated.
Otherwise this clean-up looks good to me and the jdk_core group of
tests is the right group to exercise this area.
-Alan

Hi,
ZipFile.c expects the JVM_Open() to handle the JVM_O_DELETE, with
explicit unlink on linux/solaris for example.
I would assume the open from the c library does not handle it and we
need to do it explicitly by ZipFile.c now?
-Sherman

[Alan Bateman]

Post by Frederic Parain
Thank you very much for catching the JVM_O_DELETE issue.
I moved the O_DELETE support into the ZipFile.c file
(code was the same for all non-Windows platforms) and
cleaned up the original code in HotSpot.
http://cr.openjdk.java.net/~fparain/8057777/jdk_v03/
http://cr.openjdk.java.net/~fparain/8057777/hotspot_v03/

Good spot by Sherman on the OPEN_DELETE issue, it makes me wonder what
the test coverage is on this.

The updated webrev addresses the points that I brought up and looks good
(nice to see many of these functions going away).

-Alan

[Frederic Parain]

Thank you Coleen, Harold, Alan and Sherman
for your reviews.

Fred

Post by Alan Bateman

Post by Frederic Parain
Thank you very much for catching the JVM_O_DELETE issue.
I moved the O_DELETE support into the ZipFile.c file
(code was the same for all non-Windows platforms) and
cleaned up the original code in HotSpot.
http://cr.openjdk.java.net/~fparain/8057777/jdk_v03/
http://cr.openjdk.java.net/~fparain/8057777/hotspot_v03/

Good spot by Sherman on the OPEN_DELETE issue, it makes me wonder what
the test coverage is on this.
The updated webrev addresses the points that I brought up and looks good
(nice to see many of these functions going away).
-Alan

[Coleen Phillimore]

Fred, The additional changes to these files looks fine to me also.

Thanks!
Coleen

Post by Frederic Parain
Thank you very much for catching the JVM_O_DELETE issue.
I moved the O_DELETE support into the ZipFile.c file
(code was the same for all non-Windows platforms) and
cleaned up the original code in HotSpot.
http://cr.openjdk.java.net/~fparain/8057777/jdk_v03/
http://cr.openjdk.java.net/~fparain/8057777/hotspot_v03/
Regards,
Fred

Post by Xueming Shen

Post by Alan Bateman

Post by Frederic Parain
Hi all,
Please review changes for bug JDK-8057777 "Cleanup of old
and unused VM interfaces"
https://bugs.openjdk.java.net/browse/JDK-8057777
This is basically a big cleanup of VM interfaces that are
not used anymore by the JDK but have been kept in our code
base for historical reasons (HotSpot Express for instance).
These changesets remove these interfaces from both the
JDK and the HotSpot side, and also perform some cleanup
on code that directly referenced the removed interfaces.
These changes do not modify the behavior of the Java
classes impacted by the cleanup.
VM interfaces removal has been approved by CCC and
a Release Note has been prepared that explicitly list
all the removed interfaces.
Testing: JPRT hotspot + core, vm.quick.testlist, jdk_core
http://cr.openjdk.java.net/~fparain/8057777/

cc'ing core-libs-dev as part of this is clean-up in the library code too.
I think we should deprecate java.lang.Compiler and the
Runtime.traceXXX methods. They've been non-functional for a long time
and having them in the API is a bit mis-leading to anyone reading the
javadoc. I realize you are focused on the removing the old JVM_*
functions so we can follow-up on that via other issues of course.
Can ClassLoader#resolveClass0 can be removed completely? The null
check can be done in ClassLoader#resolveClass.
In the mapfile for libjava then the comment at line 281 says
"ZipFile.c needs this one". As getLastErrorString is now exported for
use by libzip then the comment should probably be updated.
Otherwise this clean-up looks good to me and the jdk_core group of
tests is the right group to exercise this area.
-Alan

Hi,
ZipFile.c expects the JVM_Open() to handle the JVM_O_DELETE, with
explicit unlink on linux/solaris for example.
I would assume the open from the c library does not handle it and we
need to do it explicitly by ZipFile.c now?
-Sherman