This is the mail archive of the
java-patches@gcc.gnu.org
mailing list for the Java project.
Re: [RFA] JVMTI/JDWP GetAllLoadedClasses
Keith Seitz wrote:
A couple of comments...
Kyle Galloway wrote:
Index: java/lang/natClassLoader.cc
===================================================================
--- java/lang/natClassLoader.cc (revision 124192)
+++ java/lang/natClassLoader.cc (working copy)
@@ -380,6 +392,19 @@
void
_Jv_CopyClassesToSystemLoader (gnu::gcj::runtime::SystemClassLoader
*loader)
{
+ using namespace ::java::lang;
+
+ // Add any classes loaded before this point to the loaded classes
list for
+ // JVMTI. As this is done, the memory used by the list is freed.
+ for (struct _Jv_TempClassListElement *ptr = jvmti_class_list_head;
+ ptr;
+ ptr = ptr->next)
+ {
+ VMClassLoader::loaded_classes->add (new ref::WeakReference
(ptr->klass));
+ _Jv_Free (jvmti_class_list_head);
+ jvmti_class_list_head = ptr;
+ }
+
for (jclass klass = system_class_list;
klass;
klass = klass->next_or_version)
In order to minimize the impact of this on startup times for the
non-debug case, please wrap all this so that it does nothing when
JVMTI is not active. You can check JVMTI::enabled to do this.
I knew there was a reason I didn't do this originally. JVMTI::enabled
doesn't happen until pretty far into the startup sequence, which means
many classes wouldn't be logged. It is, however, possible to check if
we are going to be using JVMTI as soon as _Jv_CreateJavaVM is called, so
I have moved where JVMTI::enabled is set to here.
@@ -655,6 +680,33 @@
_Jv_PushClass (jclass k)
{
JvSynchronize sync (&java::lang::Class::class$);
+ + using namespace ::java::lang;
+
+ // Build a list of loaded classes for JVMTI.
+ if (system_class_list == SYSTEM_LOADER_INITIALIZED)
+ VMClassLoader::loaded_classes->add (new ref::WeakReference (k));
+ else
+ {
+ if (!jvmti_class_list_head)
+ {
+ jvmti_class_list_head + =
reinterpret_cast<struct _Jv_TempClassListElement *> +
(_Jv_MallocUnchecked (sizeof (struct _Jv_TempClassListElement)));
+ jvmti_class_list_head->next = NULL;
+ }
+ else
+ {
+ struct _Jv_TempClassListElement *tmp + =
reinterpret_cast<struct _Jv_TempClassListElement *> +
(_Jv_MallocUnchecked (sizeof (struct _Jv_TempClassListElement)));
+ tmp->next = jvmti_class_list_head;
+ jvmti_class_list_head = tmp;
+ }
+
+ jvmti_class_list_head->klass = k;
+ }
+ jclass tmp = stack_head;
stack_head = k;
k->chain = tmp;
Likewise with JVMTI::enabled.
Index: java/lang/VMClassLoader.h
===================================================================
--- java/lang/VMClassLoader.h (revision 124192)
+++ java/lang/VMClassLoader.h (working copy)
This header is regenerated by the build, right? I don't think you need
to post a patch for it in that case. We all know it is regenerated.
(And your ChangeLog told us it was regenerated. [Ditto the class
files.] :-)
Index: java/lang/VMClassLoader.java
===================================================================
--- java/lang/VMClassLoader.java (revision 124192)
+++ java/lang/VMClassLoader.java (working copy)
@@ -345,4 +350,24 @@
return default_sys;
}
+
+ /**
+ * First check if any of the classes in the list have been
collected, and if
+ * they have, remove them from the list. It then returns the
modified list.
+ *
+ * @return The <code>HashSet</code> of loaded classes.
+ */
+ public static HashSet getAllLoadedClasses()
+ {
+ Iterator it = loaded_classes.iterator();
+
+ while (it.hasNext())
+ {
+ WeakReference ref = (WeakReference) it.next();
+ if (ref.get() == null)
+ it.remove();
+ }
+
+ return loaded_classes;
+ }
}
I have one concern about this... If another class is loaded by another
thread at this time, your iterating functions
(_Jv_JVMTI_GetLoadedClasses and getAllLoadedClasses) are going to
throw a ConcurrentModificationException.
While this seems pretty unlikely today, sometime in the future it is
conceivable that someone might run multiple JVMTI agents or have
multiple threads running through JDWP (all suspended/running
asynchronously). I would throw in a lock just to be sure. [I'm sure
there are other parts around JDWP where this is an issue, but we have
to start somewhere.]
This seems reasonable. I've gone and added locks on both the loading
side and the two iterators.
New patch is attached.
Questions/comments/concerns?
Thanks,
Kyle
Index: jvmti.cc
===================================================================
--- jvmti.cc (revision 124295)
+++ jvmti.cc (working copy)
@@ -37,6 +37,8 @@
#include <java/lang/reflect/Modifier.h>
#include <java/util/Collection.h>
#include <java/util/HashMap.h>
+#include <java/util/HashSet.h>
+#include <java/util/Iterator.h>
#include <java/util/concurrent/locks/Lock.h>
#include <java/util/concurrent/locks/ReentrantReadWriteLock.h>
#include <java/net/URL.h>
@@ -1123,6 +1122,36 @@
}
static jvmtiError JNICALL
+_Jv_JVMTI_GetLoadedClasses (jvmtiEnv *env, jint *num_classes, jclass **classes)
+{
+ REQUIRE_PHASE (env, JVMTI_PHASE_LIVE);
+ NULL_CHECK (num_classes);
+ NULL_CHECK (classes);
+
+ using namespace ::java::lang;
+
+ java::util::HashSet *loaded_classes = VMClassLoader::getAllLoadedClasses ();
+
+ *num_classes = loaded_classes->size ();
+
+ jvmtiError jerr = env->Allocate (*num_classes * sizeof (jclass),
+ reinterpret_cast<unsigned char **> (classes));
+ if (jerr != JVMTI_ERROR_NONE)
+ return jerr;
+
+ JvSynchronize list (VMClassLoader::loaded_classes);
+ ::java::util::Iterator *it = loaded_classes->iterator ();
+
+ for (int i = 0; it->hasNext () && i < *num_classes; i++)
+ {
+ (*classes)[i] = reinterpret_cast<Class *>
+ (((ref::WeakReference *)it->next ())->get ());
+ }
+
+ return JVMTI_ERROR_NONE;
+}
+
+static jvmtiError JNICALL
_Jv_JVMTI_GetMaxLocals (jvmtiEnv *env, jmethodID method, jint *max_locals)
{
REQUIRE_PHASE (env, JVMTI_PHASE_START | JVMTI_PHASE_LIVE);
@@ -2126,7 +2155,7 @@
UNIMPLEMENTED, // GetBytecodes
_Jv_JVMTI_IsMethodNative, // IsMethodNative
_Jv_JVMTI_IsMethodSynthetic, // IsMethodSynthetic
- UNIMPLEMENTED, // GetLoadedClasses
+ _Jv_JVMTI_GetLoadedClasses, // GetLoadedClasses
_Jv_JVMTI_GetClassLoaderClasses, // GetClassLoaderClasses
UNIMPLEMENTED, // PopFrame
RESERVED, // reserved81
@@ -2228,10 +2257,6 @@
}
_envListLock->writeLock ()->unlock ();
- /* Mark JVMTI active. This is used to force the interpreter
- to use either debugging or non-debugging code. Once JVMTI
- has been enabled, the non-debug interpreter cannot be used. */
- JVMTI::enabled = true;
return env;
}
Index: prims.cc
===================================================================
--- prims.cc (revision 124295)
+++ prims.cc (working copy)
@@ -1600,6 +1600,13 @@
if (result < 0)
return -1;
+ /* Mark JVMTI active. This is used to force the interpreter
+ to use either debugging or non-debugging code. Once JVMTI
+ has been enabled, the non-debug interpreter cannot be used.
+ This is done here in order to capture all classes as they are loaded. */
+ if (remoteDebug || jvmti_agentonload)
+ JVMTI::enabled = true;
+
PROCESS_GCJ_PROPERTIES;
/* Threads must be initialized before the GC, so that it inherits the
Index: gnu/classpath/jdwp/natVMVirtualMachine.cc
===================================================================
--- gnu/classpath/jdwp/natVMVirtualMachine.cc (revision 124295)
+++ gnu/classpath/jdwp/natVMVirtualMachine.cc (working copy)
@@ -451,7 +451,24 @@
gnu::classpath::jdwp::VMVirtualMachine::getAllLoadedClasses (void)
{
using namespace ::java::util;
- return (Collection *) new ArrayList ();
+
+ jclass *classes;
+ jint num_classes;
+
+ jvmtiError err = _jdwp_jvmtiEnv->GetLoadedClasses (&num_classes, &classes);
+ if (err != JVMTI_ERROR_NONE)
+ throw_jvmti_error (err);
+
+ ArrayList *cls_list = new ArrayList (num_classes);
+
+ for (int i = 0; i < num_classes; i++)
+ {
+ cls_list->add (classes[i]);
+ }
+
+ _jdwp_jvmtiEnv->Deallocate (reinterpret_cast<unsigned char *> (classes));
+
+ return (Collection *) cls_list;
}
jint
Index: java/lang/natClassLoader.cc
===================================================================
--- java/lang/natClassLoader.cc (revision 124295)
+++ java/lang/natClassLoader.cc (working copy)
@@ -42,9 +42,14 @@
#include <java/io/Serializable.h>
#include <java/lang/Cloneable.h>
#include <java/util/HashMap.h>
+#include <java/lang/ref/WeakReference.h>
+#include <java/util/HashSet.h>
#include <gnu/gcj/runtime/BootClassLoader.h>
#include <gnu/gcj/runtime/SystemClassLoader.h>
+#include <jvmti.h>
+#include <jvmti-int.h>
+
// Size of local hash table.
#define HASH_LEN 1013
@@ -62,6 +67,16 @@
static jclass loaded_classes[HASH_LEN];
+// Element in the temporary loaded classes linked list for JVMTI.
+struct _Jv_TempClassListElement
+{
+ jclass klass;
+ _Jv_TempClassListElement *next;
+};
+
+// The head of the temporary loaded classes linked list for JVMTI.
+static struct _Jv_TempClassListElement *jvmti_class_list_head = NULL;
+
// This is the root of a linked list of classes
static jclass stack_head;
@@ -380,6 +395,24 @@
void
_Jv_CopyClassesToSystemLoader (gnu::gcj::runtime::SystemClassLoader *loader)
{
+ using namespace ::java::lang;
+
+ // Add any classes loaded before this point to the loaded classes list for
+ // JVMTI. As this is done, the memory used by the list is freed. First,
+ // check if JVMTI is enabled.
+ if (JVMTI::enabled)
+ {
+ for (struct _Jv_TempClassListElement *ptr = jvmti_class_list_head;
+ ptr;
+ ptr = ptr->next)
+ {
+ VMClassLoader::loaded_classes->add (
+ new ref::WeakReference (ptr->klass));
+ _Jv_Free (jvmti_class_list_head);
+ jvmti_class_list_head = ptr;
+ }
+ }
+
for (jclass klass = system_class_list;
klass;
klass = klass->next_or_version)
@@ -655,6 +688,41 @@
_Jv_PushClass (jclass k)
{
JvSynchronize sync (&java::lang::Class::class$);
+
+ using namespace ::java::lang;
+
+ // If JVMTI is enabled, build a list of loaded classes for it.
+ if (JVMTI::enabled)
+ {
+ if (system_class_list == SYSTEM_LOADER_INITIALIZED)
+ {
+ JvSynchronize list (VMClassLoader::loaded_classes);
+ VMClassLoader::loaded_classes->add (new ref::WeakReference (k));
+ }
+ else
+ {
+ if (!jvmti_class_list_head)
+ {
+ jvmti_class_list_head
+ = reinterpret_cast<struct _Jv_TempClassListElement *>
+ (_Jv_MallocUnchecked (
+ sizeof (struct _Jv_TempClassListElement)));
+ jvmti_class_list_head->next = NULL;
+ }
+ else
+ {
+ struct _Jv_TempClassListElement *tmp
+ = reinterpret_cast<struct _Jv_TempClassListElement *>
+ (_Jv_MallocUnchecked (
+ sizeof (struct _Jv_TempClassListElement)));
+ tmp->next = jvmti_class_list_head;
+ jvmti_class_list_head = tmp;
+ }
+
+ jvmti_class_list_head->klass = k;
+ }
+ }
+
jclass tmp = stack_head;
stack_head = k;
k->chain = tmp;
Index: java/lang/VMClassLoader.java
===================================================================
--- java/lang/VMClassLoader.java (revision 124295)
+++ java/lang/VMClassLoader.java (working copy)
@@ -40,6 +40,7 @@
import gnu.java.util.EmptyEnumeration;
import java.lang.reflect.Constructor;
+import java.lang.ref.WeakReference;
import java.io.File;
import java.io.IOException;
import java.net.URL;
@@ -52,6 +53,7 @@
import java.util.Enumeration;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.Iterator;
import java.util.Map;
import java.util.StringTokenizer;
import gnu.gcj.runtime.BootClassLoader;
@@ -99,6 +101,9 @@
private static final int LIB_CACHE = 1;
private static final int LIB_NEVER = 2;
+ // List to hold all loaded classes for JVMTI.
+ public static HashSet loaded_classes = new HashSet ();
+
/**
* Helper to define a class using a string of bytes. This assumes that
* the security checks have already been performed, if necessary.
@@ -345,4 +350,27 @@
return default_sys;
}
+
+ /**
+ * First check if any of the classes in the list have been collected, and if
+ * they have, remove them from the list. It then returns the modified list.
+ *
+ * @return The <code>HashSet</code> of loaded classes.
+ */
+ public static HashSet getAllLoadedClasses()
+ {
+ synchronized (loaded_classes)
+ {
+ Iterator it = loaded_classes.iterator();
+
+ while (it.hasNext())
+ {
+ WeakReference ref = (WeakReference) it.next();
+ if (ref.get() == null)
+ it.remove();
+ }
+ }
+
+ return loaded_classes;
+ }
}