Fredrik,
On 09/30/2013 01:45 PM, Fredrik Arvidsson wrote:
Hi
Please help me to review the changes below:
Jira case: https://bugs.openjdk.java.net/browse/JDK-8024423
Webrev: http://cr.openjdk.java.net/~allwin/farvidss/8024423/webrev.00/
<http://cr.openjdk.java.net/%7Eallwin/farvidss/8024423/webrev.00/>
In this review I still have an outstanding unanswered question about the
need to synchronize the access to the:
ClassLoaderDataGraph::classes_do(&JvmtiGetLoadedClassesClosure::increment);
and
ClassLoaderDataGraph::classes_do(&JvmtiGetLoadedClassesClosure::add);
It seems to me that the synchronization is only needed to make sure that
we get a correct count for the _list array. New Klass*'s are pushed on
the top of the _klasses list so that should not interfere with walking
the Klass links.
One approach would be to do only one walk over the ClassLoaderDataGraph
and use a growing datastructure instead of allocating one big linear
array. I don't see any obvious requirement for a random-access
datastructure so you could probably just put a Stack<Handle> there and
push the mirrors on the stack, get the stack size and put them on
result_list in the correct order.
You can also make JvmtiGetLoadedClassesClosure a KlassClosure and get
rid of the thread-local get_this/set_this since ClassLoaderDataGraph
provides an interface for a KlassClosure as well as a callback function
pointer.
/Mikael
calls in
http://cr.openjdk.java.net/~allwin/farvidss/8024423/webrev.00/src/share/vm/prims/jvmtiGetLoadedClasses.cpp.udiff.html
<http://cr.openjdk.java.net/%7Eallwin/farvidss/8024423/webrev.00/src/share/vm/prims/jvmtiGetLoadedClasses.cpp.udiff.html>
Please give me advise on this.
There will be a review soon regarding re-enabling the tests that failed because
of the above bug, see:
https://bugs.openjdk.java.net/browse/JDK-8025576
Cheers
/Fredrik
