On 8/24/18 3:31 PM, Lois Foltan wrote:
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.
Hm, you're right T_BOOLEAN starts at 4. The closure will still crash on
zero for _typeArrayKlassObj[T_ARRAY] and [T_OBJECT]. Actually
_typeArrayKlassObj only goes to T_LONG, so I could change the loop to be
from T_BOOLEAN to T_LONG inclusively (and rerunning tests).
http://cr.openjdk.java.net/~coleenp/8209821.02/webrev/src/hotspot/share/memory/universe.cpp.udiff.html
I opened this RFE to clean these up some more.
https://bugs.openjdk.java.net/browse/JDK-8209958
Thanks,
Coleen
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