From 959b98385a398d51d737a378871550f26ed51c30 Mon Sep 17 00:00:00 2001 From: tschatzl Date: Mon, 21 Jul 2014 10:00:31 +0200 Subject: [PATCH] 8048112: G1 Full GC needs to support the case when the very first region is not available Summary: Refactor preparation for compaction during Full GC so that it lazily initializes the first compaction point. This also avoids problems later when the first region may not be committed. Also reviewed by K. Barrett. Reviewed-by: brutisso --- .../gc_implementation/g1/g1CollectedHeap.cpp | 13 ++++-- .../gc_implementation/g1/g1CollectedHeap.hpp | 13 +++--- .../vm/gc_implementation/g1/g1MarkSweep.cpp | 40 ++++++++++--------- .../vm/gc_implementation/g1/heapRegion.cpp | 13 +----- .../vm/gc_implementation/g1/heapRegionSet.hpp | 2 +- src/share/vm/memory/genCollectedHeap.cpp | 2 +- src/share/vm/memory/space.hpp | 6 +-- 7 files changed, 44 insertions(+), 45 deletions(-) diff --git a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp index 2a252aac0..cd561b17c 100644 --- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp +++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp @@ -2971,11 +2971,18 @@ void G1CollectedHeap::collection_set_iterate_from(HeapRegion* r, } } -CompactibleSpace* G1CollectedHeap::first_compactible_space() { - return n_regions() > 0 ? region_at(0) : NULL; +HeapRegion* G1CollectedHeap::next_compaction_region(const HeapRegion* from) const { + // We're not using an iterator given that it will wrap around when + // it reaches the last region and this is not what we want here. + for (uint index = from->hrs_index() + 1; index < n_regions(); index++) { + HeapRegion* hr = region_at(index); + if (!hr->isHumongous()) { + return hr; + } + } + return NULL; } - Space* G1CollectedHeap::space_containing(const void* addr) const { Space* res = heap_region_containing(addr); return res; diff --git a/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp b/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp index 2acb74485..ce3bd2445 100644 --- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp +++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp @@ -1167,19 +1167,19 @@ public: } // The total number of regions in the heap. - uint n_regions() { return _hrs.length(); } + uint n_regions() const { return _hrs.length(); } // The max number of regions in the heap. - uint max_regions() { return _hrs.max_length(); } + uint max_regions() const { return _hrs.max_length(); } // The number of regions that are completely free. - uint free_regions() { return _free_list.length(); } + uint free_regions() const { return _free_list.length(); } // The number of regions that are not completely free. - uint used_regions() { return n_regions() - free_regions(); } + uint used_regions() const { return n_regions() - free_regions(); } // The number of regions available for "regular" expansion. - uint expansion_regions() { return _expansion_regions; } + uint expansion_regions() const { return _expansion_regions; } // Factory method for HeapRegion instances. It will return NULL if // the allocation fails. @@ -1401,8 +1401,7 @@ public: // As above but starting from region r void collection_set_iterate_from(HeapRegion* r, HeapRegionClosure *blk); - // Returns the first (lowest address) compactible space in the heap. - virtual CompactibleSpace* first_compactible_space(); + HeapRegion* next_compaction_region(const HeapRegion* from) const; // A CollectedHeap will contain some number of spaces. This finds the // space containing a given address, or else returns NULL. diff --git a/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp b/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp index d9e15f654..263651534 100644 --- a/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp +++ b/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp @@ -200,6 +200,23 @@ class G1PrepareCompactClosure: public HeapRegionClosure { CompactPoint _cp; HeapRegionSetCount _humongous_regions_removed; + bool is_cp_initialized() const { + return _cp.space != NULL; + } + + void prepare_for_compaction(HeapRegion* hr, HeapWord* end) { + // If this is the first live region that we came across which we can compact, + // initialize the CompactPoint. + if (!is_cp_initialized()) { + _cp.space = hr; + _cp.threshold = hr->initialize_threshold(); + } + hr->prepare_for_compaction(&_cp); + // Also clear the part of the card table that will be unused after + // compaction. + _mrbs->clear(MemRegion(hr->compaction_top(), end)); + } + void free_humongous_region(HeapRegion* hr) { HeapWord* end = hr->end(); FreeRegionList dummy_free_list("Dummy Free List for G1MarkSweep"); @@ -211,18 +228,15 @@ class G1PrepareCompactClosure: public HeapRegionClosure { _humongous_regions_removed.increment(1u, hr->capacity()); _g1h->free_humongous_region(hr, &dummy_free_list, false /* par */); - hr->prepare_for_compaction(&_cp); - // Also clear the part of the card table that will be unused after - // compaction. - _mrbs->clear(MemRegion(hr->compaction_top(), end)); + prepare_for_compaction(hr, end); dummy_free_list.remove_all(); } public: - G1PrepareCompactClosure(CompactibleSpace* cs) + G1PrepareCompactClosure() : _g1h(G1CollectedHeap::heap()), _mrbs(_g1h->g1_barrier_set()), - _cp(NULL, cs, cs->initialize_threshold()), + _cp(NULL), _humongous_regions_removed() { } void update_sets() { @@ -245,10 +259,7 @@ public: assert(hr->continuesHumongous(), "Invalid humongous."); } } else { - hr->prepare_for_compaction(&_cp); - // Also clear the part of the card table that will be unused after - // compaction. - _mrbs->clear(MemRegion(hr->compaction_top(), hr->end())); + prepare_for_compaction(hr, hr->end()); } return false; } @@ -266,14 +277,7 @@ void G1MarkSweep::mark_sweep_phase2() { GCTraceTime tm("phase 2", G1Log::fine() && Verbose, true, gc_timer(), gc_tracer()->gc_id()); GenMarkSweep::trace("2"); - // find the first region - HeapRegion* r = g1h->region_at(0); - CompactibleSpace* sp = r; - if (r->isHumongous() && oop(r->bottom())->is_gc_marked()) { - sp = r->next_compaction_space(); - } - - G1PrepareCompactClosure blk(sp); + G1PrepareCompactClosure blk; g1h->heap_region_iterate(&blk); blk.update_sets(); } diff --git a/src/share/vm/gc_implementation/g1/heapRegion.cpp b/src/share/vm/gc_implementation/g1/heapRegion.cpp index 9b2ddbfb2..616049d12 100644 --- a/src/share/vm/gc_implementation/g1/heapRegion.cpp +++ b/src/share/vm/gc_implementation/g1/heapRegion.cpp @@ -380,18 +380,7 @@ HeapRegion::HeapRegion(uint hrs_index, } CompactibleSpace* HeapRegion::next_compaction_space() const { - // We're not using an iterator given that it will wrap around when - // it reaches the last region and this is not what we want here. - G1CollectedHeap* g1h = G1CollectedHeap::heap(); - uint index = hrs_index() + 1; - while (index < g1h->n_regions()) { - HeapRegion* hr = g1h->region_at(index); - if (!hr->isHumongous()) { - return hr; - } - index += 1; - } - return NULL; + return G1CollectedHeap::heap()->next_compaction_region(this); } void HeapRegion::note_self_forwarding_removal_start(bool during_initial_mark, diff --git a/src/share/vm/gc_implementation/g1/heapRegionSet.hpp b/src/share/vm/gc_implementation/g1/heapRegionSet.hpp index 222fc694f..8502763cc 100644 --- a/src/share/vm/gc_implementation/g1/heapRegionSet.hpp +++ b/src/share/vm/gc_implementation/g1/heapRegionSet.hpp @@ -119,7 +119,7 @@ protected: public: const char* name() { return _name; } - uint length() { return _count.length(); } + uint length() const { return _count.length(); } bool is_empty() { return _count.length() == 0; } diff --git a/src/share/vm/memory/genCollectedHeap.cpp b/src/share/vm/memory/genCollectedHeap.cpp index c496906ba..1c651204e 100644 --- a/src/share/vm/memory/genCollectedHeap.cpp +++ b/src/share/vm/memory/genCollectedHeap.cpp @@ -1099,7 +1099,7 @@ void GenCollectedHeap::prepare_for_compaction() { guarantee(_n_gens = 2, "Wrong number of generations"); Generation* old_gen = _gens[1]; // Start by compacting into same gen. - CompactPoint cp(old_gen, NULL, NULL); + CompactPoint cp(old_gen); old_gen->prepare_for_compaction(&cp); Generation* young_gen = _gens[0]; young_gen->prepare_for_compaction(&cp); diff --git a/src/share/vm/memory/space.hpp b/src/share/vm/memory/space.hpp index 0611f7428..f3723a8fb 100644 --- a/src/share/vm/memory/space.hpp +++ b/src/share/vm/memory/space.hpp @@ -330,9 +330,9 @@ public: Generation* gen; CompactibleSpace* space; HeapWord* threshold; - CompactPoint(Generation* _gen, CompactibleSpace* _space, - HeapWord* _threshold) : - gen(_gen), space(_space), threshold(_threshold) {} + + CompactPoint(Generation* _gen) : + gen(_gen), space(NULL), threshold(0) {} }; -- GitLab