8314502: Change the comparator taking version of GrowableArray::find to be a template method

Reviewed-by: jsjolen, sspitsyn, stefank

Author: Afshin Zafari

src/hotspot/share/gc/parallel/mutableNUMASpace.cpp

@@ -142,6 +142,11 @@ size_t MutableNUMASpace::free_in_words() const {
   return s;
 }
 
+int MutableNUMASpace::lgrp_space_index(int lgrp_id) const {
+  return lgrp_spaces()->find_if([&](LGRPSpace* space) {
+    return space->lgrp_id() == checked_cast<uint>(lgrp_id);
+  });
+}
 
 size_t MutableNUMASpace::tlab_capacity(Thread *thr) const {
   guarantee(thr != nullptr, "No thread");
@@ -160,7 +165,7 @@ size_t MutableNUMASpace::tlab_capacity(Thread *thr) const {
     }
   }
   // That's the normal case, where we know the locality group of the thread.
-  int i = lgrp_spaces()->find(&lgrp_id, LGRPSpace::equals);
+  int i = lgrp_space_index(lgrp_id);
   if (i == -1) {
     return 0;
   }
@@ -179,7 +184,7 @@ size_t MutableNUMASpace::tlab_used(Thread *thr) const {
       return 0;
     }
   }
-  int i = lgrp_spaces()->find(&lgrp_id, LGRPSpace::equals);
+  int i = lgrp_space_index(lgrp_id);
   if (i == -1) {
     return 0;
   }
@@ -199,7 +204,7 @@ size_t MutableNUMASpace::unsafe_max_tlab_alloc(Thread *thr) const {
       return 0;
     }
   }
-  int i = lgrp_spaces()->find(&lgrp_id, LGRPSpace::equals);
+  int i = lgrp_space_index(lgrp_id);
   if (i == -1) {
     return 0;
   }
@@ -569,7 +574,7 @@ HeapWord* MutableNUMASpace::cas_allocate(size_t size) {
     thr->set_lgrp_id(lgrp_id);
   }
 
