7095236: G1: _markedRegions never contains NULL regions

Thu, 06 Oct 2011 18:56:47 -0700

author
ysr
date
Thu, 06 Oct 2011 18:56:47 -0700
changeset 3185
b9390528617c
parent 3183
fd65bc7c09b6
child 3186
f32dae5d5677

7095236: G1: _markedRegions never contains NULL regions
Summary: Removed the code for skipping over NULL regions in _markedRegions, replacing it with an assertion that a NULL region is never encountered; removed dead methods, remove() and remove_region(), and inlined a simplified addRegion() directly into fillCache().
Reviewed-by: brutisso, tonyp

src/share/vm/gc_implementation/g1/collectionSetChooser.cpp file | annotate | diff | comparison | revisions
src/share/vm/gc_implementation/g1/collectionSetChooser.hpp file | annotate | diff | comparison | revisions
     1.1 --- a/src/share/vm/gc_implementation/g1/collectionSetChooser.cpp	Thu Oct 06 13:28:09 2011 -0400
     1.2 +++ b/src/share/vm/gc_implementation/g1/collectionSetChooser.cpp	Thu Oct 06 18:56:47 2011 -0700
     1.3 @@ -118,30 +118,6 @@
     1.4    }
     1.5  }
     1.6  
     1.7 -// this is a bit expensive... but we expect that it should not be called
     1.8 -// to often.
     1.9 -void CSetChooserCache::remove(HeapRegion *hr) {
    1.10 -  assert(_occupancy > 0, "cache should not be empty");
    1.11 -  assert(hr->sort_index() < -1, "should already be in the cache");
    1.12 -  int index = get_index(hr->sort_index());
    1.13 -  assert(_cache[index] == hr, "index should be correct");
    1.14 -  int next_index = trim_index(index + 1);
    1.15 -  int last_index = trim_index(_first + _occupancy - 1);
    1.16 -  while (index != last_index) {
    1.17 -    assert(_cache[next_index] != NULL, "should not be null");
    1.18 -    _cache[index] = _cache[next_index];
    1.19 -    _cache[index]->set_sort_index(get_sort_index(index));
    1.20 -
    1.21 -    index = next_index;
    1.22 -    next_index = trim_index(next_index+1);
    1.23 -  }
    1.24 -  assert(index == last_index, "should have reached the last one");
    1.25 -  _cache[index] = NULL;
    1.26 -  hr->set_sort_index(-1);
    1.27 -  --_occupancy;
    1.28 -  assert(verify(), "cache should be consistent");
    1.29 -}
    1.30 -
    1.31  static inline int orderRegions(HeapRegion* hr1, HeapRegion* hr2) {
    1.32    if (hr1 == NULL) {
    1.33      if (hr2 == NULL) return 0;
    1.34 @@ -197,43 +173,34 @@
    1.35    HeapRegion *prev = NULL;
    1.36    while (index < _numMarkedRegions) {
    1.37      HeapRegion *curr = _markedRegions.at(index++);
    1.38 -    if (curr != NULL) {
    1.39 -      int si = curr->sort_index();
    1.40 -      guarantee(!curr->is_young(), "should not be young!");
    1.41 -      guarantee(si > -1 && si == (index-1), "sort index invariant");
    1.42 -      if (prev != NULL) {
    1.43 -        guarantee(orderRegions(prev, curr) != 1, "regions should be sorted");
    1.44 -      }
    1.45 -      prev = curr;
    1.46 +    guarantee(curr != NULL, "Regions in _markedRegions array cannot be NULL");
    1.47 +    int si = curr->sort_index();
    1.48 +    guarantee(!curr->is_young(), "should not be young!");
    1.49 +    guarantee(si > -1 && si == (index-1), "sort index invariant");
    1.50 +    if (prev != NULL) {
    1.51 +      guarantee(orderRegions(prev, curr) != 1, "regions should be sorted");
    1.52      }
    1.53 +    prev = curr;
    1.54    }
    1.55    return _cache.verify();
    1.56  }
    1.57  #endif
    1.58  
    1.59 -bool
    1.60 -CollectionSetChooser::addRegionToCache() {
    1.61 -  assert(!_cache.is_full(), "cache should not be full");
    1.62 -
    1.63 -  HeapRegion *hr = NULL;
    1.64 -  while (hr == NULL && _curMarkedIndex < _numMarkedRegions) {
    1.65 -    hr = _markedRegions.at(_curMarkedIndex++);
    1.66 -  }
    1.67 -  if (hr == NULL)
    1.68 -    return false;
    1.69 -  assert(!hr->is_young(), "should not be young!");
    1.70 -  assert(hr->sort_index() == _curMarkedIndex-1, "sort_index invariant");
    1.71 -  _markedRegions.at_put(hr->sort_index(), NULL);
    1.72 -  _cache.insert(hr);
    1.73 -  assert(!_cache.is_empty(), "cache should not be empty");
    1.74 -  assert(verify(), "cache should be consistent");
    1.75 -  return false;
    1.76 -}
    1.77 -
    1.78  void
    1.79  CollectionSetChooser::fillCache() {
    1.80 -  while (!_cache.is_full() && addRegionToCache()) {
    1.81 +  while (!_cache.is_full() && (_curMarkedIndex < _numMarkedRegions)) {
    1.82 +    HeapRegion* hr = _markedRegions.at(_curMarkedIndex);
    1.83 +    assert(hr != NULL,
    1.84 +           err_msg("Unexpected NULL hr in _markedRegions at index %d",
    1.85 +                   _curMarkedIndex));
    1.86 +    _curMarkedIndex += 1;
    1.87 +    assert(!hr->is_young(), "should not be young!");
    1.88 +    assert(hr->sort_index() == _curMarkedIndex-1, "sort_index invariant");
    1.89 +    _markedRegions.at_put(hr->sort_index(), NULL);
    1.90 +    _cache.insert(hr);
    1.91 +    assert(!_cache.is_empty(), "cache should not be empty");
    1.92    }
    1.93 +  assert(verify(), "cache should be consistent");
    1.94  }
    1.95  
    1.96  void
    1.97 @@ -334,20 +301,6 @@
    1.98    clearMarkedHeapRegions();
    1.99  }
   1.100  
   1.101 -void CollectionSetChooser::removeRegion(HeapRegion *hr) {
   1.102 -  int si = hr->sort_index();
   1.103 -  assert(si == -1 || hr->is_marked(), "Sort index not valid.");
   1.104 -  if (si > -1) {
   1.105 -    assert(_markedRegions.at(si) == hr, "Sort index not valid." );
   1.106 -    _markedRegions.at_put(si, NULL);
   1.107 -  } else if (si < -1) {
   1.108 -    assert(_cache.region_in_cache(hr), "should be in the cache");
   1.109 -    _cache.remove(hr);
   1.110 -    assert(hr->sort_index() == -1, "sort index invariant");
   1.111 -  }
   1.112 -  hr->set_sort_index(-1);
   1.113 -}
   1.114 -
   1.115  // if time_remaining < 0.0, then this method should try to return
   1.116  // a region, whether it fits within the remaining time or not
   1.117  HeapRegion*
     2.1 --- a/src/share/vm/gc_implementation/g1/collectionSetChooser.hpp	Thu Oct 06 13:28:09 2011 -0400
     2.2 +++ b/src/share/vm/gc_implementation/g1/collectionSetChooser.hpp	Thu Oct 06 18:56:47 2011 -0700
     2.3 @@ -1,5 +1,5 @@
     2.4  /*
     2.5 - * Copyright (c) 2001, 2010, Oracle and/or its affiliates. All rights reserved.
     2.6 + * Copyright (c) 2001, 2011, Oracle and/or its affiliates. All rights reserved.
     2.7   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
     2.8   *
     2.9   * This code is free software; you can redistribute it and/or modify it
    2.10 @@ -29,6 +29,26 @@
    2.11  #include "utilities/growableArray.hpp"
    2.12  
    2.13  // We need to sort heap regions by collection desirability.
    2.14 +// This sorting is currently done in two "stages". An initial sort is
    2.15 +// done following a cleanup pause as soon as all of the marked but
    2.16 +// non-empty regions have been identified and the completely empty
    2.17 +// ones reclaimed.
    2.18 +// This gives us a global sort on a GC efficiency metric
    2.19 +// based on predictive data available at that time. However,
    2.20 +// any of these regions that are collected will only be collected
    2.21 +// during a future GC pause, by which time it is possible that newer
    2.22 +// data might allow us to revise and/or refine the earlier
    2.23 +// pause predictions, leading to changes in expected gc efficiency
    2.24 +// order. To somewhat mitigate this obsolescence, more so in the
    2.25 +// case of regions towards the end of the list, which will be
    2.26 +// picked later, these pre-sorted regions from the _markedRegions
    2.27 +// array are not used as is, but a small prefix thereof is
    2.28 +// insertion-sorted again into a small cache, based on more
    2.29 +// recent remembered set information. Regions are then drawn
    2.30 +// from this cache to construct the collection set at each
    2.31 +// incremental GC.
    2.32 +// This scheme and/or its implementation may be subject to
    2.33 +// revision in the future.
    2.34  
    2.35  class CSetChooserCache VALUE_OBJ_CLASS_SPEC {
    2.36  private:
    2.37 @@ -37,8 +57,8 @@
    2.38    } PrivateConstants;
    2.39  
    2.40    HeapRegion*  _cache[CacheLength];
    2.41 -  int          _occupancy; // number of region in cache
    2.42 -  int          _first; // "first" region in the cache
    2.43 +  int          _occupancy; // number of regions in cache
    2.44 +  int          _first;     // (index of) "first" region in the cache
    2.45  
    2.46    // adding CacheLength to deal with negative values
    2.47    inline int trim_index(int index) {
    2.48 @@ -62,7 +82,6 @@
    2.49    void clear(void);
    2.50    void insert(HeapRegion *hr);
    2.51    HeapRegion *remove_first(void);
    2.52 -  void remove (HeapRegion *hr);
    2.53    inline HeapRegion *get_first(void) {
    2.54      return _cache[_first];
    2.55    }
    2.56 @@ -102,7 +121,6 @@
    2.57  
    2.58    void sortMarkedHeapRegions();
    2.59    void fillCache();
    2.60 -  bool addRegionToCache(void);
    2.61    void addMarkedHeapRegion(HeapRegion *hr);
    2.62  
    2.63    // Must be called before calls to getParMarkedHeapRegionChunk.
    2.64 @@ -122,9 +140,6 @@
    2.65  
    2.66    void updateAfterFullCollection();
    2.67  
    2.68 -  // Ensure that "hr" is not a member of the marked region array or the cache
    2.69 -  void removeRegion(HeapRegion* hr);
    2.70 -
    2.71    bool unmarked_age_1_returned_as_new() { return _unmarked_age_1_returned_as_new; }
    2.72  
    2.73    // Returns true if the used portion of "_markedRegions" is properly

mercurial