8062116: JVMTI GetClassMethods is Slow
Summary: Allocate enough space for all jmethodids; make adding a jmethodid O(1)
Reviewed-by: coleenp, rasbold, sspitsyn
--- a/hotspot/src/share/vm/oops/instanceKlass.cpp Thu Nov 06 01:31:31 2014 +0000
+++ b/hotspot/src/share/vm/oops/instanceKlass.cpp Wed Nov 05 16:47:37 2014 -0800
@@ -1730,6 +1730,25 @@
return id;
}
+// Figure out how many jmethodIDs haven't been allocated, and make
+// sure space for them is pre-allocated. This makes getting all
+// method ids much, much faster with classes with more than 8
+// methods, and has a *substantial* effect on performance with jvmti
+// code that loads all jmethodIDs for all classes.
+void InstanceKlass::ensure_space_for_methodids(int start_offset) {
+ int new_jmeths = 0;
+ int length = methods()->length();
+ for (int index = start_offset; index < length; index++) {
+ Method* m = methods()->at(index);
+ jmethodID id = m->find_jmethod_id_or_null();
+ if (id == NULL) {
+ new_jmeths++;
+ }
+ }
+ if (new_jmeths != 0) {
+ Method::ensure_jmethod_ids(class_loader_data(), new_jmeths);
+ }
+}
// Common code to fetch the jmethodID from the cache or update the
// cache with the new jmethodID. This function should never do anything
--- a/hotspot/src/share/vm/oops/instanceKlass.hpp Thu Nov 06 01:31:31 2014 +0000
+++ b/hotspot/src/share/vm/oops/instanceKlass.hpp Wed Nov 05 16:47:37 2014 -0800
@@ -698,6 +698,7 @@
jmethodID** to_dealloc_jmeths_p);
static void get_jmethod_id_length_value(jmethodID* cache, size_t idnum,
size_t *length_p, jmethodID* id_p);
+ void ensure_space_for_methodids(int start_offset = 0);
jmethodID jmethod_id_or_null(Method* method);
// annotations support
--- a/hotspot/src/share/vm/oops/method.cpp Thu Nov 06 01:31:31 2014 +0000
+++ b/hotspot/src/share/vm/oops/method.cpp Wed Nov 05 16:47:37 2014 -0800
@@ -1727,59 +1727,98 @@
// jmethodID handling
// This is a block allocating object, sort of like JNIHandleBlock, only a
-// lot simpler. There aren't many of these, they aren't long, they are rarely
-// deleted and so we can do some suboptimal things.
+// lot simpler.
// It's allocated on the CHeap because once we allocate a jmethodID, we can
// never get rid of it.
-// It would be nice to be able to parameterize the number of methods for
-// the null_class_loader but then we'd have to turn this and ClassLoaderData
-// into templates.
+
+static const int min_block_size = 8;
+
+class JNIMethodBlockNode : public CHeapObj<mtClass> {
+ friend class JNIMethodBlock;
+ Method** _methods;
+ int _number_of_methods;
+ int _top;
+ JNIMethodBlockNode* _next;
+
+ public:
+
+ JNIMethodBlockNode(int num_methods = min_block_size);
-// I feel like this brain dead class should exist somewhere in the STL
+ ~JNIMethodBlockNode() { FREE_C_HEAP_ARRAY(Method*, _methods, mtInternal); }
+
+ void ensure_methods(int num_addl_methods) {
+ if (_top < _number_of_methods) {
+ num_addl_methods -= _number_of_methods - _top;
+ if (num_addl_methods <= 0) {
+ return;
+ }
+ }
+ if (_next == NULL) {
+ _next = new JNIMethodBlockNode(MAX2(num_addl_methods, min_block_size));
+ } else {
+ _next->ensure_methods(num_addl_methods);
+ }
+ }
+};
class JNIMethodBlock : public CHeapObj<mtClass> {
- enum { number_of_methods = 8 };
-
- Method* _methods[number_of_methods];
- int _top;
- JNIMethodBlock* _next;
+ JNIMethodBlockNode _head;
+ JNIMethodBlockNode *_last_free;
public:
static Method* const _free_method;
- JNIMethodBlock() : _next(NULL), _top(0) {
- for (int i = 0; i< number_of_methods; i++) _methods[i] = _free_method;
+ JNIMethodBlock(int initial_capacity = min_block_size)
+ : _head(initial_capacity), _last_free(&_head) {}
+
+ void ensure_methods(int num_addl_methods) {
+ _last_free->ensure_methods(num_addl_methods);
}
Method** add_method(Method* m) {
- if (_top < number_of_methods) {
- // top points to the next free entry.
- int i = _top;
- _methods[i] = m;
- _top++;
- return &_methods[i];
- } else if (_top == number_of_methods) {
- // if the next free entry ran off the block see if there's a free entry
- for (int i = 0; i< number_of_methods; i++) {
- if (_methods[i] == _free_method) {
- _methods[i] = m;
- return &_methods[i];
+ for (JNIMethodBlockNode* b = _last_free; b != NULL; b = b->_next) {
+ if (b->_top < b->_number_of_methods) {
+ // top points to the next free entry.
+ int i = b->_top;
+ b->_methods[i] = m;
+ b->_top++;
+ _last_free = b;
+ return &(b->_methods[i]);
+ } else if (b->_top == b->_number_of_methods) {
+ // if the next free entry ran off the block see if there's a free entry
+ for (int i = 0; i < b->_number_of_methods; i++) {
+ if (b->_methods[i] == _free_method) {
+ b->_methods[i] = m;
+ _last_free = b;
+ return &(b->_methods[i]);
+ }
}
+ // Only check each block once for frees. They're very unlikely.
+ // Increment top past the end of the block.
+ b->_top++;
}
- // Only check each block once for frees. They're very unlikely.
- // Increment top past the end of the block.
- _top++;
+ // need to allocate a next block.
+ if (b->_next == NULL) {
+ b->_next = _last_free = new JNIMethodBlockNode();
+ }
}
- // need to allocate a next block.
- if (_next == NULL) {
- _next = new JNIMethodBlock();
- }
- return _next->add_method(m);
+ guarantee(false, "Should always allocate a free block");
+ return NULL;
}
bool contains(Method** m) {
- for (JNIMethodBlock* b = this; b != NULL; b = b->_next) {
- for (int i = 0; i< number_of_methods; i++) {
- if (&(b->_methods[i]) == m) {
+ if (m == NULL) return false;
+ for (JNIMethodBlockNode* b = &_head; b != NULL; b = b->_next) {
+ if (b->_methods <= m && m < b->_methods + b->_number_of_methods) {
+ // This is a bit of extra checking, for two reasons. One is
+ // that contains() deals with pointers that are passed in by
+ // JNI code, so making sure that the pointer is aligned
+ // correctly is valuable. The other is that <= and > are
+ // technically not defined on pointers, so the if guard can
+ // pass spuriously; no modern compiler is likely to make that
+ // a problem, though (and if one did, the guard could also
+ // fail spuriously, which would be bad).
+ ptrdiff_t idx = m - b->_methods;
+ if (b->_methods + idx == m) {
return true;
}
}
@@ -1798,9 +1837,9 @@
// During class unloading the methods are cleared, which is different
// than freed.
void clear_all_methods() {
- for (JNIMethodBlock* b = this; b != NULL; b = b->_next) {
- for (int i = 0; i< number_of_methods; i++) {
- _methods[i] = NULL;
+ for (JNIMethodBlockNode* b = &_head; b != NULL; b = b->_next) {
+ for (int i = 0; i< b->_number_of_methods; i++) {
+ b->_methods[i] = NULL;
}
}
}
@@ -1808,9 +1847,9 @@
int count_methods() {
// count all allocated methods
int count = 0;
- for (JNIMethodBlock* b = this; b != NULL; b = b->_next) {
- for (int i = 0; i< number_of_methods; i++) {
- if (_methods[i] != _free_method) count++;
+ for (JNIMethodBlockNode* b = &_head; b != NULL; b = b->_next) {
+ for (int i = 0; i< b->_number_of_methods; i++) {
+ if (b->_methods[i] != _free_method) count++;
}
}
return count;
@@ -1821,6 +1860,36 @@
// Something that can't be mistaken for an address or a markOop
Method* const JNIMethodBlock::_free_method = (Method*)55;
+JNIMethodBlockNode::JNIMethodBlockNode(int num_methods) : _next(NULL), _top(0) {
+ _number_of_methods = MAX2(num_methods, min_block_size);
+ _methods = NEW_C_HEAP_ARRAY(Method*, _number_of_methods, mtInternal);
+ for (int i = 0; i < _number_of_methods; i++) {
+ _methods[i] = JNIMethodBlock::_free_method;
+ }
+}
+
+void Method::ensure_jmethod_ids(ClassLoaderData* loader_data, int capacity) {
+ ClassLoaderData* cld = loader_data;
+ if (!SafepointSynchronize::is_at_safepoint()) {
+ // Have to add jmethod_ids() to class loader data thread-safely.
+ // Also have to add the method to the list safely, which the cld lock
+ // protects as well.
+ MutexLockerEx ml(cld->metaspace_lock(), Mutex::_no_safepoint_check_flag);
+ if (cld->jmethod_ids() == NULL) {
+ cld->set_jmethod_ids(new JNIMethodBlock(capacity));
+ } else {
+ cld->jmethod_ids()->ensure_methods(capacity);
+ }
+ } else {
+ // At safepoint, we are single threaded and can set this.
+ if (cld->jmethod_ids() == NULL) {
+ cld->set_jmethod_ids(new JNIMethodBlock(capacity));
+ } else {
+ cld->jmethod_ids()->ensure_methods(capacity);
+ }
+ }
+}
+
// Add a method id to the jmethod_ids
jmethodID Method::make_jmethod_id(ClassLoaderData* loader_data, Method* m) {
ClassLoaderData* cld = loader_data;
--- a/hotspot/src/share/vm/oops/method.hpp Thu Nov 06 01:31:31 2014 +0000
+++ b/hotspot/src/share/vm/oops/method.hpp Wed Nov 05 16:47:37 2014 -0800
@@ -729,6 +729,11 @@
static jmethodID make_jmethod_id(ClassLoaderData* loader_data, Method* mh);
static void destroy_jmethod_id(ClassLoaderData* loader_data, jmethodID mid);
+ // Ensure there is enough capacity in the internal tracking data
+ // structures to hold the number of jmethodIDs you plan to generate.
+ // This saves substantial time doing allocations.
+ static void ensure_jmethod_ids(ClassLoaderData* loader_data, int capacity);
+
// Use resolve_jmethod_id() in situations where the caller is expected
// to provide a valid jmethodID; the only sanity checks are in asserts;
// result guaranteed not to be NULL.
--- a/hotspot/src/share/vm/prims/jvmtiEnv.cpp Thu Nov 06 01:31:31 2014 +0000
+++ b/hotspot/src/share/vm/prims/jvmtiEnv.cpp Wed Nov 05 16:47:37 2014 -0800
@@ -2263,6 +2263,8 @@
int result_length = instanceK_h->methods()->length();
jmethodID* result_list = (jmethodID*)jvmtiMalloc(result_length * sizeof(jmethodID));
int index;
+ bool jmethodids_found = true;
+
if (JvmtiExport::can_maintain_original_method_order()) {
// Use the original method ordering indices stored in the class, so we can emit
// jmethodIDs in the order they appeared in the class file
@@ -2270,14 +2272,40 @@
Method* m = instanceK_h->methods()->at(index);
int original_index = instanceK_h->method_ordering()->at(index);
assert(original_index >= 0 && original_index < result_length, "invalid original method index");
- jmethodID id = m->jmethod_id();
+ jmethodID id;
+ if (jmethodids_found) {
+ id = m->find_jmethod_id_or_null();
+ if (id == NULL) {
+ // If we find an uninitialized value, make sure there is
+ // enough space for all the uninitialized values we might
+ // find.
+ instanceK_h->ensure_space_for_methodids(index);
+ jmethodids_found = false;
+ id = m->jmethod_id();
+ }
+ } else {
+ id = m->jmethod_id();
+ }
result_list[original_index] = id;
}
} else {
// otherwise just copy in any order
for (index = 0; index < result_length; index++) {
Method* m = instanceK_h->methods()->at(index);
- jmethodID id = m->jmethod_id();
+ jmethodID id;
+ if (jmethodids_found) {
+ id = m->find_jmethod_id_or_null();
+ if (id == NULL) {
+ // If we find an uninitialized value, make sure there is
+ // enough space for all the uninitialized values we might
+ // find.
+ instanceK_h->ensure_space_for_methodids(index);
+ jmethodids_found = false;
+ id = m->jmethod_id();
+ }
+ } else {
+ id = m->jmethod_id();
+ }
result_list[index] = id;
}
}