Mercurial > hg > truffle
diff src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp @ 355:0edda524b58c
6722565: G1: assert !r->is_on_unclean_list() fires
Summary: Under certain circumstances, two cleanup threads can claim and process the same region.
Reviewed-by: apetrusenko, ysr
author | tonyp |
---|---|
date | Wed, 06 Aug 2008 11:57:31 -0400 |
parents | c0f8f7790199 |
children | cc68c8e9b309 |
line wrap: on
line diff
--- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp Wed Jul 30 10:45:52 2008 -0700 +++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp Wed Aug 06 11:57:31 2008 -0400 @@ -1715,38 +1715,129 @@ HeapRegion* G1CollectedHeap::region_at(size_t idx) { return _hrs->at(idx); } -const int OverpartitionFactor = 4; void G1CollectedHeap::heap_region_par_iterate_chunked(HeapRegionClosure* cl, int worker, jint claim_value) { - // We break up the heap regions into blocks of size ParallelGCThreads (to - // decrease iteration costs). - const size_t nregions = n_regions(); - const size_t n_thrds = (ParallelGCThreads > 0 ? ParallelGCThreads : 1); - const size_t partitions = n_thrds * OverpartitionFactor; - const size_t BlkSize = MAX2(nregions/partitions, (size_t)1); - const size_t n_blocks = (nregions + BlkSize - 1)/BlkSize; - assert(ParallelGCThreads > 0 || worker == 0, "Precondition"); - const int init_idx = (int) (n_blocks/n_thrds * worker); - for (size_t blk = 0; blk < n_blocks; blk++) { - size_t idx = init_idx + blk; - if (idx >= n_blocks) idx = idx - n_blocks; - size_t reg_idx = idx * BlkSize; - assert(reg_idx < nregions, "Because we rounded blk up."); - HeapRegion* r = region_at(reg_idx); + const size_t regions = n_regions(); + const size_t worker_num = (ParallelGCThreads > 0 ? ParallelGCThreads : 1); + // try to spread out the starting points of the workers + const size_t start_index = regions / worker_num * (size_t) worker; + + // each worker will actually look at all regions + for (size_t count = 0; count < regions; ++count) { + const size_t index = (start_index + count) % regions; + assert(0 <= index && index < regions, "sanity"); + HeapRegion* r = region_at(index); + // we'll ignore "continues humongous" regions (we'll process them + // when we come across their corresponding "start humongous" + // region) and regions already claimed + if (r->claim_value() == claim_value || r->continuesHumongous()) { + continue; + } + // OK, try to claim it if (r->claimHeapRegion(claim_value)) { - for (size_t j = 0; j < BlkSize; j++) { - size_t reg_idx2 = reg_idx + j; - if (reg_idx2 == nregions) break; - HeapRegion* r2 = region_at(reg_idx2); - if (j > 0) r2->set_claim_value(claim_value); - bool res = cl->doHeapRegion(r2); - guarantee(!res, "Should not abort."); + // success! + assert(!r->continuesHumongous(), "sanity"); + if (r->startsHumongous()) { + // If the region is "starts humongous" we'll iterate over its + // "continues humongous" first; in fact we'll do them + // first. The order is important. In on case, calling the + // closure on the "starts humongous" region might de-allocate + // and clear all its "continues humongous" regions and, as a + // result, we might end up processing them twice. So, we'll do + // them first (notice: most closures will ignore them anyway) and + // then we'll do the "starts humongous" region. + for (size_t ch_index = index + 1; ch_index < regions; ++ch_index) { + HeapRegion* chr = region_at(ch_index); + + // if the region has already been claimed or it's not + // "continues humongous" we're done + if (chr->claim_value() == claim_value || + !chr->continuesHumongous()) { + break; + } + + // Noone should have claimed it directly. We can given + // that we claimed its "starts humongous" region. + assert(chr->claim_value() != claim_value, "sanity"); + assert(chr->humongous_start_region() == r, "sanity"); + + if (chr->claimHeapRegion(claim_value)) { + // we should always be able to claim it; noone else should + // be trying to claim this region + + bool res2 = cl->doHeapRegion(chr); + assert(!res2, "Should not abort"); + + // Right now, this holds (i.e., no closure that actually + // does something with "continues humongous" regions + // clears them). We might have to weaken it in the future, + // but let's leave these two asserts here for extra safety. + assert(chr->continuesHumongous(), "should still be the case"); + assert(chr->humongous_start_region() == r, "sanity"); + } else { + guarantee(false, "we should not reach here"); + } + } + } + + assert(!r->continuesHumongous(), "sanity"); + bool res = cl->doHeapRegion(r); + assert(!res, "Should not abort"); + } + } +} + +#ifdef ASSERT +// This checks whether all regions in the heap have the correct claim +// value. I also piggy-backed on this a check to ensure that the +// humongous_start_region() information on "continues humongous" +// regions is correct. + +class CheckClaimValuesClosure : public HeapRegionClosure { +private: + jint _claim_value; + size_t _failures; + HeapRegion* _sh_region; +public: + CheckClaimValuesClosure(jint claim_value) : + _claim_value(claim_value), _failures(0), _sh_region(NULL) { } + bool doHeapRegion(HeapRegion* r) { + if (r->claim_value() != _claim_value) { + gclog_or_tty->print_cr("Region ["PTR_FORMAT","PTR_FORMAT"), " + "claim value = %d, should be %d", + r->bottom(), r->end(), r->claim_value(), + _claim_value); + ++_failures; + } + if (!r->isHumongous()) { + _sh_region = NULL; + } else if (r->startsHumongous()) { + _sh_region = r; + } else if (r->continuesHumongous()) { + if (r->humongous_start_region() != _sh_region) { + gclog_or_tty->print_cr("Region ["PTR_FORMAT","PTR_FORMAT"), " + "HS = "PTR_FORMAT", should be "PTR_FORMAT, + r->bottom(), r->end(), + r->humongous_start_region(), + _sh_region); + ++_failures; } } - } -} + return false; + } + size_t failures() { + return _failures; + } +}; + +bool G1CollectedHeap::check_heap_region_claim_values(jint claim_value) { + CheckClaimValuesClosure cl(claim_value); + heap_region_iterate(&cl); + return cl.failures() == 0; +} +#endif // ASSERT void G1CollectedHeap::collection_set_iterate(HeapRegionClosure* cl) { HeapRegion* r = g1_policy()->collection_set();