Merge

hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp

@@ -273,7 +273,7 @@ void G1CMRootRegions::prepare_for_scan() {
 
   // Currently, only survivors can be root regions.
   _claimed_survivor_index = 0;
-  _scan_in_progress = true;
+  _scan_in_progress = _survivors->regions()->is_nonempty();
   _should_abort = false;
 }
 
@@ -294,6 +294,10 @@ HeapRegion* G1CMRootRegions::claim_next() {
   return NULL;
 }
 
+uint G1CMRootRegions::num_root_regions() const {
+  return (uint)_survivors->regions()->length();
+}
+
 void G1CMRootRegions::notify_scan_done() {
   MutexLockerEx x(RootRegionScan_lock, Mutex::_no_safepoint_check_flag);
   _scan_in_progress = false;
@@ -912,15 +916,16 @@ uint G1ConcurrentMark::calc_parallel_marking_threads() {
     n_conc_workers = max_parallel_marking_threads();
   } else {
     n_conc_workers =
-      AdaptiveSizePolicy::calc_default_active_workers(
+      AdaptiveSizePolicy::calc_default_active_workers(max_parallel_marking_threads(),
-                                   max_parallel_marking_threads(),
+                                                      1, /* Minimum workers */
-                                   1, /* Minimum workers */
+                                                      parallel_marking_threads(),
-                                   parallel_marking_threads(),
+                                                      Threads::number_of_non_daemon_threads());
-                                   Threads::number_of_non_daemon_threads());
     // Don't scale down "n_conc_workers" by scale_parallel_threads() because
     // that scaling has already gone into "_max_parallel_marking_threads".
   }
-  assert(n_conc_workers > 0, "Always need at least 1");
+  assert(n_conc_workers > 0 && n_conc_workers <= max_parallel_marking_threads(),
+         "Calculated number of workers must be larger than zero and at most the maximum %u, but is %u",
+         max_parallel_marking_threads(), n_conc_workers);
   return n_conc_workers;
 }
 
@@ -947,7 +952,7 @@ private:
 
 public:
   G1CMRootRegionScanTask(G1ConcurrentMark* cm) :