-  int i = lgrp_spaces()->find(&lgrp_id, LGRPSpace::equals);
+  int i = lgrp_space_index(lgrp_id);
   // It is possible that a new CPU has been hotplugged and
   // we haven't reshaped the space accordingly.
   if (i == -1) {

src/hotspot/share/gc/parallel/mutableNUMASpace.hpp

@@ -93,10 +93,6 @@ class MutableNUMASpace : public MutableSpace {
       delete _alloc_rate;
     }
 
-    static bool equals(void* lgrp_id_value, LGRPSpace* p) {
-      return *(uint*)lgrp_id_value == p->lgrp_id();
-    }
-
     // Report a failed allocation.
     void set_allocation_failed() { _allocation_failed = true;  }
 
@@ -158,6 +154,8 @@ class MutableNUMASpace : public MutableSpace {
   void select_tails(MemRegion new_region, MemRegion intersection,
                     MemRegion* bottom_region, MemRegion *top_region);
 
+  int lgrp_space_index(int lgrp_id) const;
+
 public:
   GrowableArray<LGRPSpace*>* lgrp_spaces() const     { return _lgrp_spaces;       }
   MutableNUMASpace(size_t alignment);

src/hotspot/share/prims/jvmtiImpl.cpp

@@ -119,14 +119,6 @@ void GrowableCache::recache() {
   _listener_fun(_this_obj,_cache);
 }
 
-bool GrowableCache::equals(void* v, GrowableElement *e2) {
-  GrowableElement *e1 = (GrowableElement *) v;
-  assert(e1 != nullptr, "e1 != nullptr");
-  assert(e2 != nullptr, "e2 != nullptr");
-
-  return e1->equals(e2);
-}
-
 //
 // class GrowableCache - public methods
 //
@@ -163,8 +155,8 @@ GrowableElement* GrowableCache::at(int index) {
   return e;
 }
 
-int GrowableCache::find(GrowableElement* e) {
-  return _elements->find(e, GrowableCache::equals);
+int GrowableCache::find(const GrowableElement* e) const {
+  return _elements->find_if([&](const GrowableElement* other_e) { return e->equals(other_e); });
 }
 
 // append a copy of the element to the end of the collection
@@ -216,7 +208,7 @@ void JvmtiBreakpoint::copy(JvmtiBreakpoint& bp) {
   _class_holder = OopHandle(JvmtiExport::jvmti_oop_storage(), bp._class_holder.resolve());
 }
 
-bool JvmtiBreakpoint::equals(JvmtiBreakpoint& bp) {
+bool JvmtiBreakpoint::equals(const JvmtiBreakpoint& bp) const {
   return _method   == bp._method
     &&   _bci      == bp._bci;
 }

src/hotspot/share/prims/jvmtiImpl.hpp

@@ -66,9 +66,9 @@ class JvmtiBreakpoints;
 class GrowableElement : public CHeapObj<mtInternal> {
 public:
   virtual ~GrowableElement() {}
-  virtual address getCacheValue()          =0;
-  virtual bool equals(GrowableElement* e)  =0;
-  virtual GrowableElement *clone()         =0;
+  virtual address getCacheValue()                     =0;
+  virtual bool equals(const GrowableElement* e) const =0;
+  virtual GrowableElement* clone()                    =0;
 };
 
 class GrowableCache {
@@ -88,8 +88,6 @@ class GrowableCache {
   // (but NOT when cached elements are recomputed).
   void (*_listener_fun)(void *, address*);
 
-  static bool equals(void *, GrowableElement *);
-
   // recache all elements after size change, notify listener
   void recache();
 
@@ -104,7 +102,7 @@ class GrowableCache {
   // get the value of the index element in the collection
   GrowableElement* at(int index);
   // find the index of the element, -1 if it doesn't exist
-  int find(GrowableElement* e);
+  int find(const GrowableElement* e) const;
   // append a copy of the element to the end of the collection, notify listener
   void append(GrowableElement* e);
   // remove the element at index, notify listener
@@ -165,7 +163,7 @@ class JvmtiBreakpoint : public GrowableElement {
   JvmtiBreakpoint() : _method(nullptr), _bci(0) {}
   JvmtiBreakpoint(Method* m_method, jlocation location);
   virtual ~JvmtiBreakpoint();
-  bool equals(JvmtiBreakpoint& bp);
+  bool equals(const JvmtiBreakpoint& bp) const;
   void copy(JvmtiBreakpoint& bp);
   address getBcp() const;
   void each_method_version_do(method_action meth_act);
@@ -177,7 +175,7 @@ class JvmtiBreakpoint : public GrowableElement {
 
   // GrowableElement implementation
   address getCacheValue()         { return getBcp(); }
-  bool equals(GrowableElement* e) { return equals((JvmtiBreakpoint&) *e); }
+  bool equals(const GrowableElement* e) const { return equals((const JvmtiBreakpoint&) *e); }
 
   GrowableElement *clone()        {
     JvmtiBreakpoint *bp = new JvmtiBreakpoint();

src/hotspot/share/runtime/perfData.cpp

@@ -185,6 +185,10 @@ void PerfData::create_entry(BasicType dtype, size_t dsize, size_t vlen) {
   PerfMemory::mark_updated();
 }
 
+bool PerfData::name_equals(const char* name) const {
+  return strcmp(name, this->name()) == 0;
+}
+
 PerfLong::PerfLong(CounterNS ns, const char* namep, Units u, Variability v)
                  : PerfData(ns, namep, u, v) {
 
@@ -501,17 +505,9 @@ PerfDataList::~PerfDataList() {
 
 }
 
-bool PerfDataList::by_name(void* name, PerfData* pd) {
-
-  if (pd == nullptr)
-    return false;
-
-  return strcmp((const char*)name, pd->name()) == 0;
-}
-
 PerfData* PerfDataList::find_by_name(const char* name) {
 
-  int i = _set->find((void*)name, PerfDataList::by_name);
+  int i = _set->find_if([&](PerfData* pd) { return pd->name_equals(name); });
 
   if (i >= 0 && i <= _set->length())
     return _set->at(i);

src/hotspot/share/runtime/perfData.hpp

@@ -319,7 +319,8 @@ class PerfData : public CHeapObj<mtInternal> {
     // PerfData memory region. This redundancy is maintained for
     // security reasons as the PerfMemory region may be in shared
     // memory.
-    const char* name() { return _name; }
+    const char* name() const { return _name; }
+    bool name_equals(const char* name) const;
 
     // returns the variability classification associated with this item
     Variability variability() { return _v; }
@@ -576,7 +577,7 @@ class PerfDataList : public CHeapObj<mtInternal> {
     PerfDataArray* _set;
 
     // method to search for a instrumentation object by name
-    static bool by_name(void* name, PerfData* pd);
+    static bool by_name(const char* name, PerfData* pd);
 
   protected:
     // we expose the implementation here to facilitate the clone

src/hotspot/share/runtime/unhandledOops.cpp

@@ -71,19 +71,16 @@ void UnhandledOops::register_unhandled_oop(oop* op) {
   _oop_list->push(entry);
 }
 
-
-bool match_oop_entry(void *op, UnhandledOopEntry e) {
-  return (e.oop_ptr() == op);
-}
-
 // Mark unhandled oop as okay for GC - the containing struct has an oops_do and
 // for some reason the oop has to be on the stack.
 // May not be called for the current thread, as in the case of
 // VM_GetOrSetLocal in jvmti.
 void UnhandledOops::allow_unhandled_oop(oop* op) {
   assert (CheckUnhandledOops, "should only be called with checking option");
 
-  int i = _oop_list->find_from_end(op, match_oop_entry);
+  int i = _oop_list->find_from_end_if([&](const UnhandledOopEntry& e) {
+    return e.match_oop_entry(op);
+  });
   assert(i!=-1, "safe for gc oop not in unhandled_oop_list");
 
   UnhandledOopEntry entry = _oop_list->at(i);
@@ -105,7 +102,9 @@ void UnhandledOops::unregister_unhandled_oop(oop* op) {
   }
   _level--;
 
-  int i = _oop_list->find_from_end(op, match_oop_entry);
+  int i = _oop_list->find_from_end_if([&](const UnhandledOopEntry& e) {
+    return e.match_oop_entry(op);
+  });
   assert(i!=-1, "oop not in unhandled_oop_list");
   _oop_list->remove_at(i);
 }

src/hotspot/share/runtime/unhandledOops.hpp

@@ -53,14 +53,17 @@ class UnhandledOopEntry : public CHeapObj<mtThread> {
  private:
   oop* _oop_ptr;
   bool _ok_for_gc;
+
+  bool match_oop_entry(oop* op) const {
+    return _oop_ptr == op;
+  }
+
  public:
-  oop* oop_ptr() { return _oop_ptr; }
   UnhandledOopEntry() : _oop_ptr(nullptr), _ok_for_gc(false) {}
   UnhandledOopEntry(oop* op) :
                         _oop_ptr(op),   _ok_for_gc(false) {}
 };
 
-
 class UnhandledOops : public CHeapObj<mtThread> {
  friend class Thread;
  private:

src/hotspot/share/services/diagnosticFramework.cpp

@@ -144,9 +144,8 @@ bool DCmdArgIter::next(TRAPS) {
   return _key_len != 0;
 }
 
-bool DCmdInfo::by_name(void* cmd_name, DCmdInfo* info) {
-  if (info == nullptr) return false;
-  return strcmp((const char*)cmd_name, info->name()) == 0;
+bool DCmdInfo::name_equals(const char* name) const {
+  return strcmp(name, this->name()) == 0;
 }
 
 void DCmdParser::add_dcmd_option(GenDCmdArgument* arg) {

src/hotspot/share/services/diagnosticFramework.hpp

@@ -140,13 +140,12 @@ class DCmdInfo : public ResourceObj {
   : _name(name), _description(description), _impact(impact), _permission(permission),
     _num_arguments(num_arguments), _is_enabled(enabled) {}
   const char* name() const          { return _name; }
+  bool name_equals(const char* cmd_name) const;
   const char* description() const   { return _description; }
   const char* impact() const        { return _impact; }
   const JavaPermission& permission() const { return _permission; }
   int num_arguments() const         { return _num_arguments; }
   bool is_enabled() const           { return _is_enabled; }
-
-  static bool by_name(void* name, DCmdInfo* info);
 };
 
 // A DCmdArgumentInfo instance provides a description of a diagnostic command

src/hotspot/share/services/management.cpp

@@ -2004,7 +2004,9 @@ JVM_ENTRY(void, jmm_GetDiagnosticCommandInfo(JNIEnv *env, jobjectArray cmds,
         THROW_MSG(vmSymbols::java_lang_NullPointerException(),
                 "Command name cannot be null.");
     }
-    int pos = info_list->find((void*)cmd_name,DCmdInfo::by_name);
+    int pos = info_list->find_if([&](DCmdInfo* info) {
+      return info->name_equals(cmd_name);
+    });
     if (pos == -1) {
         THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
              "Unknown diagnostic command");

src/hotspot/share/utilities/growableArray.hpp

@@ -209,17 +209,29 @@ class GrowableArrayView : public GrowableArrayBase {
     return -1;
   }
 
-  int  find(void* token, bool f(void*, E)) const {
+  // Find first element that matches the given predicate.
+  //
+  // Predicate: bool predicate(const E& elem)
+  //
+  // Returns the index of the element or -1 if no element matches the predicate
+  template<typename Predicate>
+  int find_if(Predicate predicate) const {
     for (int i = 0; i < _len; i++) {
-      if (f(token, _data[i])) return i;
+      if (predicate(_data[i])) return i;
     }
     return -1;
   }
 
-  int  find_from_end(void* token, bool f(void*, E)) const {
+  // Find last element that matches the given predicate.
+  //
+  // Predicate: bool predicate(const E& elem)
+  //
+  // Returns the index of the element or -1 if no element matches the predicate
+  template<typename Predicate>
+  int find_from_end_if(Predicate predicate) const {
     // start at the end of the array
     for (int i = _len-1; i >= 0; i--) {
-      if (f(token, _data[i])) return i;
+      if (predicate(_data[i])) return i;
     }
     return -1;
   }

test/hotspot/gtest/utilities/test_growableArray.cpp

@@ -601,3 +601,65 @@ TEST(GrowableArrayCHeap, sanity) {
     delete a;
   }
 }
+
+TEST(GrowableArrayCHeap, find_if) {
+  struct Element {
+    int value;
+  };
+  GrowableArrayCHeap<Element, mtTest> array;
+  array.push({1});
+  array.push({2});
+  array.push({3});
+
+  {
+    int index = array.find_if([&](const Element& elem) {
+      return elem.value == 1;
+    });
+    ASSERT_EQ(index, 0);
+  }
+
+  {
+    int index = array.find_if([&](const Element& elem) {
+      return elem.value > 1;
+    });
+    ASSERT_EQ(index, 1);
+  }
+
+  {
+    int index = array.find_if([&](const Element& elem) {
+      return elem.value == 4;
+    });
+    ASSERT_EQ(index, -1);
+  }
+}
+
+TEST(GrowableArrayCHeap, find_from_end_if) {
+  struct Element {
+    int value;
+  };
+  GrowableArrayCHeap<Element, mtTest> array;
+  array.push({1});
+  array.push({2});
+  array.push({3});
+
+  {
+    int index = array.find_from_end_if([&](const Element& elem) {
+      return elem.value == 1;
+    });
+    ASSERT_EQ(index, 0);
+  }
+
+  {
+    int index = array.find_from_end_if([&](const Element& elem) {
+      return elem.value > 1;
+    });
+    ASSERT_EQ(index, 2);
+  }
+
+  {
+    int index = array.find_from_end_if([&](const Element& elem) {
+      return elem.value == 4;
+    });
+    ASSERT_EQ(index, -1);
+  }
+}