8048268: G1 Code Root Migration performs poorly

Summary: Replace G1CodeRootSet with a Hashtable based implementation, merge Code Root Migration phase into Code Root Scanning
Reviewed-by: jmasa, brutisso, tschatzl

src/share/vm/gc_implementation/g1/g1CodeCacheRemSet.cpp

@@ -22,372 +22,375 @@
  *
  */
 
-
 #include "precompiled.hpp"
+#include "code/codeCache.hpp"
 #include "code/nmethod.hpp"
 #include "gc_implementation/g1/g1CodeCacheRemSet.hpp"
+#include "gc_implementation/g1/heapRegion.hpp"
+#include "memory/heap.hpp"
 #include "memory/iterator.hpp"
+#include "oops/oop.inline.hpp"
+#include "utilities/hashtable.inline.hpp"
+#include "utilities/stack.inline.hpp"
 
 PRAGMA_FORMAT_MUTE_WARNINGS_FOR_GCC
 
-G1CodeRootChunk::G1CodeRootChunk() : _top(NULL), _next(NULL), _prev(NULL), _free(NULL) {
-  _top = bottom();
-}
-
-void G1CodeRootChunk::reset() {
-  _next = _prev = NULL;
-  _free = NULL;
-  _top = bottom();
-}
+class CodeRootSetTable : public Hashtable<nmethod*, mtGC> {
+  friend class G1CodeRootSetTest;
+  typedef HashtableEntry<nmethod*, mtGC> Entry;
 
-void G1CodeRootChunk::nmethods_do(CodeBlobClosure* cl) {
-  NmethodOrLink* cur = bottom();
-  while (cur != _top) {
-    if (is_nmethod(cur)) {
-      cl->do_code_blob(cur->_nmethod);
-    }
-    cur++;
-  }
-}
+  static CodeRootSetTable* volatile _purge_list;
 
-bool G1CodeRootChunk::remove_lock_free(nmethod* method) {
-  NmethodOrLink* cur = bottom();
+  CodeRootSetTable* _purge_next;
 
-  for (NmethodOrLink* cur = bottom(); cur != _top; cur++) {
-    if (cur->_nmethod == method) {
-      bool result = Atomic::cmpxchg_ptr(NULL, &cur->_nmethod, method) == method;
+  unsigned int compute_hash(nmethod* nm) {
+    uintptr_t hash = (uintptr_t)nm;
+    return hash ^ (hash >> 7); // code heap blocks are 128byte aligned
+  }
 
-      if (!result) {
-        // Someone else cleared out this entry.
-        return false;
-      }
+  Entry* new_entry(nmethod* nm);
 
-      // The method was cleared. Time to link it into the free list.
-      NmethodOrLink* prev_free;
-      do {
-        prev_free = (NmethodOrLink*)_free;
-        cur->_link = prev_free;
-      } while (Atomic::cmpxchg_ptr(cur, &_free, prev_free) != prev_free);
+ public:
+  CodeRootSetTable(int size) : Hashtable<nmethod*, mtGC>(size, sizeof(Entry)), _purge_next(NULL) {}
+  ~CodeRootSetTable();
 
-      return true;
-    }
-  }
+  // Needs to be protected locks
+  bool add(nmethod* nm);
+  bool remove(nmethod* nm);
 
-  return false;
-}
+  // Can be called without locking
+  bool contains(nmethod* nm);
 
-G1CodeRootChunkManager::G1CodeRootChunkManager() : _free_list(), _num_chunks_handed_out(0) {
-  _free_list.initialize();
-  _free_list.set_size(G1CodeRootChunk::word_size());
-}
+  int entry_size() const { return BasicHashtable<mtGC>::entry_size(); }
 
-size_t G1CodeRootChunkManager::fl_mem_size() {
-  return _free_list.count() * _free_list.size();
-}
+  void copy_to(CodeRootSetTable* new_table);
+  void nmethods_do(CodeBlobClosure* blk);
 
-void G1CodeRootChunkManager::free_all_chunks(FreeList<G1CodeRootChunk>* list) {
-  _num_chunks_handed_out -= list->count();
-  _free_list.prepend(list);
-}
+  template<typename CB>
+  void remove_if(CB& should_remove);
 
-void G1CodeRootChunkManager::free_chunk(G1CodeRootChunk* chunk) {
-  _free_list.return_chunk_at_head(chunk);
-  _num_chunks_handed_out--;
-}
+  static void purge_list_append(CodeRootSetTable* tbl);
+  static void purge();
 
-void G1CodeRootChunkManager::purge_chunks(size_t keep_ratio) {
-  size_t keep = _num_chunks_handed_out * keep_ratio / 100;
-  if (keep >= (size_t)_free_list.count()) {
-    return;
+  static size_t static_mem_size() {
+    return sizeof(_purge_list);
   }
+};
 
-  FreeList<G1CodeRootChunk> temp;
-  temp.initialize();
-  temp.set_size(G1CodeRootChunk::word_size());
-
-  _free_list.getFirstNChunksFromList((size_t)_free_list.count() - keep, &temp);
+CodeRootSetTable* volatile CodeRootSetTable::_purge_list = NULL;
 
-  G1CodeRootChunk* cur = temp.get_chunk_at_head();
-  while (cur != NULL) {
-    delete cur;
-    cur = temp.get_chunk_at_head();
+CodeRootSetTable::Entry* CodeRootSetTable::new_entry(nmethod* nm) {
+  unsigned int hash = compute_hash(nm);
+  Entry* entry = (Entry*) new_entry_free_list();
+  if (entry == NULL) {
+    entry = (Entry*) NEW_C_HEAP_ARRAY2(char, entry_size(), mtGC, CURRENT_PC);
   }
+  entry->set_next(NULL);
+  entry->set_hash(hash);
+  entry->set_literal(nm);
+  return entry;
 }
 
-size_t G1CodeRootChunkManager::static_mem_size() {
-  return sizeof(G1CodeRootChunkManager);
+CodeRootSetTable::~CodeRootSetTable() {
+  for (int index = 0; index < table_size(); ++index) {
+    for (Entry* e = bucket(index); e != NULL; ) {
+      Entry* to_remove = e;
+      // read next before freeing.
+      e = e->next();
+      unlink_entry(to_remove);
+      FREE_C_HEAP_ARRAY(char, to_remove, mtGC);
+    }
+  }
+  assert(number_of_entries() == 0, "should have removed all entries");
+  free_buckets();
+  for (BasicHashtableEntry<mtGC>* e = new_entry_free_list(); e != NULL; e = new_entry_free_list()) {
+    FREE_C_HEAP_ARRAY(char, e, mtGC);
+  }
 }
 
-
-G1CodeRootChunk* G1CodeRootChunkManager::new_chunk() {
-  G1CodeRootChunk* result = _free_list.get_chunk_at_head();
-  if (result == NULL) {
-    result = new G1CodeRootChunk();
+bool CodeRootSetTable::add(nmethod* nm) {
+  if (!contains(nm)) {
+    Entry* e = new_entry(nm);
+    int index = hash_to_index(e->hash());
+    add_entry(index, e);
+    return true;
   }
-  _num_chunks_handed_out++;
-  result->reset();
-  return result;
+  return false;
 }
 
-#ifndef PRODUCT
+bool CodeRootSetTable::contains(nmethod* nm) {
+  int index = hash_to_index(compute_hash(nm));
+  for (Entry* e = bucket(index); e != NULL; e = e->next()) {
+    if (e->literal() == nm) {
+      return true;
+    }
+  }
+  return false;
+}
 
-size_t G1CodeRootChunkManager::num_chunks_handed_out() const {
-  return _num_chunks_handed_out;
+bool CodeRootSetTable::remove(nmethod* nm) {
+  int index = hash_to_index(compute_hash(nm));
+  Entry* previous = NULL;
+  for (Entry* e = bucket(index); e != NULL; previous = e, e = e->next()) {
+    if (e->literal() == nm) {
+      if (previous != NULL) {
+        previous->set_next(e->next());
+      } else {
+        set_entry(index, e->next());
+      }
+      free_entry(e);
+      return true;
+    }
+  }
+  return false;
 }
 
-size_t G1CodeRootChunkManager::num_free_chunks() const {
-  return (size_t)_free_list.count();
+void CodeRootSetTable::copy_to(CodeRootSetTable* new_table) {
+  for (int index = 0; index < table_size(); ++index) {
+    for (Entry* e = bucket(index); e != NULL; e = e->next()) {
+      new_table->add(e->literal());
+    }
+  }
+  new_table->copy_freelist(this);
 }
 
-#endif
+void CodeRootSetTable::nmethods_do(CodeBlobClosure* blk) {
+  for (int index = 0; index < table_size(); ++index) {
+    for (Entry* e = bucket(index); e != NULL; e = e->next()) {
+      blk->do_code_blob(e->literal());
+    }
+  }
+}
 
-G1CodeRootChunkManager G1CodeRootSet::_default_chunk_manager;
+template<typename CB>
+void CodeRootSetTable::remove_if(CB& should_remove) {
+  for (int index = 0; index < table_size(); ++index) {
+    Entry* previous = NULL;
+    Entry* e = bucket(index);
+    while (e != NULL) {
+      Entry* next = e->next();
+      if (should_remove(e->literal())) {
+        if (previous != NULL) {
+          previous->set_next(next);
+        } else {
+          set_entry(index, next);
+        }
+        free_entry(e);
+      } else {
+        previous = e;
+      }
+      e = next;
+    }
+  }
+}
 
-void G1CodeRootSet::purge_chunks(size_t keep_ratio) {
-  _default_chunk_manager.purge_chunks(keep_ratio);
+G1CodeRootSet::~G1CodeRootSet() {
+  delete _table;
 }
 
-size_t G1CodeRootSet::free_chunks_static_mem_size() {
-  return _default_chunk_manager.static_mem_size();
+CodeRootSetTable* G1CodeRootSet::load_acquire_table() {
+  return (CodeRootSetTable*) OrderAccess::load_ptr_acquire(&_table);
 }
 
-size_t G1CodeRootSet::free_chunks_mem_size() {
-  return _default_chunk_manager.fl_mem_size();
+void G1CodeRootSet::allocate_small_table() {
+  _table = new CodeRootSetTable(SmallSize);
 }
 
-G1CodeRootSet::G1CodeRootSet(G1CodeRootChunkManager* manager) : _manager(manager), _list(), _length(0) {
-  if (_manager == NULL) {
-    _manager = &_default_chunk_manager;
+void CodeRootSetTable::purge_list_append(CodeRootSetTable* table) {
+  for (;;) {
+    table->_purge_next = _purge_list;
+    CodeRootSetTable* old = (CodeRootSetTable*) Atomic::cmpxchg_ptr(table, &_purge_list, table->_purge_next);
+    if (old == table->_purge_next) {
+      break;
+    }
   }
-  _list.initialize();
-  _list.set_size(G1CodeRootChunk::word_size());
 }
 
-G1CodeRootSet::~G1CodeRootSet() {
-  clear();
+void CodeRootSetTable::purge() {
+  CodeRootSetTable* table = _purge_list;
+  _purge_list = NULL;
+  while (table != NULL) {
+    CodeRootSetTable* to_purge = table;
+    table = table->_purge_next;
+    delete to_purge;
+  }
 }
 
-void G1CodeRootSet::add(nmethod* method) {
-  if (!contains(method)) {
-    // Find the first chunk that isn't full.
-    G1CodeRootChunk* cur = _list.head();
-    while (cur != NULL) {
-      if (!cur->is_full()) {
-        break;
-      }
-      cur = cur->next();
-    }
+void G1CodeRootSet::move_to_large() {
+  CodeRootSetTable* temp = new CodeRootSetTable(LargeSize);
 
-    // All chunks are full, get a new chunk.
-    if (cur == NULL) {
-      cur = new_chunk();
-      _list.return_chunk_at_head(cur);
-    }
+  _table->copy_to(temp);
 
-    // Add the nmethod.
-    bool result = cur->add(method);
+  CodeRootSetTable::purge_list_append(_table);
 
-    guarantee(result, err_msg("Not able to add nmethod "PTR_FORMAT" to newly allocated chunk.", method));
+  OrderAccess::release_store_ptr(&_table, temp);
+}
 
-    _length++;
-  }
+
+void G1CodeRootSet::purge() {
+  CodeRootSetTable::purge();
 }
 
-void G1CodeRootSet::remove_lock_free(nmethod* method) {
-  G1CodeRootChunk* found = find(method);
-  if (found != NULL) {
-    bool result = found->remove_lock_free(method);
-    if (result) {
-      Atomic::dec_ptr((volatile intptr_t*)&_length);
-    }
-  }
-  assert(!contains(method), err_msg(PTR_FORMAT" still contains nmethod "PTR_FORMAT, this, method));
+size_t G1CodeRootSet::static_mem_size() {
+  return CodeRootSetTable::static_mem_size();
 }
 
-nmethod* G1CodeRootSet::pop() {
-  while (true) {
-    G1CodeRootChunk* cur = _list.head();
-    if (cur == NULL) {
-      assert(_length == 0, "when there are no chunks, there should be no elements");
-      return NULL;
-    }
-    nmethod* result = cur->pop();
-    if (result != NULL) {
-      _length--;
-      return result;
-    } else {
-      free(_list.get_chunk_at_head());
-    }
+void G1CodeRootSet::add(nmethod* method) {
+  bool added = false;
+  if (is_empty()) {
+    allocate_small_table();
+  }
+  added = _table->add(method);
+  if (_length == Threshold) {
+    move_to_large();
+  }
+  if (added) {
+    ++_length;
   }
 }
 
-G1CodeRootChunk* G1CodeRootSet::find(nmethod* method) {
-  G1CodeRootChunk* cur = _list.head();
-  while (cur != NULL) {
-    if (cur->contains(method)) {
-      return cur;
+bool G1CodeRootSet::remove(nmethod* method) {
+  bool removed = false;
+  if (_table != NULL) {
+    removed = _table->remove(method);
+  }
+  if (removed) {
+    _length--;
+    if (_length == 0) {
+      clear();
     }
-    cur = (G1CodeRootChunk*)cur->next();
   }
-  return NULL;
-}
-
-void G1CodeRootSet::free(G1CodeRootChunk* chunk) {
-  free_chunk(chunk);
+  return removed;
 }
 
 bool G1CodeRootSet::contains(nmethod* method) {
-  return find(method) != NULL;
+  CodeRootSetTable* table = load_acquire_table();
+  if (table != NULL) {
+    return table->contains(method);
+  }
+  return false;
 }
 
 void G1CodeRootSet::clear() {
-  free_all_chunks(&_list);
+  delete _table;
+  _table = NULL;
   _length = 0;
 }
 
+size_t G1CodeRootSet::mem_size() {
+  return sizeof(*this) +
+      (_table != NULL ? sizeof(CodeRootSetTable) + _table->entry_size() * _length : 0);
+}
+
 void G1CodeRootSet::nmethods_do(CodeBlobClosure* blk) const {
-  G1CodeRootChunk* cur = _list.head();
-  while (cur != NULL) {
-    cur->nmethods_do(blk);
-    cur = (G1CodeRootChunk*)cur->next();
+  if (_table != NULL) {
+    _table->nmethods_do(blk);
   }
 }
 
-size_t G1CodeRootSet::static_mem_size() {
-  return sizeof(G1CodeRootSet);
-}
+class CleanCallback : public StackObj {
+  class PointsIntoHRDetectionClosure : public OopClosure {
+    HeapRegion* _hr;
+   public:
+    bool _points_into;
+    PointsIntoHRDetectionClosure(HeapRegion* hr) : _hr(hr), _points_into(false) {}
 
-size_t G1CodeRootSet::mem_size() {
-  return G1CodeRootSet::static_mem_size() + _list.count() * _list.size();
-}
+    void do_oop(narrowOop* o) {
+      do_oop_work(o);
+    }
 
-#ifndef PRODUCT
+    void do_oop(oop* o) {
+      do_oop_work(o);
+    }
 
-void G1CodeRootSet::test() {
-  G1CodeRootChunkManager mgr;
+    template <typename T>
+    void do_oop_work(T* p) {
+      if (_hr->is_in(oopDesc::load_decode_heap_oop(p))) {
+        _points_into = true;
+      }
+    }
+  };
 
-  assert(mgr.num_chunks_handed_out() == 0, "Must not have handed out chunks yet");
+  PointsIntoHRDetectionClosure _detector;
+  CodeBlobToOopClosure _blobs;
 
-  assert(G1CodeRootChunkManager::static_mem_size() > sizeof(void*),
-         err_msg("The chunk manager's static memory usage seems too small, is only "SIZE_FORMAT" bytes.", G1CodeRootChunkManager::static_mem_size()));
+ public:
+  CleanCallback(HeapRegion* hr) : _detector(hr), _blobs(&_detector, !CodeBlobToOopClosure::FixRelocations) {}
 
-  // The number of chunks that we allocate for purge testing.
-  size_t const num_chunks = 10;
+  bool operator() (nmethod* nm) {
+    _detector._points_into = false;
+    _blobs.do_code_blob(nm);
+    return _detector._points_into;
+  }
+};
 
-  {
-    G1CodeRootSet set1(&mgr);
-    assert(set1.is_empty(), "Code root set must be initially empty but is not.");
+void G1CodeRootSet::clean(HeapRegion* owner) {
+  CleanCallback should_clean(owner);
+  if (_table != NULL) {
+    _table->remove_if(should_clean);
+  }
+}
 
-    assert(G1CodeRootSet::static_mem_size() > sizeof(void*),
-           err_msg("The code root set's static memory usage seems too small, is only "SIZE_FORMAT" bytes", G1CodeRootSet::static_mem_size()));
+#ifndef PRODUCT
+
+class G1CodeRootSetTest {
+ public:
+  static void test() {
+    {
+      G1CodeRootSet set1;
+      assert(set1.is_empty(), "Code root set must be initially empty but is not.");
 
-    set1.add((nmethod*)1);
-    assert(mgr.num_chunks_handed_out() == 1,
-           err_msg("Must have allocated and handed out one chunk, but handed out "
-                   SIZE_FORMAT" chunks", mgr.num_chunks_handed_out()));
-    assert(set1.length() == 1, err_msg("Added exactly one element, but set contains "
-                                       SIZE_FORMAT" elements", set1.length()));
+      assert(G1CodeRootSet::static_mem_size() == sizeof(void*),
+          err_msg("The code root set's static memory usage is incorrect, "SIZE_FORMAT" bytes", G1CodeRootSet::static_mem_size()));
 
-    // G1CodeRootChunk::word_size() is larger than G1CodeRootChunk::num_entries which
-    // we cannot access.
-    for (uint i = 0; i < G1CodeRootChunk::word_size() + 1; i++) {
       set1.add((nmethod*)1);
-    }
-    assert(mgr.num_chunks_handed_out() == 1,
-           err_msg("Duplicate detection must have prevented allocation of further "
-                   "chunks but allocated "SIZE_FORMAT, mgr.num_chunks_handed_out()));
-    assert(set1.length() == 1,
-           err_msg("Duplicate detection should not have increased the set size but "
-                   "is "SIZE_FORMAT, set1.length()));
-
-    size_t num_total_after_add = G1CodeRootChunk::word_size() + 1;
-    for (size_t i = 0; i < num_total_after_add - 1; i++) {
-      set1.add((nmethod*)(uintptr_t)(2 + i));
-    }
-    assert(mgr.num_chunks_handed_out() > 1,
-           "After adding more code roots, more than one additional chunk should have been handed out");
-    assert(set1.length() == num_total_after_add,
-           err_msg("After adding in total "SIZE_FORMAT" distinct code roots, they "
-                   "need to be in the set, but there are only "SIZE_FORMAT,
-                   num_total_after_add, set1.length()));
-
-    size_t num_popped = 0;
-    while (set1.pop() != NULL) {
-      num_popped++;
-    }
-    assert(num_popped == num_total_after_add,
-           err_msg("Managed to pop "SIZE_FORMAT" code roots, but only "SIZE_FORMAT" "
-                   "were added", num_popped, num_total_after_add));
-    assert(mgr.num_chunks_handed_out() == 0,
-           err_msg("After popping all elements, all chunks must have been returned "
-                   "but there are still "SIZE_FORMAT" additional", mgr.num_chunks_handed_out()));
-
-    mgr.purge_chunks(0);
-    assert(mgr.num_free_chunks() == 0,
-           err_msg("After purging everything, the free list must be empty but still "
-                   "contains "SIZE_FORMAT" chunks", mgr.num_free_chunks()));
-
-    // Add some more handed out chunks.
-    size_t i = 0;
-    while (mgr.num_chunks_handed_out() < num_chunks) {
-      set1.add((nmethod*)i);
-      i++;
-    }
+      assert(set1.length() == 1, err_msg("Added exactly one element, but set contains "
+          SIZE_FORMAT" elements", set1.length()));
 
-    {
-      // Generate chunks on the free list.
-      G1CodeRootSet set2(&mgr);
-      size_t i = 0;
-      while (mgr.num_chunks_handed_out() < (num_chunks * 2)) {
-        set2.add((nmethod*)i);
-        i++;
+      const size_t num_to_add = (size_t)G1CodeRootSet::Threshold + 1;
+
+      for (size_t i = 1; i <= num_to_add; i++) {
+        set1.add((nmethod*)1);
+      }
+      assert(set1.length() == 1,
+          err_msg("Duplicate detection should not have increased the set size but "
+              "is "SIZE_FORMAT, set1.length()));
+
+      for (size_t i = 2; i <= num_to_add; i++) {
+        set1.add((nmethod*)(uintptr_t)(i));
+      }
+      assert(set1.length() == num_to_add,
+          err_msg("After adding in total "SIZE_FORMAT" distinct code roots, they "
+              "need to be in the set, but there are only "SIZE_FORMAT,
+              num_to_add, set1.length()));
+
+      assert(CodeRootSetTable::_purge_list != NULL, "should have grown to large hashtable");
+
+      size_t num_popped = 0;
+      for (size_t i = 1; i <= num_to_add; i++) {
+        bool removed = set1.remove((nmethod*)i);
+        if (removed) {
+          num_popped += 1;
+        } else {
+          break;
+        }
       }
-      // Exit of the scope of the set2 object will call the destructor that generates
-      // num_chunks elements on the free list.
+      assert(num_popped == num_to_add,
+          err_msg("Managed to pop "SIZE_FORMAT" code roots, but only "SIZE_FORMAT" "
+              "were added", num_popped, num_to_add));
+      assert(CodeRootSetTable::_purge_list != NULL, "should have grown to large hashtable");
+
+      G1CodeRootSet::purge();
+
+      assert(CodeRootSetTable::_purge_list == NULL, "should have purged old small tables");
+
     }
 
-    assert(mgr.num_chunks_handed_out() == num_chunks,
-           err_msg("Deletion of the second set must have resulted in giving back "
-                   "those, but there are still "SIZE_FORMAT" additional handed out, expecting "
-                   SIZE_FORMAT, mgr.num_chunks_handed_out(), num_chunks));
-    assert(mgr.num_free_chunks() == num_chunks,
-           err_msg("After freeing "SIZE_FORMAT" chunks, they must be on the free list "
-                   "but there are only "SIZE_FORMAT, num_chunks, mgr.num_free_chunks()));
-
-    size_t const test_percentage = 50;
-    mgr.purge_chunks(test_percentage);
-    assert(mgr.num_chunks_handed_out() == num_chunks,
-           err_msg("Purging must not hand out chunks but there are "SIZE_FORMAT,
-                   mgr.num_chunks_handed_out()));
-    assert(mgr.num_free_chunks() == (size_t)(mgr.num_chunks_handed_out() * test_percentage / 100),
-           err_msg("Must have purged "SIZE_FORMAT" percent of "SIZE_FORMAT" chunks"
-                   "but there are "SIZE_FORMAT, test_percentage, num_chunks,
-                   mgr.num_free_chunks()));
-    // Purge the remainder of the chunks on the free list.
-    mgr.purge_chunks(0);
-    assert(mgr.num_free_chunks() == 0, "Free List must be empty");
-    assert(mgr.num_chunks_handed_out() == num_chunks,
-           err_msg("Expected to be "SIZE_FORMAT" chunks handed out from the first set "
-                   "but there are "SIZE_FORMAT, num_chunks, mgr.num_chunks_handed_out()));
-
-    // Exit of the scope of the set1 object will call the destructor that generates
-    // num_chunks additional elements on the free list.
-   }
-
-  assert(mgr.num_chunks_handed_out() == 0,
-         err_msg("Deletion of the only set must have resulted in no chunks handed "
-                 "out, but there is still "SIZE_FORMAT" handed out", mgr.num_chunks_handed_out()));
-  assert(mgr.num_free_chunks() == num_chunks,
-         err_msg("After freeing "SIZE_FORMAT" chunks, they must be on the free list "
-                 "but there are only "SIZE_FORMAT, num_chunks, mgr.num_free_chunks()));
-
-  // Restore initial state.
-  mgr.purge_chunks(0);
-  assert(mgr.num_free_chunks() == 0, "Free List must be empty");
-  assert(mgr.num_chunks_handed_out() == 0, "No additional elements must have been handed out yet");
-}
+  }
+};
 
 void TestCodeCacheRemSet_test() {
-  G1CodeRootSet::test();
+  G1CodeRootSetTest::test();
 }
+
 #endif

src/share/vm/gc_implementation/g1/g1CodeCacheRemSet.hpp

@@ -26,222 +26,64 @@
 #define SHARE_VM_GC_IMPLEMENTATION_G1_G1CODECACHEREMSET_HPP
 
 #include "memory/allocation.hpp"
-#include "memory/freeList.hpp"
-#include "runtime/globals.hpp"
 
 class CodeBlobClosure;
-
-// The elements of the G1CodeRootChunk is either:
-//  1) nmethod pointers
-//  2) nodes in an internally chained free list
-typedef union {
-  nmethod* _nmethod;
-  void*    _link;
-} NmethodOrLink;
-
-class G1CodeRootChunk : public CHeapObj<mtGC> {
- private:
-  static const int NUM_ENTRIES = 32;
- public:
-  G1CodeRootChunk*     _next;
-  G1CodeRootChunk*     _prev;
-
-  NmethodOrLink*          _top;
-  // First free position within the chunk.
-  volatile NmethodOrLink* _free;
-
-  NmethodOrLink _data[NUM_ENTRIES];
-
-  NmethodOrLink* bottom() const {
-    return (NmethodOrLink*) &(_data[0]);
-  }
-
-  NmethodOrLink* end() const {
-    return (NmethodOrLink*) &(_data[NUM_ENTRIES]);
-  }
-
-  bool is_link(NmethodOrLink* nmethod_or_link) {
-    return nmethod_or_link->_link == NULL ||
-        (bottom() <= nmethod_or_link->_link
-        && nmethod_or_link->_link < end());
-  }
-
-  bool is_nmethod(NmethodOrLink* nmethod_or_link) {
-    return !is_link(nmethod_or_link);
-  }
-
- public:
-  G1CodeRootChunk();
-  ~G1CodeRootChunk() {}
-
-  static size_t word_size() { return (size_t)(align_size_up_(sizeof(G1CodeRootChunk), HeapWordSize) / HeapWordSize); }
-
-  // FreeList "interface" methods
-
-  G1CodeRootChunk* next() const         { return _next; }
-  G1CodeRootChunk* prev() const         { return _prev; }
-  void set_next(G1CodeRootChunk* v)     { _next = v; assert(v != this, "Boom");}
-  void set_prev(G1CodeRootChunk* v)     { _prev = v; assert(v != this, "Boom");}
-  void clear_next()       { set_next(NULL); }
-  void clear_prev()       { set_prev(NULL); }
-
-  size_t size() const { return word_size(); }
-
-  void link_next(G1CodeRootChunk* ptr)  { set_next(ptr); }
-  void link_prev(G1CodeRootChunk* ptr)  { set_prev(ptr); }
-  void link_after(G1CodeRootChunk* ptr) {
-    link_next(ptr);
-    if (ptr != NULL) ptr->link_prev((G1CodeRootChunk*)this);
-  }
-
-  bool is_free()                 { return true; }
-
-  // New G1CodeRootChunk routines
-
-  void reset();
-
-  bool is_empty() const {
-    return _top == bottom();
-  }
-
-  bool is_full() const {
-    return _top == end() && _free == NULL;
-  }
-
-  bool contains(nmethod* method) {
-    NmethodOrLink* cur = bottom();
-    while (cur != _top) {
-      if (cur->_nmethod == method) return true;
-      cur++;
-    }
-    return false;
-  }
-
-  bool add(nmethod* method) {
-    if (is_full()) {
-      return false;
-    }
-
-    if (_free != NULL) {
-      // Take from internally chained free list
-      NmethodOrLink* first_free = (NmethodOrLink*)_free;
-      _free = (NmethodOrLink*)_free->_link;
-      first_free->_nmethod = method;
-    } else {
-      // Take from top.
-      _top->_nmethod = method;
-      _top++;
-    }
-
-    return true;
-  }
-
-  bool remove_lock_free(nmethod* method);
-
-  void nmethods_do(CodeBlobClosure* blk);
-
-  nmethod* pop() {
-    if (_free != NULL) {
-      // Kill the free list.
-      _free = NULL;
-    }
-
-    while (!is_empty()) {
-      _top--;
-      if (is_nmethod(_top)) {
-        return _top->_nmethod;
-      }
-    }
-
-    return NULL;
-  }
-};
-
-// Manages free chunks.
-class G1CodeRootChunkManager VALUE_OBJ_CLASS_SPEC {
- private:
-  // Global free chunk list management
-  FreeList<G1CodeRootChunk> _free_list;
-  // Total number of chunks handed out
-  size_t _num_chunks_handed_out;
-
- public:
-  G1CodeRootChunkManager();
-
-  G1CodeRootChunk* new_chunk();
-  void free_chunk(G1CodeRootChunk* chunk);
-  // Free all elements of the given list.
-  void free_all_chunks(FreeList<G1CodeRootChunk>* list);
-
-  void initialize();
-  void purge_chunks(size_t keep_ratio);
-
-  static size_t static_mem_size();
-  size_t fl_mem_size();
-
-#ifndef PRODUCT
-  size_t num_chunks_handed_out() const;
-  size_t num_free_chunks() const;
-#endif
-};
+class CodeRootSetTable;
+class HeapRegion;
+class nmethod;
 
 // Implements storage for a set of code roots.
 // All methods that modify the set are not thread-safe except if otherwise noted.
 class G1CodeRootSet VALUE_OBJ_CLASS_SPEC {
+  friend class G1CodeRootSetTest;
  private:
-  // Global default free chunk manager instance.
-  static G1CodeRootChunkManager _default_chunk_manager;
 
-  G1CodeRootChunk* new_chunk() { return _manager->new_chunk(); }
-  void free_chunk(G1CodeRootChunk* chunk) { _manager->free_chunk(chunk); }
-  // Free all elements of the given list.
-  void free_all_chunks(FreeList<G1CodeRootChunk>* list) { _manager->free_all_chunks(list); }
+  const static size_t SmallSize = 32;
+  const static size_t Threshold = 24;
+  const static size_t LargeSize = 512;
 
-  // Return the chunk that contains the given nmethod, NULL otherwise.
-  // Scans the list of chunks backwards, as this method is used to add new
-  // entries, which are typically added in bulk for a single nmethod.
-  G1CodeRootChunk* find(nmethod* method);
-  void free(G1CodeRootChunk* chunk);
+  CodeRootSetTable* _table;
+  CodeRootSetTable* load_acquire_table();
 
   size_t _length;
-  FreeList<G1CodeRootChunk> _list;
-  G1CodeRootChunkManager* _manager;
+
+  void move_to_large();
+  void allocate_small_table();
 
  public:
-  // If an instance is initialized with a chunk manager of NULL, use the global
-  // default one.
-  G1CodeRootSet(G1CodeRootChunkManager* manager = NULL);
+  G1CodeRootSet() : _table(NULL), _length(0) {}
   ~G1CodeRootSet();
 
-  static void purge_chunks(size_t keep_ratio);
+  static void purge();
 
-  static size_t free_chunks_static_mem_size();
-  static size_t free_chunks_mem_size();
+  static size_t static_mem_size();
 
-  // Search for the code blob from the recently allocated ones to find duplicates more quickly, as this
-  // method is likely to be repeatedly called with the same nmethod.
   void add(nmethod* method);
 
-  void remove_lock_free(nmethod* method);
-  nmethod* pop();
+  bool remove(nmethod* method);
 
+  // Safe to call without synchronization, but may return false negatives.
   bool contains(nmethod* method);
 
   void clear();
 
   void nmethods_do(CodeBlobClosure* blk) const;
 
-  bool is_empty() { return length() == 0; }
+  // Remove all nmethods which no longer contain pointers into our "owner" region
+  void clean(HeapRegion* owner);
+
+  bool is_empty() {
+    bool empty = length() == 0;
+    assert(empty == (_table == NULL), "is empty only if table is deallocated");
+    return empty;
+  }
 
   // Length in elements
   size_t length() const { return _length; }
 
-  // Static data memory size in bytes of this set.
-  static size_t static_mem_size();
   // Memory size in bytes taken by this set.
   size_t mem_size();
 
-  static void test() PRODUCT_RETURN;
 };
 
 #endif // SHARE_VM_GC_IMPLEMENTATION_G1_G1CODECACHEREMSET_HPP

src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp

@@ -4580,6 +4580,56 @@ class G1KlassScanClosure : public KlassClosure {
   }
 };
 
+class G1CodeBlobClosure : public CodeBlobClosure {
+  class HeapRegionGatheringOopClosure : public OopClosure {
+    G1CollectedHeap* _g1h;
+    OopClosure* _work;
+    nmethod* _nm;
+
+    template <typename T>
+    void do_oop_work(T* p) {
+      _work->do_oop(p);
+      T oop_or_narrowoop = oopDesc::load_heap_oop(p);
+      if (!oopDesc::is_null(oop_or_narrowoop)) {
+        oop o = oopDesc::decode_heap_oop_not_null(oop_or_narrowoop);
+        HeapRegion* hr = _g1h->heap_region_containing_raw(o);
+        assert(!_g1h->obj_in_cs(o) || hr->rem_set()->strong_code_roots_list_contains(_nm), "if o still in CS then evacuation failed and nm must already be in the remset");
+        hr->add_strong_code_root(_nm);
+      }
+    }
+
+  public:
+    HeapRegionGatheringOopClosure(OopClosure* oc) : _g1h(G1CollectedHeap::heap()), _work(oc), _nm(NULL) {}
+
+    void do_oop(oop* o) {
+      do_oop_work(o);
+    }
+
+    void do_oop(narrowOop* o) {
+      do_oop_work(o);
+    }
+
+    void set_nm(nmethod* nm) {
+      _nm = nm;
+    }
+  };
+
+  HeapRegionGatheringOopClosure _oc;
+public:
+  G1CodeBlobClosure(OopClosure* oc) : _oc(oc) {}
+
+  void do_code_blob(CodeBlob* cb) {
+    nmethod* nm = cb->as_nmethod_or_null();
+    if (nm != NULL) {
+      if (!nm->test_set_oops_do_mark()) {
+        _oc.set_nm(nm);
+        nm->oops_do(&_oc);
+        nm->fix_oop_relocations();
+      }
+    }
+  }
+};
+
 class G1ParTask : public AbstractGangTask {
 protected:
   G1CollectedHeap*       _g1h;
@@ -4648,22 +4698,6 @@ public:
     }
   };
 
-  class G1CodeBlobClosure: public CodeBlobClosure {
-    OopClosure* _f;
-
-   public:
-    G1CodeBlobClosure(OopClosure* f) : _f(f) {}
-    void do_code_blob(CodeBlob* blob) {
-      nmethod* that = blob->as_nmethod_or_null();
-      if (that != NULL) {
-        if (!that->test_set_oops_do_mark()) {
-          that->oops_do(_f);
-          that->fix_oop_relocations();
-        }
-      }
-    }
-  };
-
   void work(uint worker_id) {
     if (worker_id >= _n_workers) return;  // no work needed this round
 
@@ -4854,7 +4888,7 @@ g1_process_roots(OopClosure* scan_non_heap_roots,
   g1_policy()->phase_times()->record_satb_filtering_time(worker_i, satb_filtering_ms);
 
   // Now scan the complement of the collection set.
-  MarkingCodeBlobClosure scavenge_cs_nmethods(scan_non_heap_weak_roots, CodeBlobToOopClosure::FixRelocations);
+  G1CodeBlobClosure scavenge_cs_nmethods(scan_non_heap_weak_roots);
 
   g1_rem_set()->oops_into_collection_set_do(scan_rs, &scavenge_cs_nmethods, worker_i);
 
@@ -5901,12 +5935,6 @@ void G1CollectedHeap::evacuate_collection_set(EvacuationInfo& evacuation_info) {
   hot_card_cache->reset_hot_cache();
   hot_card_cache->set_use_cache(true);
 
-  // Migrate the strong code roots attached to each region in
-  // the collection set. Ideally we would like to do this
-  // after we have finished the scanning/evacuation of the
-  // strong code roots for a particular heap region.
-  migrate_strong_code_roots();
-
   purge_code_root_memory();
 
   if (g1_policy()->during_initial_mark_pause()) {
@@ -6902,13 +6930,8 @@ class RegisterNMethodOopClosure: public OopClosure {
                      " starting at "HR_FORMAT,
                      _nm, HR_FORMAT_PARAMS(hr), HR_FORMAT_PARAMS(hr->humongous_start_region())));
 
-      // HeapRegion::add_strong_code_root() avoids adding duplicate
-      // entries but having duplicates is  OK since we "mark" nmethods
-      // as visited when we scan the strong code root lists during the GC.
-      hr->add_strong_code_root(_nm);
-      assert(hr->rem_set()->strong_code_roots_list_contains(_nm),
-             err_msg("failed to add code root "PTR_FORMAT" to remembered set of region "HR_FORMAT,
-                     _nm, HR_FORMAT_PARAMS(hr)));
+      // HeapRegion::add_strong_code_root_locked() avoids adding duplicate entries.
+      hr->add_strong_code_root_locked(_nm);
     }
   }
 
@@ -6935,9 +6958,6 @@ class UnregisterNMethodOopClosure: public OopClosure {
                      _nm, HR_FORMAT_PARAMS(hr), HR_FORMAT_PARAMS(hr->humongous_start_region())));
 
       hr->remove_strong_code_root(_nm);
-      assert(!hr->rem_set()->strong_code_roots_list_contains(_nm),
-             err_msg("failed to remove code root "PTR_FORMAT" of region "HR_FORMAT,
-                     _nm, HR_FORMAT_PARAMS(hr)));
     }
   }
 
@@ -6965,28 +6985,9 @@ void G1CollectedHeap::unregister_nmethod(nmethod* nm) {
   nm->oops_do(&reg_cl, true);
 }
 
-class MigrateCodeRootsHeapRegionClosure: public HeapRegionClosure {
-public:
-  bool doHeapRegion(HeapRegion *hr) {
-    assert(!hr->isHumongous(),
-           err_msg("humongous region "HR_FORMAT" should not have been added to collection set",
-                   HR_FORMAT_PARAMS(hr)));
-    hr->migrate_strong_code_roots();
-    return false;
-  }
-};
-
-void G1CollectedHeap::migrate_strong_code_roots() {
-  MigrateCodeRootsHeapRegionClosure cl;
-  double migrate_start = os::elapsedTime();
-  collection_set_iterate(&cl);
-  double migration_time_ms = (os::elapsedTime() - migrate_start) * 1000.0;
-  g1_policy()->phase_times()->record_strong_code_root_migration_time(migration_time_ms);
-}
-
 void G1CollectedHeap::purge_code_root_memory() {
   double purge_start = os::elapsedTime();
-  G1CodeRootSet::purge_chunks(G1CodeRootsChunkCacheKeepPercent);
+  G1CodeRootSet::purge();
   double purge_time_ms = (os::elapsedTime() - purge_start) * 1000.0;
   g1_policy()->phase_times()->record_strong_code_root_purge_time(purge_time_ms);
 }

src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp

-/*
+  /*
  * Copyright (c) 2001, 2014, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -1633,12 +1633,6 @@ public:
   // Unregister the given nmethod from the G1 heap
   virtual void unregister_nmethod(nmethod* nm);
 
-  // Migrate the nmethods in the code root lists of the regions
-  // in the collection set to regions in to-space. In the event
-  // of an evacuation failure, nmethods that reference objects
-  // that were not successfullly evacuated are not migrated.
-  void migrate_strong_code_roots();
-
   // Free up superfluous code root memory.
   void purge_code_root_memory();
 

src/share/vm/gc_implementation/g1/g1EvacFailure.hpp

@@ -217,6 +217,8 @@ public:
         _update_rset_cl->set_region(hr);
         hr->object_iterate(&rspc);
 
+        hr->rem_set()->clean_strong_code_roots(hr);
+
         hr->note_self_forwarding_removal_end(during_initial_mark,
                                              during_conc_mark,
                                              rspc.marked_bytes());

src/share/vm/gc_implementation/g1/g1GCPhaseTimes.cpp

@@ -274,9 +274,6 @@ double G1GCPhaseTimes::accounted_time_ms() {
     // Now subtract the time taken to fix up roots in generated code
     misc_time_ms += _cur_collection_code_root_fixup_time_ms;
 
-    // Strong code root migration time
-    misc_time_ms += _cur_strong_code_root_migration_time_ms;
-
     // Strong code root purge time
     misc_time_ms += _cur_strong_code_root_purge_time_ms;
 
@@ -327,7 +324,6 @@ void G1GCPhaseTimes::print(double pause_time_sec) {
     _last_obj_copy_times_ms.print(1, "Object Copy (ms)");
   }
   print_stats(1, "Code Root Fixup", _cur_collection_code_root_fixup_time_ms);
-  print_stats(1, "Code Root Migration", _cur_strong_code_root_migration_time_ms);
   print_stats(1, "Code Root Purge", _cur_strong_code_root_purge_time_ms);
   if (G1StringDedup::is_enabled()) {
     print_stats(1, "String Dedup Fixup", _cur_string_dedup_fixup_time_ms, _active_gc_threads);

src/share/vm/gc_implementation/g1/g1GCPhaseTimes.hpp

@@ -129,7 +129,6 @@ class G1GCPhaseTimes : public CHeapObj<mtGC> {
 
   double _cur_collection_par_time_ms;
   double _cur_collection_code_root_fixup_time_ms;
-  double _cur_strong_code_root_migration_time_ms;
   double _cur_strong_code_root_purge_time_ms;
 
   double _cur_evac_fail_recalc_used;
@@ -233,10 +232,6 @@ class G1GCPhaseTimes : public CHeapObj<mtGC> {
     _cur_collection_code_root_fixup_time_ms = ms;
   }
 
-  void record_strong_code_root_migration_time(double ms) {
-    _cur_strong_code_root_migration_time_ms = ms;
-  }
-
   void record_strong_code_root_purge_time(double ms) {
     _cur_strong_code_root_purge_time_ms = ms;
   }

src/share/vm/gc_implementation/g1/g1RemSet.cpp

@@ -109,7 +109,7 @@ class ScanRSClosure : public HeapRegionClosure {
   G1CollectedHeap* _g1h;
 
   OopsInHeapRegionClosure* _oc;
-  CodeBlobToOopClosure* _code_root_cl;
+  CodeBlobClosure* _code_root_cl;
 
   G1BlockOffsetSharedArray* _bot_shared;
   G1SATBCardTableModRefBS *_ct_bs;
@@ -121,7 +121,7 @@ class ScanRSClosure : public HeapRegionClosure {
 
 public:
   ScanRSClosure(OopsInHeapRegionClosure* oc,
-                CodeBlobToOopClosure* code_root_cl,
+                CodeBlobClosure* code_root_cl,
                 uint worker_i) :
     _oc(oc),
     _code_root_cl(code_root_cl),
@@ -241,7 +241,7 @@ public:
 };
 
 void G1RemSet::scanRS(OopsInHeapRegionClosure* oc,
-                      CodeBlobToOopClosure* code_root_cl,
+                      CodeBlobClosure* code_root_cl,
                       uint worker_i) {
   double rs_time_start = os::elapsedTime();
   HeapRegion *startRegion = _g1->start_cset_region_for_worker(worker_i);
@@ -320,7 +320,7 @@ void G1RemSet::cleanupHRRS() {
 }
 
 void G1RemSet::oops_into_collection_set_do(OopsInHeapRegionClosure* oc,
-                                           CodeBlobToOopClosure* code_root_cl,
+                                           CodeBlobClosure* code_root_cl,
                                            uint worker_i) {
 #if CARD_REPEAT_HISTO
   ct_freq_update_histo_and_reset();

src/share/vm/gc_implementation/g1/g1RemSet.hpp

@@ -96,7 +96,7 @@ public:
   // the "i" passed to the calling thread's work(i) function.
   // In the sequential case this param will be ignored.
   void oops_into_collection_set_do(OopsInHeapRegionClosure* blk,
-                                   CodeBlobToOopClosure* code_root_cl,
+                                   CodeBlobClosure* code_root_cl,
                                    uint worker_i);
 
   // Prepare for and cleanup after an oops_into_collection_set_do
@@ -108,7 +108,7 @@ public:
   void cleanup_after_oops_into_collection_set_do();
 
   void scanRS(OopsInHeapRegionClosure* oc,
-              CodeBlobToOopClosure* code_root_cl,
+              CodeBlobClosure* code_root_cl,
               uint worker_i);
 
   void updateRS(DirtyCardQueue* into_cset_dcq, uint worker_i);

src/share/vm/gc_implementation/g1/g1RemSetSummary.cpp

@@ -253,6 +253,7 @@ public:
     size_t occupied_cards = hrrs->occupied();
     size_t code_root_mem_sz = hrrs->strong_code_roots_mem_size();
     if (code_root_mem_sz > max_code_root_mem_sz()) {
+      _max_code_root_mem_sz = code_root_mem_sz;
       _max_code_root_mem_sz_region = r;
     }
     size_t code_root_elems = hrrs->strong_code_roots_list_length();

src/share/vm/gc_implementation/g1/g1_globals.hpp

@@ -285,10 +285,6 @@
   product(uintx, G1MixedGCCountTarget, 8,                                   \
           "The target number of mixed GCs after a marking cycle.")          \
                                                                             \
-  experimental(uintx, G1CodeRootsChunkCacheKeepPercent, 10,                 \
-          "The amount of code root chunks that should be kept at most "     \
-          "as percentage of already allocated.")                            \
-                                                                            \
   experimental(bool, G1ReclaimDeadHumongousObjectsAtYoungGC, true,          \
           "Try to reclaim dead large objects at every young GC.")           \
                                                                             \

src/share/vm/gc_implementation/g1/heapRegion.cpp

@@ -549,19 +549,15 @@ void HeapRegion::add_strong_code_root(nmethod* nm) {
   hrrs->add_strong_code_root(nm);
 }
 
-void HeapRegion::remove_strong_code_root(nmethod* nm) {
+void HeapRegion::add_strong_code_root_locked(nmethod* nm) {
+  assert_locked_or_safepoint(CodeCache_lock);
   HeapRegionRemSet* hrrs = rem_set();
-  hrrs->remove_strong_code_root(nm);
+  hrrs->add_strong_code_root_locked(nm);
 }
 
-void HeapRegion::migrate_strong_code_roots() {
-  assert(in_collection_set(), "only collection set regions");
-  assert(!isHumongous(),
-          err_msg("humongous region "HR_FORMAT" should not have been added to collection set",
-                  HR_FORMAT_PARAMS(this)));
-
+void HeapRegion::remove_strong_code_root(nmethod* nm) {
   HeapRegionRemSet* hrrs = rem_set();
-  hrrs->migrate_strong_code_roots();
+  hrrs->remove_strong_code_root(nm);
 }
 
 void HeapRegion::strong_code_roots_do(CodeBlobClosure* blk) const {

src/share/vm/gc_implementation/g1/heapRegion.hpp

@@ -772,14 +772,9 @@ class HeapRegion: public G1OffsetTableContigSpace {
   // Routines for managing a list of code roots (attached to the
   // this region's RSet) that point into this heap region.
   void add_strong_code_root(nmethod* nm);
+  void add_strong_code_root_locked(nmethod* nm);
   void remove_strong_code_root(nmethod* nm);
 
-  // During a collection, migrate the successfully evacuated
-  // strong code roots that referenced into this region to the
-  // new regions that they now point into. Unsuccessfully
-  // evacuated code roots are not migrated.
-  void migrate_strong_code_roots();
-
   // Applies blk->do_code_blob() to each of the entries in
   // the strong code roots list for this region
   void strong_code_roots_do(CodeBlobClosure* blk) const;

src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp

@@ -923,8 +923,24 @@ void HeapRegionRemSet::scrub(CardTableModRefBS* ctbs,
 }
 
 // Code roots support
+//
+// The code root set is protected by two separate locking schemes
+// When at safepoint the per-hrrs lock must be held during modifications
+// except when doing a full gc.
+// When not at safepoint the CodeCache_lock must be held during modifications.
+// When concurrent readers access the contains() function
+// (during the evacuation phase) no removals are allowed.
 
 void HeapRegionRemSet::add_strong_code_root(nmethod* nm) {
+  assert(nm != NULL, "sanity");
+  // Optimistic unlocked contains-check
+  if (!_code_roots.contains(nm)) {
+    MutexLockerEx ml(&_m, Mutex::_no_safepoint_check_flag);
+    add_strong_code_root_locked(nm);
+  }
+}
+
+void HeapRegionRemSet::add_strong_code_root_locked(nmethod* nm) {
   assert(nm != NULL, "sanity");
   _code_roots.add(nm);
 }
@@ -933,98 +949,21 @@ void HeapRegionRemSet::remove_strong_code_root(nmethod* nm) {
   assert(nm != NULL, "sanity");
   assert_locked_or_safepoint(CodeCache_lock);
 
-  _code_roots.remove_lock_free(nm);
+  MutexLockerEx ml(CodeCache_lock->owned_by_self() ? NULL : &_m, Mutex::_no_safepoint_check_flag);
+  _code_roots.remove(nm);
 
   // Check that there were no duplicates
   guarantee(!_code_roots.contains(nm), "duplicate entry found");
 }
 
-class NMethodMigrationOopClosure : public OopClosure {
-  G1CollectedHeap* _g1h;
-  HeapRegion* _from;
-  nmethod* _nm;
-
-  uint _num_self_forwarded;
-
-  template <class T> void do_oop_work(T* p) {
-    T heap_oop = oopDesc::load_heap_oop(p);
-    if (!oopDesc::is_null(heap_oop)) {
-      oop obj = oopDesc::decode_heap_oop_not_null(heap_oop);
-      if (_from->is_in(obj)) {
-        // Reference still points into the source region.
-        // Since roots are immediately evacuated this means that
-        // we must have self forwarded the object
-        assert(obj->is_forwarded(),
-               err_msg("code roots should be immediately evacuated. "
-                       "Ref: "PTR_FORMAT", "
-                       "Obj: "PTR_FORMAT", "
-                       "Region: "HR_FORMAT,
-                       p, (void*) obj, HR_FORMAT_PARAMS(_from)));
-        assert(obj->forwardee() == obj,
-               err_msg("not self forwarded? obj = "PTR_FORMAT, (void*)obj));
-
-        // The object has been self forwarded.
-        // Note, if we're during an initial mark pause, there is
-        // no need to explicitly mark object. It will be marked
-        // during the regular evacuation failure handling code.
-        _num_self_forwarded++;
-      } else {
-        // The reference points into a promotion or to-space region
-        HeapRegion* to = _g1h->heap_region_containing(obj);
-        to->rem_set()->add_strong_code_root(_nm);
-      }
-    }
-  }
-
-public:
-  NMethodMigrationOopClosure(G1CollectedHeap* g1h, HeapRegion* from, nmethod* nm):
-    _g1h(g1h), _from(from), _nm(nm), _num_self_forwarded(0) {}
-
-  void do_oop(narrowOop* p) { do_oop_work(p); }
-  void do_oop(oop* p)       { do_oop_work(p); }
-
-  uint retain() { return _num_self_forwarded > 0; }
-};
-
-void HeapRegionRemSet::migrate_strong_code_roots() {
-  assert(hr()->in_collection_set(), "only collection set regions");
-  assert(!hr()->isHumongous(),
-         err_msg("humongous region "HR_FORMAT" should not have been added to the collection set",
-                 HR_FORMAT_PARAMS(hr())));
-
-  ResourceMark rm;
-
-  // List of code blobs to retain for this region
-  GrowableArray<nmethod*> to_be_retained(10);
-  G1CollectedHeap* g1h = G1CollectedHeap::heap();
-
-  while (!_code_roots.is_empty()) {
-    nmethod *nm = _code_roots.pop();
-    if (nm != NULL) {
-      NMethodMigrationOopClosure oop_cl(g1h, hr(), nm);
-      nm->oops_do(&oop_cl);
-      if (oop_cl.retain()) {
-        to_be_retained.push(nm);
-      }
-    }
-  }
-
-  // Now push any code roots we need to retain
-  assert(to_be_retained.is_empty() || hr()->evacuation_failed(),
-         "Retained nmethod list must be empty or "
-         "evacuation of this region failed");
-
-  while (to_be_retained.is_nonempty()) {
-    nmethod* nm = to_be_retained.pop();
-    assert(nm != NULL, "sanity");
-    add_strong_code_root(nm);
-  }
-}
-
 void HeapRegionRemSet::strong_code_roots_do(CodeBlobClosure* blk) const {
   _code_roots.nmethods_do(blk);
 }
 
+void HeapRegionRemSet::clean_strong_code_roots(HeapRegion* hr) {
+  _code_roots.clean(hr);
+}
+
 size_t HeapRegionRemSet::strong_code_roots_mem_size() {
   return _code_roots.mem_size();
 }

src/share/vm/gc_implementation/g1/heapRegionRemSet.hpp

@@ -353,13 +353,13 @@ public:
   // Returns the memory occupancy of all static data structures associated
   // with remembered sets.
   static size_t static_mem_size() {
-    return OtherRegionsTable::static_mem_size() + G1CodeRootSet::free_chunks_static_mem_size();
+    return OtherRegionsTable::static_mem_size() + G1CodeRootSet::static_mem_size();
   }
 
   // Returns the memory occupancy of all free_list data structures associated
   // with remembered sets.
   static size_t fl_mem_size() {
-    return OtherRegionsTable::fl_mem_size() + G1CodeRootSet::free_chunks_mem_size();
+    return OtherRegionsTable::fl_mem_size();
   }
 
   bool contains_reference(OopOrNarrowOopStar from) const {
@@ -369,18 +369,15 @@ public:
   // Routines for managing the list of code roots that point into
   // the heap region that owns this RSet.
   void add_strong_code_root(nmethod* nm);
+  void add_strong_code_root_locked(nmethod* nm);
   void remove_strong_code_root(nmethod* nm);
 
-  // During a collection, migrate the successfully evacuated strong
-  // code roots that referenced into the region that owns this RSet
-  // to the RSets of the new regions that they now point into.
-  // Unsuccessfully evacuated code roots are not migrated.
-  void migrate_strong_code_roots();
-
   // Applies blk->do_code_blob() to each of the entries in
   // the strong code roots list
   void strong_code_roots_do(CodeBlobClosure* blk) const;
 
+  void clean_strong_code_roots(HeapRegion* hr);
+
   // Returns the number of elements in the strong code roots list
   size_t strong_code_roots_list_length() const {
     return _code_roots.length();

src/share/vm/memory/freeList.cpp

@@ -34,7 +34,6 @@
 
 #if INCLUDE_ALL_GCS
 #include "gc_implementation/concurrentMarkSweep/freeChunk.hpp"
-#include "gc_implementation/g1/g1CodeCacheRemSet.hpp"
 #endif // INCLUDE_ALL_GCS
 
 // Free list.  A FreeList is used to access a linked list of chunks
@@ -333,5 +332,4 @@ template class FreeList<Metablock>;
 template class FreeList<Metachunk>;
 #if INCLUDE_ALL_GCS
 template class FreeList<FreeChunk>;
-template class FreeList<G1CodeRootChunk>;
 #endif // INCLUDE_ALL_GCS

src/share/vm/utilities/hashtable.cpp

@@ -353,6 +353,11 @@ template <MEMFLAGS F> void BasicHashtable<F>::verify_lookup_length(double load)
 
 #endif
 // Explicitly instantiate these types
+#if INCLUDE_ALL_GCS
+template class Hashtable<nmethod*, mtGC>;
+template class HashtableEntry<nmethod*, mtGC>;
+template class BasicHashtable<mtGC>;
+#endif
 template class Hashtable<ConstantPool*, mtClass>;
 template class RehashableHashtable<Symbol*, mtSymbol>;
 template class RehashableHashtable<oopDesc*, mtSymbol>;