8034839: jvm hangs with gc/gctests/LoadUnloadGC test

Provide fast lookup of checked dependencies via hashmap

Reviewed-by: kvn, roland

hotspot/src/share/vm/code/codeCache.cpp

@@ -595,11 +595,8 @@ void CodeCache::clear_inline_caches() {
   }
 }
 
-#ifndef PRODUCT
 // Keeps track of time spent for checking dependencies
-static elapsedTimer dependentCheckTime;
-#endif
-
+NOT_PRODUCT(static elapsedTimer dependentCheckTime;)
 
 int CodeCache::mark_for_deoptimization(DepChange& changes) {
   MutexLockerEx mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);

hotspot/src/share/vm/code/dependencies.cpp

@@ -725,56 +725,19 @@ Klass* Dependencies::DepStream::context_type() {
 }
 
 // ----------------- DependencySignature --------------------------------------
-bool DependencySignature::equals(const DependencySignature& sig) const {
-  if (type() != sig.type()) {
+bool DependencySignature::equals(DependencySignature* sig) const {
+  if ((type() != sig->type()) || (args_count() != sig->args_count())) {
     return false;
   }
 
-  if (args_count() != sig.args_count()) {
-    return false;
-  }
-
-  for (int i = 0; i < sig.args_count(); i++) {
-    if (arg(i) != sig.arg(i)) {
+  for (int i = 0; i < sig->args_count(); i++) {
+    if (arg(i) != sig->arg(i)) {
       return false;
     }
   }
   return true;
 }
 
-
-// ----------------- DependencySignatureBuffer --------------------------------------
-DependencySignatureBuffer::DependencySignatureBuffer() {
-  _signatures = NEW_RESOURCE_ARRAY(GrowableArray<DependencySignature*>*, Dependencies::TYPE_LIMIT);
-  memset(_signatures, 0, sizeof(DependencySignature*) * Dependencies::TYPE_LIMIT);
-}
-
-/* Check if arguments are identical. Two dependency signatures are considered
- * identical, if the type as well as all argument identifiers are identical.
- * If the dependency has not already been checked, the dependency signature is
- * added to the checked dependencies of the same type. The function returns
- * false, which causes the dependency to be checked in the caller.
- */
-bool DependencySignatureBuffer::add_if_missing(const DependencySignature& sig) {
-  const int index = sig.type();
-  GrowableArray<DependencySignature*>* buffer = _signatures[index];
-  if (buffer == NULL) {
-    buffer = new GrowableArray<DependencySignature*>();
-    _signatures[index] = buffer;
-  }
-
-  // Check if we have already checked the dependency
-  for (int i = 0; i < buffer->length(); i++) {
-    DependencySignature* checked_signature = buffer->at(i);
-    if (checked_signature->equals(sig)) {
-      return true;
-    }
-  }
-  buffer->append((DependencySignature*)&sig);
-  return false;
-}
-
-
 /// Checking dependencies:
 
 // This hierarchy walker inspects subtypes of a given type,

hotspot/src/share/vm/code/dependencies.hpp

@@ -32,6 +32,7 @@
 #include "code/compressedStream.hpp"
 #include "code/nmethod.hpp"
 #include "utilities/growableArray.hpp"
+#include "utilities/hashtable.hpp"
 
 //** Dependencies represent assertions (approximate invariants) within
 // the runtime system, e.g. class hierarchy changes.  An example is an
@@ -526,13 +527,12 @@ class Dependencies: public ResourceObj {
 };
 
 
-class DependencySignature : public ResourceObj {
+class DependencySignature : public GenericHashtableEntry<DependencySignature, ResourceObj> {
  private:
   int                   _args_count;
   uintptr_t             _argument_hash[Dependencies::max_arg_count];
   Dependencies::DepType _type;
 
-
  public:
   DependencySignature(Dependencies::DepStream& dep) {
     _args_count = dep.argument_count();
@@ -542,21 +542,14 @@ class DependencySignature : public ResourceObj {
     }
   }
 
-  bool equals(const DependencySignature& sig) const;
+  bool equals(DependencySignature* sig) const;
+  uintptr_t key() const { return _argument_hash[0] >> 2; }
 
   int args_count()             const { return _args_count; }
   uintptr_t arg(int idx)       const { return _argument_hash[idx]; }
   Dependencies::DepType type() const { return _type; }
 };
 
-class DependencySignatureBuffer : public StackObj {
- private:
-  GrowableArray<DependencySignature*>**  _signatures;
-
- public:
-  DependencySignatureBuffer();
-  bool add_if_missing(const DependencySignature& sig);
-};
 
 // Every particular DepChange is a sub-class of this class.
 class DepChange : public StackObj {

hotspot/src/share/vm/code/nmethod.cpp

@@ -2168,25 +2168,21 @@ void nmethod::check_all_dependencies(DepChange& changes) {
   // Turn off dependency tracing while actually testing dependencies.
   NOT_PRODUCT( FlagSetting fs(TraceDependencies, false) );
 
-  // 'dep_signature_buffers' caches already checked dependencies.
-  DependencySignatureBuffer dep_signature_buffers;
-
+ GenericHashtable<DependencySignature, ResourceObj>* table = new GenericHashtable<DependencySignature, ResourceObj>(11027);
   // Iterate over live nmethods and check dependencies of all nmethods that are not
   // marked for deoptimization. A particular dependency is only checked once.
   for(nmethod* nm = CodeCache::alive_nmethod(CodeCache::first()); nm != NULL; nm = CodeCache::alive_nmethod(CodeCache::next(nm))) {
     if (!nm->is_marked_for_deoptimization()) {
       for (Dependencies::DepStream deps(nm); deps.next(); ) {
         // Construct abstraction of a dependency.
-        const DependencySignature* current_sig = new DependencySignature(deps);
-        // Determine if 'deps' is already checked. If it is not checked,
-        // 'add_if_missing()' adds the dependency signature and returns
-        // false.
-        if (!dep_signature_buffers.add_if_missing(*current_sig)) {
+        DependencySignature* current_sig = new DependencySignature(deps);
+        // Determine if 'deps' is already checked. table->add() returns
+        // 'true' if the dependency was added (i.e., was not in the hashtable).
+        if (table->add(current_sig)) {
           if (deps.check_dependency() != NULL) {
             // Dependency checking failed. Print out information about the failed
             // dependency and finally fail with an assert. We can fail here, since
             // dependency checking is never done in a product build.
-            ResourceMark rm;
             changes.print();
             nm->print();
             nm->print_dependencies();

hotspot/src/share/vm/utilities/hashtable.cpp

@@ -25,6 +25,7 @@
 #include "precompiled.hpp"
 #include "classfile/altHashing.hpp"
 #include "classfile/javaClasses.hpp"
+#include "code/dependencies.hpp"
 #include "memory/allocation.inline.hpp"
 #include "memory/filemap.hpp"
 #include "memory/resourceArea.hpp"
@@ -338,7 +339,6 @@ template <MEMFLAGS F> void BasicHashtable<F>::verify() {
 
 #endif // PRODUCT
 
-
 #ifdef ASSERT
 
 template <MEMFLAGS F> void BasicHashtable<F>::verify_lookup_length(double load) {
@@ -351,6 +351,118 @@ template <MEMFLAGS F> void BasicHashtable<F>::verify_lookup_length(double load)
 }
 
 #endif
+
+
+template<class T, class M> GenericHashtable<T, M>::GenericHashtable(int size, bool C_heap, MEMFLAGS memflag) {
+  assert(size > 0, " Invalid hashtable size");
+  _size    = size;
+  _C_heap  = C_heap;
+  _memflag = memflag;
+  // Perform subtype-specific resource allocation
+  _items = (C_heap) ?  NEW_C_HEAP_ARRAY(T*, size, memflag) : NEW_RESOURCE_ARRAY(T*, size);
+  memset(_items, 0, sizeof(T*) * size);
+
+  DEBUG_ONLY(_num_items = 0;)
+}
+
+template<class T, class M> GenericHashtable<T, M>::~GenericHashtable() {
+  if (on_C_heap()) {
+    // Check backing array
+    for (int i = 0; i < size(); i++) {
+      T* item = head(i);
+      // Delete all items in linked list
+      while (item != NULL) {
+        T* next_item = item->next();
+        delete item;
+        DEBUG_ONLY(_num_items--);
+        item = next_item;
+      }
+    }
+    FREE_C_HEAP_ARRAY(T*, _items, _memflag);
+    _items = NULL;
+    assert (_num_items == 0, "Not all memory released");
+  }
+}
+
+/**
+ * Return a pointer to the item 'I' that is stored in the hashtable for
+ * which match_item->equals(I) == true. If no such item is found, NULL
+ * is returned.
+ */
+template<class T, class F> T* GenericHashtable<T, F>::contains(T* match_item) {
+  if (match_item != NULL) {
+    int idx = index(match_item);
+    return contains_impl(match_item, idx);
+  }
+  return NULL;
+}
+
+/**
+ * Add item to the hashtable. Return 'true' if the item was added
+ * and false otherwise.
+ */
+template<class T, class F> bool GenericHashtable<T, F>::add(T* item) {
+  if (item != NULL) {
+    int idx = index(item);
+    T* found_item = contains_impl(item, idx);
+    if (found_item == NULL) {
+      T* list_head = head(idx);
+      item->set_next(list_head);
+      item->set_prev(NULL);
+
+      if (list_head != NULL) {
+        list_head->set_prev(item);
+      }
+      set_head(item, idx);
+      DEBUG_ONLY(_num_items++);
+      return true;
+    }
+  }
+  return false;
+}
+
+/**
+ * Removes an item 'I' from the hashtable, if present. 'I' is removed, if
+ * match_item->equals(I) == true. Removing an item from the hashtable does
+ * not free memory.
+ */
+template<class T, class F> T* GenericHashtable<T, F>::remove(T* match_item) {
+  if (match_item != NULL) {
+    int idx = index(match_item);
+    T* found_item = contains_impl(match_item, idx);
+    if (found_item != NULL) {
+      // Remove item from linked list
+      T* prev = found_item->prev();
+      T* next = found_item->next();
+      if (prev != NULL) {
+        prev->set_next(next);
+      } else {
+        set_head(next, idx);
+      }
+      if (next != NULL) {
+        next->set_prev(prev);
+      }
+
+      DEBUG_ONLY(_num_items--);
+      return found_item;
+    }
+  }
+  return NULL;
+}
+
+
+template<class T, class F> T* GenericHashtable<T, F>::contains_impl(T* item, int idx) {
+  T* current_item = head(idx);
+  while (current_item != NULL) {
+    if (current_item->equals(item)) {
+      return current_item;
+    }
+    current_item = current_item->next();
+  }
+  return NULL;
+}
+
+
 // Explicitly instantiate these types
 template class Hashtable<ConstantPool*, mtClass>;
 template class Hashtable<Symbol*, mtSymbol>;
@@ -370,3 +482,5 @@ template class BasicHashtable<mtClass>;
 template class BasicHashtable<mtSymbol>;
 template class BasicHashtable<mtCode>;
 template class BasicHashtable<mtInternal>;
+
+template class GenericHashtable<DependencySignature, ResourceObj>;

hotspot/src/share/vm/utilities/hashtable.hpp

@@ -300,7 +300,7 @@ public:
 };
 
 
-//  Verions of hashtable where two handles are used to compute the index.
+// Versions of hashtable where two handles are used to compute the index.
 
 template <class T, MEMFLAGS F> class TwoOopHashtable : public Hashtable<T, F> {
   friend class VMStructs;
@@ -327,4 +327,86 @@ public:
   }
 };
 
+
+/*
+ * Usage of GenericHashtable:
+ *
+ * class X : public GenericHashtableEntry<X, ResourceObj> {
+ *
+ *   // Implement virtual functions in class X
+ *   bool      equals(X* sig) const;
+ *   uintptr_t hash()         const;
+ * };
+ *
+ * void foo() {
+ *   GenericHashtable<X, ResourceObj>* table = new GenericHashtable<X, ResourceObj>(11027, false);
+ *
+ *   X* elem = new X();
+ *   table->add(elem);
+ *   table->contains(elem);
+ * }
+ *
+ * You can choose other allocation types as well. For example, to store the hashtable to a
+ * particular region (CHeapObj<type>) simply replace ResourceObj with the desired type:
+ *
+ * class X : public GenericHashtableEntry<X, CHeapObj<mtCode> > { ... };
+ *
+ * To make the destructor (and remove) of the hashtable work:
+ * 1) override the delete operator of X
+ * 2) provide a destructor of the X
+ *
+ * You may also find it convenient to override the new operator.
+ *
+ * If you use this templates do not forget to add an explicit initialization
+ * (at the end of hashtable.cpp).
+ *
+ *  template class GenericHashtable<X, ResourceObj>;
+ */
+template <class T, class M> class GenericHashtableEntry : public M {
+ private:
+  T* _next;
+  T* _prev;
+ public:
+  // Must be implemented by subclass.
+  virtual uintptr_t key()            const = 0;
+  virtual bool      equals(T* other) const = 0;
+
+  T* next() const        { return _next; }
+  T* prev() const        { return _prev; }
+  void set_next(T* item) { _next = item; }
+  void set_prev(T* item) { _prev = item; }
+
+  // Constructor and destructor
+  GenericHashtableEntry() : _next(NULL), _prev(NULL) { };
+  virtual ~GenericHashtableEntry() {};
+};
+
+template <class T, class M> class GenericHashtable : public M {
+ private:
+  T**      _items;
+  int      _size;
+  bool     _C_heap;
+  MEMFLAGS _memflag;
+
+  // Accessor methods
+  T*   head    (int idx) const    { return _items[idx]; }
+  void set_head(T* item, int idx) { _items[idx] = item; }
+  int  index   (T* item)          { assert(item != NULL, "missing null check"); return item->key() % size(); }
+
+  // Helper function
+  T* contains_impl(T* item, int idx);
+
+  DEBUG_ONLY(int _num_items;)
+ public:
+  GenericHashtable(int size, bool C_heap = false, MEMFLAGS memflag = mtNone);
+  ~GenericHashtable();
+  T*   contains(T* match_item);
+  T*   remove  (T* match_item);
+  bool add     (T* item);
+
+
+  bool on_C_heap() const { return _C_heap; }
+  int  size()      const { return _size; }
+};
+
 #endif // SHARE_VM_UTILITIES_HASHTABLE_HPP