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

rev 9982 : 8147087: Race when reusing PerRegionTable bitmaps may result in dropped remembered set entries
Summary: Do not make reused PRTs available to other threads before the bitmap of the PRT has been cleared.
Contributed-by: Poonam Bajaj <poonam.bajaj@oracle.com>, Thomas Schatzl <thomas.schatzl@oracle.com>
rev 9983 : [mq]: 8147087-comments

*** 1,7 ****
  /*
!  * Copyright (c) 2001, 2015, 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
   * under the terms of the GNU General Public License version 2 only, as
   * published by the Free Software Foundation.
--- 1,7 ----
  /*
!  * 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
   * under the terms of the GNU General Public License version 2 only, as
   * published by the Free Software Foundation.
*** 108,118 ****
      }
    }
  
  public:
  
!   HeapRegion* hr() const { return _hr; }
  
    jint occupied() const {
      // Overkill, but if we ever need it...
      // guarantee(_occupied == _bm.count_one_bits(), "Check");
      return _occupied;
--- 108,120 ----
      }
    }
  
  public:
  
!   HeapRegion* hr() const { 
!     return (HeapRegion*) OrderAccess::load_ptr_acquire(&_hr);
!   }
  
    jint occupied() const {
      // Overkill, but if we ever need it...
      // guarantee(_occupied == _bm.count_one_bits(), "Check");
      return _occupied;
*** 121,134 ****
    void init(HeapRegion* hr, bool clear_links_to_all_list) {
      if (clear_links_to_all_list) {
        set_next(NULL);
        set_prev(NULL);
      }
-     _hr = hr;
      _collision_list_next = NULL;
      _occupied = 0;
      _bm.clear();
    }
  
    void add_reference(OopOrNarrowOopStar from) {
      add_reference_work(from, /*parallel*/ true);
    }
--- 123,138 ----
    void init(HeapRegion* hr, bool clear_links_to_all_list) {
      if (clear_links_to_all_list) {
        set_next(NULL);
        set_prev(NULL);
      }
      _collision_list_next = NULL;
      _occupied = 0;
      _bm.clear();
+     // Make sure that the bitmap clearing above has been finished before publishing
+     // this PRT to concurrent threads.
+     OrderAccess::release_store_ptr(&_hr, hr);
    }
  
    void add_reference(OopOrNarrowOopStar from) {
      add_reference_work(from, /*parallel*/ true);
    }
*** 355,375 ****
    uint cur_hrm_ind = _hr->hrm_index();
  
    int from_card = (int)(uintptr_t(from) >> CardTableModRefBS::card_shift);
  
    if (G1FromCardCache::contains_or_replace(tid, cur_hrm_ind, from_card)) {
!     assert(contains_reference(from), "We just added it!");
      return;
    }
  
    // Note that this may be a continued H region.
    HeapRegion* from_hr = _g1h->heap_region_containing(from);
    RegionIdx_t from_hrm_ind = (RegionIdx_t) from_hr->hrm_index();
  
    // If the region is already coarsened, return.
    if (_coarse_map.at(from_hrm_ind)) {
!     assert(contains_reference(from), "We just added it!");
      return;
    }
  
    // Otherwise find a per-region table to add it to.
    size_t ind = from_hrm_ind & _mod_max_fine_entries_mask;
--- 359,379 ----
    uint cur_hrm_ind = _hr->hrm_index();
  
    int from_card = (int)(uintptr_t(from) >> CardTableModRefBS::card_shift);
  
    if (G1FromCardCache::contains_or_replace(tid, cur_hrm_ind, from_card)) {
!     assert(contains_reference(from), "We just found " PTR_FORMAT " in the FromCardCache", p2i(from));
      return;
    }
  
    // Note that this may be a continued H region.
    HeapRegion* from_hr = _g1h->heap_region_containing(from);
    RegionIdx_t from_hrm_ind = (RegionIdx_t) from_hr->hrm_index();
  
    // If the region is already coarsened, return.
    if (_coarse_map.at(from_hrm_ind)) {
!     assert(contains_reference(from), "We just found " PTR_FORMAT " in the Coarse table", p2i(from));
      return;
    }
  
    // Otherwise find a per-region table to add it to.
    size_t ind = from_hrm_ind & _mod_max_fine_entries_mask;
*** 386,396 ****
        CardIdx_t card_index = from_card - from_hr_bot_card_index;
        assert(0 <= card_index && (size_t)card_index < HeapRegion::CardsPerRegion,
               "Must be in range.");
        if (G1HRRSUseSparseTable &&
            _sparse_table.add_card(from_hrm_ind, card_index)) {
!         assert(contains_reference_locked(from), "We just added it!");
          return;
        }
  
        if (_n_fine_entries == _max_fine_entries) {
          prt = delete_region_table();
--- 390,400 ----
        CardIdx_t card_index = from_card - from_hr_bot_card_index;
        assert(0 <= card_index && (size_t)card_index < HeapRegion::CardsPerRegion,
               "Must be in range.");
        if (G1HRRSUseSparseTable &&
            _sparse_table.add_card(from_hrm_ind, card_index)) {
!         assert(contains_reference_locked(from), "We just added " PTR_FORMAT " to the Sparse table", p2i(from));
          return;
        }
  
        if (_n_fine_entries == _max_fine_entries) {
          prt = delete_region_table();
*** 436,446 ****
    // possibility of concurrent reuse.  But see head comment of
    // OtherRegionsTable for why this is OK.
    assert(prt != NULL, "Inv");
  
    prt->add_reference(from);
!   assert(contains_reference(from), "We just added it!");
  }
  
  PerRegionTable*
  OtherRegionsTable::find_region_table(size_t ind, HeapRegion* hr) const {
    assert(ind < _max_fine_entries, "Preconditions.");
--- 440,450 ----
    // possibility of concurrent reuse.  But see head comment of
    // OtherRegionsTable for why this is OK.
    assert(prt != NULL, "Inv");
  
    prt->add_reference(from);
!   assert(contains_reference(from), "We just added " PTR_FORMAT " to the PRT", p2i(from));
  }
  
  PerRegionTable*
  OtherRegionsTable::find_region_table(size_t ind, HeapRegion* hr) const {
    assert(ind < _max_fine_entries, "Preconditions.");