-    AbstractGangTask("Root Region Scan"), _cm(cm) { }
+    AbstractGangTask("G1 Root Region Scan"), _cm(cm) { }
 
   void work(uint worker_id) {
     assert(Thread::current()->is_ConcurrentGC_thread(),
@@ -969,14 +974,17 @@ void G1ConcurrentMark::scan_root_regions() {
   if (root_regions()->scan_in_progress()) {
     assert(!has_aborted(), "Aborting before root region scanning is finished not supported.");
 
-    _parallel_marking_threads = calc_parallel_marking_threads();
+    _parallel_marking_threads = MIN2(calc_parallel_marking_threads(),
+                                     // We distribute work on a per-region basis, so starting
+                                     // more threads than that is useless.
+                                     root_regions()->num_root_regions());
     assert(parallel_marking_threads() <= max_parallel_marking_threads(),
            "Maximum number of marking threads exceeded");
-    uint active_workers = MAX2(1U, parallel_marking_threads());
 
     G1CMRootRegionScanTask task(this);
-    _parallel_workers->set_active_workers(active_workers);
+    log_debug(gc, ergo)("Running %s using %u workers for %u work units.",
-    _parallel_workers->run_task(&task);
+                        task.name(), _parallel_marking_threads, root_regions()->num_root_regions());
+    _parallel_workers->run_task(&task, _parallel_marking_threads);
 
     // It's possible that has_aborted() is true here without actually
     // aborting the survivor scan earlier. This is OK as it's

hotspot/src/share/vm/gc/g1/g1ConcurrentMark.hpp

@@ -248,6 +248,9 @@ public:
   // all have been claimed.
   HeapRegion* claim_next();
 
+  // The number of root regions to scan.
+  uint num_root_regions() const;
+
   void cancel_scan();
 
   // Flag that we're done with root region scanning and notify anyone

hotspot/src/share/vm/gc/g1/heapRegionRemSet.cpp

@@ -103,7 +103,7 @@ protected:
       CardIdx_t from_card = (CardIdx_t)
           hw_offset >> (CardTableModRefBS::card_shift - LogHeapWordSize);
 
-      assert(0 <= from_card && (size_t)from_card < HeapRegion::CardsPerRegion,
+      assert((size_t)from_card < HeapRegion::CardsPerRegion,
              "Must be in range.");
       add_card_work(from_card, par);
     }
@@ -386,7 +386,7 @@ void OtherRegionsTable::add_reference(OopOrNarrowOopStar from, uint tid) {
         uintptr_t(from_hr->bottom())
           >> CardTableModRefBS::card_shift;
       CardIdx_t card_index = from_card - from_hr_bot_card_index;
-      assert(0 <= card_index && (size_t)card_index < HeapRegion::CardsPerRegion,
+      assert((size_t)card_index < HeapRegion::CardsPerRegion,
              "Must be in range.");
       if (G1HRRSUseSparseTable &&
           _sparse_table.add_card(from_hrm_ind, card_index)) {
@@ -421,11 +421,9 @@ void OtherRegionsTable::add_reference(OopOrNarrowOopStar from, uint tid) {
         // Transfer from sparse to fine-grain.
         SparsePRTEntry *sprt_entry = _sparse_table.get_entry(from_hrm_ind);
         assert(sprt_entry != NULL, "There should have been an entry");
-        for (int i = 0; i < SparsePRTEntry::cards_num(); i++) {
+        for (int i = 0; i < sprt_entry->num_valid_cards(); i++) {
           CardIdx_t c = sprt_entry->card(i);
-          if (c != SparsePRTEntry::NullEntry) {
+          prt->add_card(c);
-            prt->add_card(c);
-          }
         }
         // Now we can delete the sparse entry.
         bool res = _sparse_table.delete_entry(from_hrm_ind);
@@ -680,7 +678,7 @@ bool OtherRegionsTable::contains_reference_locked(OopOrNarrowOopStar from) const
       uintptr_t(hr->bottom()) >> CardTableModRefBS::card_shift;
     assert(from_card >= hr_bot_card_index, "Inv");
     CardIdx_t card_index = from_card - hr_bot_card_index;
-    assert(0 <= card_index && (size_t)card_index < HeapRegion::CardsPerRegion,
+    assert((size_t)card_index < HeapRegion::CardsPerRegion,
            "Must be in range.");
     return _sparse_table.contains_card(hr_ind, card_index);
   }

hotspot/src/share/vm/gc/g1/sparsePRT.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,6 +24,7 @@
 
 #include "precompiled.hpp"
 #include "gc/g1/heapRegion.hpp"
+#include "gc/g1/heapRegionBounds.inline.hpp"
 #include "gc/g1/heapRegionRemSet.hpp"
 #include "gc/g1/sparsePRT.hpp"
 #include "gc/shared/cardTableModRefBS.hpp"
@@ -32,57 +33,69 @@
 #include "runtime/atomic.inline.hpp"
 #include "runtime/mutexLocker.hpp"
 
-void SparsePRTEntry::init(RegionIdx_t region_ind) {
+// Check that the size of the SparsePRTEntry is evenly divisible by the maximum
-  _region_ind = region_ind;
+// member type to avoid SIGBUS when accessing them.
-  _next_index = NullEntry;
+STATIC_ASSERT(sizeof(SparsePRTEntry) % sizeof(int) == 0);
 
-  for (int i = 0; i < cards_num(); i++) {
+void SparsePRTEntry::init(RegionIdx_t region_ind) {
-    _cards[i] = NullEntry;
+  // Check that the card array element type can represent all cards in the region.
-  }
+  // Choose a large SparsePRTEntry::card_elem_t (e.g. CardIdx_t) if required.
+  assert(((size_t)1 << (sizeof(SparsePRTEntry::card_elem_t) * BitsPerByte)) *
+         G1SATBCardTableModRefBS::card_size >= HeapRegionBounds::max_size(), "precondition");
+  assert(G1RSetSparseRegionEntries > 0, "precondition");
+  _region_ind = region_ind;
+  _next_index = RSHashTable::NullEntry;
+  _next_null = 0;
 }
 
 bool SparsePRTEntry::contains_card(CardIdx_t card_index) const {
-  for (int i = 0; i < cards_num(); i++) {
+  for (int i = 0; i < num_valid_cards(); i++) {
-    if (_cards[i] == card_index) return true;
+    if (card(i) == card_index) {
+      return true;
+    }
   }
   return false;
 }
 
-int SparsePRTEntry::num_valid_cards() const {
-  int sum = 0;
-  for (int i = 0; i < cards_num(); i++) {
-    sum += (_cards[i] != NullEntry);
-  }
-  return sum;
-}
-
 SparsePRTEntry::AddCardResult SparsePRTEntry::add_card(CardIdx_t card_index) {
-  for (int i = 0; i < cards_num(); i++) {
+  for (int i = 0; i < num_valid_cards(); i++) {
-    CardIdx_t c = _cards[i];
+    if (card(i) == card_index) {
-    if (c == card_index) return found;
+      return found;
-    if (c == NullEntry) { _cards[i] = card_index; return added; }
+    }
   }
+  if (num_valid_cards() < cards_num() - 1) {
+    _cards[_next_null] = (card_elem_t)card_index;
+    _next_null++;
+    return added;
+   }
   // Otherwise, we're full.
   return overflow;
 }
 
-void SparsePRTEntry::copy_cards(CardIdx_t* cards) const {
+void SparsePRTEntry::copy_cards(card_elem_t* cards) const {
-  memcpy(cards, _cards, cards_num() * sizeof(CardIdx_t));
+  memcpy(cards, _cards, cards_num() * sizeof(card_elem_t));
 }
 
 void SparsePRTEntry::copy_cards(SparsePRTEntry* e) const {
-  copy_cards(&e->_cards[0]);
+  copy_cards(e->_cards);
+  assert(_next_null >= 0, "invariant");
+  assert(_next_null <= cards_num(), "invariant");
+  e->_next_null = _next_null;
 }
 
 // ----------------------------------------------------------------------
 
+float RSHashTable::TableOccupancyFactor = 0.5f;
+
 RSHashTable::RSHashTable(size_t capacity) :
   _capacity(capacity), _capacity_mask(capacity-1),
   _occupied_entries(0), _occupied_cards(0),
-  _entries((SparsePRTEntry*)NEW_C_HEAP_ARRAY(char, SparsePRTEntry::size() * capacity, mtGC)),
+  _entries(NULL),
   _buckets(NEW_C_HEAP_ARRAY(int, capacity, mtGC)),
   _free_list(NullEntry), _free_region(0)
 {
+  _num_entries = (capacity * TableOccupancyFactor) + 1;
+  _entries = (SparsePRTEntry*)NEW_C_HEAP_ARRAY(char, _num_entries * SparsePRTEntry::size(), mtGC);
   clear();
 }
 
@@ -107,7 +120,7 @@ void RSHashTable::clear() {
                 "_capacity too large");
 
   // This will put -1 == NullEntry in the key field of all entries.
-  memset(_entries, NullEntry, _capacity * SparsePRTEntry::size());
+  memset(_entries, NullEntry, _num_entries * SparsePRTEntry::size());
   memset(_buckets, NullEntry, _capacity * sizeof(int));
   _free_list = NullEntry;
   _free_region = 0;
@@ -123,16 +136,6 @@ bool RSHashTable::add_card(RegionIdx_t region_ind, CardIdx_t card_index) {
   return res != SparsePRTEntry::overflow;
 }
 
-bool RSHashTable::get_cards(RegionIdx_t region_ind, CardIdx_t* cards) {
-  SparsePRTEntry* entry = get_entry(region_ind);
-  if (entry == NULL) {
-    return false;
-  }
-  // Otherwise...
-  entry->copy_cards(cards);
-  return true;
-}
-
 SparsePRTEntry* RSHashTable::get_entry(RegionIdx_t region_ind) const {
   int ind = (int) (region_ind & capacity_mask());
   int cur_ind = _buckets[ind];
@@ -174,7 +177,6 @@ RSHashTable::entry_for_region_ind_create(RegionIdx_t region_ind) {
   SparsePRTEntry* res = get_entry(region_ind);
   if (res == NULL) {
     int new_ind = alloc_entry();
-    assert(0 <= new_ind && (size_t)new_ind < capacity(), "There should be room.");
     res = entry(new_ind);
     res->init(region_ind);
     // Insert at front.
@@ -192,7 +194,7 @@ int RSHashTable::alloc_entry() {
     res = _free_list;
     _free_list = entry(res)->next_index();
     return res;
-  } else if ((size_t) _free_region+1 < capacity()) {
+  } else if ((size_t)_free_region < _num_entries) {
     res = _free_region;
     _free_region++;
     return res;
@@ -215,17 +217,16 @@ void RSHashTable::add_entry(SparsePRTEntry* e) {
 }
 
 CardIdx_t RSHashTableIter::find_first_card_in_list() {
-  CardIdx_t res;
   while (_bl_ind != RSHashTable::NullEntry) {
-    res = _rsht->entry(_bl_ind)->card(0);
+    SparsePRTEntry* sparse_entry = _rsht->entry(_bl_ind);
-    if (res != SparsePRTEntry::NullEntry) {
+    if (sparse_entry->num_valid_cards() > 0) {
-      return res;
+      return sparse_entry->card(0);
     } else {
-      _bl_ind = _rsht->entry(_bl_ind)->next_index();
+      _bl_ind = sparse_entry->next_index();
     }
   }
   // Otherwise, none found:
-  return SparsePRTEntry::NullEntry;
+  return NoCardFound;
 }
 
 size_t RSHashTableIter::compute_card_ind(CardIdx_t ci) {
@@ -234,20 +235,22 @@ size_t RSHashTableIter::compute_card_ind(CardIdx_t ci) {
 
 bool RSHashTableIter::has_next(size_t& card_index) {
   _card_ind++;
-  CardIdx_t ci;
+  if (_bl_ind >= 0) {
-  if (_card_ind < SparsePRTEntry::cards_num() &&
+    SparsePRTEntry* e = _rsht->entry(_bl_ind);
-      ((ci = _rsht->entry(_bl_ind)->card(_card_ind)) !=
+    if (_card_ind < e->num_valid_cards()) {
-       SparsePRTEntry::NullEntry)) {
+      CardIdx_t ci = e->card(_card_ind);
-    card_index = compute_card_ind(ci);
+      card_index = compute_card_ind(ci);
-    return true;
+      return true;
+    }
   }
+
   // Otherwise, must find the next valid entry.
   _card_ind = 0;
 
   if (_bl_ind != RSHashTable::NullEntry) {
       _bl_ind = _rsht->entry(_bl_ind)->next_index();
-      ci = find_first_card_in_list();
+      CardIdx_t ci = find_first_card_in_list();
-      if (ci != SparsePRTEntry::NullEntry) {
+      if (ci != NoCardFound) {
         card_index = compute_card_ind(ci);
         return true;
       }
@@ -256,8 +259,8 @@ bool RSHashTableIter::has_next(size_t& card_index) {
   _tbl_ind++;
   while ((size_t)_tbl_ind < _rsht->capacity()) {
     _bl_ind = _rsht->_buckets[_tbl_ind];
-    ci = find_first_card_in_list();
+    CardIdx_t ci = find_first_card_in_list();
-    if (ci != SparsePRTEntry::NullEntry) {
+    if (ci != NoCardFound) {
       card_index = compute_card_ind(ci);
       return true;
     }
@@ -275,7 +278,7 @@ bool RSHashTable::contains_card(RegionIdx_t region_index, CardIdx_t card_index)
 
 size_t RSHashTable::mem_size() const {
   return sizeof(RSHashTable) +
-    capacity() * (SparsePRTEntry::size() + sizeof(int));
+    _num_entries * (SparsePRTEntry::size() + sizeof(int));
 }
 
 // ----------------------------------------------------------------------
@@ -380,16 +383,12 @@ size_t SparsePRT::mem_size() const {
 }
 
 bool SparsePRT::add_card(RegionIdx_t region_id, CardIdx_t card_index) {
-  if (_next->occupied_entries() * 2 > _next->capacity()) {
+  if (_next->should_expand()) {
     expand();
   }
   return _next->add_card(region_id, card_index);
 }
 
-bool SparsePRT::get_cards(RegionIdx_t region_id, CardIdx_t* cards) {
-  return _next->get_cards(region_id, cards);
-}
-
 SparsePRTEntry* SparsePRT::get_entry(RegionIdx_t region_id) {
   return _next->get_entry(region_id);
 }
@@ -427,7 +426,7 @@ void SparsePRT::cleanup() {
 void SparsePRT::expand() {
   RSHashTable* last = _next;
   _next = new RSHashTable(last->capacity() * 2);
-  for (size_t i = 0; i < last->capacity(); i++) {
+  for (size_t i = 0; i < last->num_entries(); i++) {
     SparsePRTEntry* e = last->entry((int)i);
     if (e->valid_entry()) {
       _next->add_entry(e);

hotspot/src/share/vm/gc/g1/sparsePRT.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -43,26 +43,32 @@
 // old versions synchronously.
 
 class SparsePRTEntry: public CHeapObj<mtGC> {
-public:
-  enum SomePublicConstants {
-    NullEntry     = -1,
-    UnrollFactor  =  4
-  };
 private:
+  // The type of a card entry.
+  typedef uint16_t card_elem_t;
+
+  // We need to make sizeof(SparsePRTEntry) an even multiple of maximum member size,
+  // in order to force correct alignment that could otherwise cause SIGBUS errors
+  // when reading the member variables. This calculates the minimum number of card
+  // array elements required to get that alignment.
+  static const size_t card_array_alignment = sizeof(int) / sizeof(card_elem_t);
+
   RegionIdx_t _region_ind;
   int         _next_index;
-  CardIdx_t   _cards[1];
+  int         _next_null;
+  // The actual cards stored in this array.
   // WARNING: Don't put any data members beyond this line. Card array has, in fact, variable length.
   // It should always be the last data member.
+  card_elem_t _cards[card_array_alignment];
+
+  // Copy the current entry's cards into "cards".
+  inline void copy_cards(card_elem_t* cards) const;
 public:
   // Returns the size of the entry, used for entry allocation.
-  static size_t size() { return sizeof(SparsePRTEntry) + sizeof(CardIdx_t) * (cards_num() - 1); }
+  static size_t size() { return sizeof(SparsePRTEntry) + sizeof(card_elem_t) * (cards_num() - card_array_alignment); }
   // Returns the size of the card array.
   static int cards_num() {
-    // The number of cards should be a multiple of 4, because that's our current
+    return align_size_up(G1RSetSparseRegionEntries, card_array_alignment);
-    // unrolling factor.
-    static const int s = MAX2<int>(G1RSetSparseRegionEntries & ~(UnrollFactor - 1), UnrollFactor);
-    return s;
   }
 
   // Set the region_ind to the given value, and delete all cards.
@@ -80,7 +86,7 @@ public:
   inline bool contains_card(CardIdx_t card_index) const;
 
   // Returns the number of non-NULL card entries.
-  inline int num_valid_cards() const;
+  inline int num_valid_cards() const { return _next_null; }
 
   // Requires that the entry not contain the given card index.  If there is
   // space available, add the given card index to the entry and return
@@ -92,22 +98,25 @@ public:
   };
   inline AddCardResult add_card(CardIdx_t card_index);
 
-  // Copy the current entry's cards into "cards".
-  inline void copy_cards(CardIdx_t* cards) const;
   // Copy the current entry's cards into the "_card" array of "e."
   inline void copy_cards(SparsePRTEntry* e) const;
 
-  inline CardIdx_t card(int i) const { return _cards[i]; }
+  inline CardIdx_t card(int i) const {
+    assert(i >= 0, "must be nonnegative");
+    assert(i < cards_num(), "range checking");
+    return (CardIdx_t)_cards[i];
+  }
 };
 
-
 class RSHashTable : public CHeapObj<mtGC> {
 
   friend class RSHashTableIter;
 
-  enum SomePrivateConstants {
+
-    NullEntry = -1
+  // Inverse maximum hash table occupancy used.
-  };
+  static float TableOccupancyFactor;
+
+  size_t _num_entries;
 
   size_t _capacity;
   size_t _capacity_mask;
@@ -136,6 +145,10 @@ public:
   RSHashTable(size_t capacity);
   ~RSHashTable();
 
+  static const int NullEntry = -1;
+
+  bool should_expand() const { return _occupied_entries == _num_entries; }
+
   // Attempts to ensure that the given card_index in the given region is in
   // the sparse table.  If successful (because the card was already
   // present, or because it was successfully added) returns "true".
@@ -156,27 +169,35 @@ public:
 
   void clear();
 
-  size_t capacity() const      { return _capacity;       }
+  size_t capacity() const      { return _capacity; }
   size_t capacity_mask() const { return _capacity_mask;  }
   size_t occupied_entries() const { return _occupied_entries; }
-  size_t occupied_cards() const   { return _occupied_cards;   }
+  size_t occupied_cards() const   { return _occupied_cards; }
   size_t mem_size() const;
+  // The number of SparsePRTEntry instances available.
+  size_t num_entries() const { return _num_entries; }
 
-  SparsePRTEntry* entry(int i) const { return (SparsePRTEntry*)((char*)_entries + SparsePRTEntry::size() * i); }
+  SparsePRTEntry* entry(int i) const {
+    assert(i >= 0 && (size_t)i < _num_entries, "precondition");
+    return (SparsePRTEntry*)((char*)_entries + SparsePRTEntry::size() * i);
+  }
 
   void print();
 };
 
 // ValueObj because will be embedded in HRRS iterator.
 class RSHashTableIter VALUE_OBJ_CLASS_SPEC {
+  // Return value indicating "invalid/no card".
+  static const int NoCardFound = -1;
+
   int _tbl_ind;         // [-1, 0.._rsht->_capacity)
   int _bl_ind;          // [-1, 0.._rsht->_capacity)
   short _card_ind;      // [0..SparsePRTEntry::cards_num())
   RSHashTable* _rsht;
 
   // If the bucket list pointed to by _bl_ind contains a card, sets
-  // _bl_ind to the index of that entry, and returns the card.
+  // _bl_ind to the index of that entry,
-  // Otherwise, returns SparseEntry::NullEntry.
+  // Returns the card found if there is, otherwise returns InvalidCard.
   CardIdx_t find_first_card_in_list();
 
   // Computes the proper card index for the card whose offset in the
@@ -247,12 +268,6 @@ public:
   // entries to a larger-capacity representation.
   bool add_card(RegionIdx_t region_id, CardIdx_t card_index);
 
-  // If the table hold an entry for "region_ind",  Copies its
-  // cards into "cards", which must be an array of length at least
-  // "SparePRTEntry::cards_num()", and returns "true"; otherwise,
-  // returns "false".
-  bool get_cards(RegionIdx_t region_ind, CardIdx_t* cards);
-
   // Return the pointer to the entry associated with the given region.
   SparsePRTEntry* get_entry(RegionIdx_t region_ind);