On 8/24/2018 2:59 PM, coleen.phillim...@oracle.com wrote:
On 8/24/18 11:44 AM, Lois Foltan wrote:
On 8/23/2018 8:37 AM, coleen.phillim...@oracle.com wrote:
Summary: And also added function with KlassClosure to remove the hacks.
There are about 10 vmTestbase/nsk/jvmti tests that test various
parts of this change. Also ran mach5 tier1-7.
open webrev at http://cr.openjdk.java.net/~coleenp/8209821.01/webrev
bug link https://bugs.openjdk.java.net/browse/JDK-8209821
Thanks,
Coleen
Hi Coleen,
I think this is a good clean up. Couple of comments.
- memory/universe.cpp
You could make basic_type_classes_do() be a for loop
for (int i = 0; i < T_VOID+1; i++) {
closure->do_klass(typeArrayKlassObjs[i]());
}
Interesting observation. This is equivalent except T_OBJECT and
T_ARRAY elements aren't initiatialized. I believe that the do_klass
in the closure I am passing will choke on NULL. I've never understood
why these statics needed to be duplicated like this, and have tried to
clean this up before. Maybe an RFE to do so would be better.
Hi Coleen,
Then just have the for loop be instead for (int i = T_BOOLEAN; i <
T_LONG+1; i++). I'm okay with what you decide. However, to me the code
in Universe::metaspace_pointers_do() at line 230-232 looks wrong:
for (int i = 0; i < T_VOID+1; i++) {
it->push(&_typeArrayKlassObjs[i]);
}
The VM does not appear to store anything in _typeArrayKlassObjs for
elements 0-3, so hopefully those elements are NULL. It should at least
start that for loop off at T_BOOLEAN.
Thanks,
Lois
- prims/jvmtiGetLoadedClasses.cpp
In JvmtiGetLoadedClasses::getClassLoaderClasses() you could pull the
call to basic_type_classes_do() from both sections of the if
statement to line #139
Thanks, I'll fix it.
Thanks for the code review.
Coleen
Thanks,
Lois
