From 3dda40c087f1bc502caefb3bd22dbbf3698c9367 Mon Sep 17 00:00:00 2001 From: tschatzl Date: Fri, 10 Oct 2014 15:51:58 +0200 Subject: [PATCH] 8059758: Footprint regressions with JDK-8038423 Summary: Changes in JDK-8038423 always initialize (zero out) virtual memory used for auxiliary data structures. This causes a footprint regression for G1 in startup benchmarks. This is because they do not touch that memory at all, so the operating system does not actually commit these pages. The fix is to, if the initialization value of the data structures matches the default value of just committed memory (=0), do not do anything. Reviewed-by: jwilhelm, brutisso --- src/share/vm/gc_implementation/g1/concurrentMark.cpp | 5 ++++- src/share/vm/gc_implementation/g1/concurrentMark.hpp | 2 +- .../vm/gc_implementation/g1/g1BlockOffsetTable.hpp | 2 +- src/share/vm/gc_implementation/g1/g1CardCounts.cpp | 5 ++++- src/share/vm/gc_implementation/g1/g1CardCounts.hpp | 2 +- src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp | 4 +++- src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp | 2 +- .../vm/gc_implementation/g1/g1RegionToSpaceMapper.cpp | 10 ++++++---- .../vm/gc_implementation/g1/g1RegionToSpaceMapper.hpp | 6 ++++-- .../gc_implementation/g1/g1SATBCardTableModRefBS.cpp | 3 ++- .../gc_implementation/g1/g1SATBCardTableModRefBS.hpp | 2 +- 11 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/share/vm/gc_implementation/g1/concurrentMark.cpp b/src/share/vm/gc_implementation/g1/concurrentMark.cpp index e138e8cce..88cf78a6a 100644 --- a/src/share/vm/gc_implementation/g1/concurrentMark.cpp +++ b/src/share/vm/gc_implementation/g1/concurrentMark.cpp @@ -130,7 +130,10 @@ void CMBitMap::initialize(MemRegion heap, G1RegionToSpaceMapper* storage) { storage->set_mapping_changed_listener(&_listener); } -void CMBitMapMappingChangedListener::on_commit(uint start_region, size_t num_regions) { +void CMBitMapMappingChangedListener::on_commit(uint start_region, size_t num_regions, bool zero_filled) { + if (zero_filled) { + return; + } // We need to clear the bitmap on commit, removing any existing information. MemRegion mr(G1CollectedHeap::heap()->bottom_addr_for_region(start_region), num_regions * HeapRegion::GrainWords); _bm->clearRange(mr); diff --git a/src/share/vm/gc_implementation/g1/concurrentMark.hpp b/src/share/vm/gc_implementation/g1/concurrentMark.hpp index e507f677e..8a1120e73 100644 --- a/src/share/vm/gc_implementation/g1/concurrentMark.hpp +++ b/src/share/vm/gc_implementation/g1/concurrentMark.hpp @@ -127,7 +127,7 @@ class CMBitMapMappingChangedListener : public G1MappingChangedListener { void set_bitmap(CMBitMap* bm) { _bm = bm; } - virtual void on_commit(uint start_idx, size_t num_regions); + virtual void on_commit(uint start_idx, size_t num_regions, bool zero_filled); }; class CMBitMap : public CMBitMapRO { diff --git a/src/share/vm/gc_implementation/g1/g1BlockOffsetTable.hpp b/src/share/vm/gc_implementation/g1/g1BlockOffsetTable.hpp index d6767a80c..e0de6f15d 100644 --- a/src/share/vm/gc_implementation/g1/g1BlockOffsetTable.hpp +++ b/src/share/vm/gc_implementation/g1/g1BlockOffsetTable.hpp @@ -109,7 +109,7 @@ public: class G1BlockOffsetSharedArrayMappingChangedListener : public G1MappingChangedListener { public: - virtual void on_commit(uint start_idx, size_t num_regions) { + virtual void on_commit(uint start_idx, size_t num_regions, bool zero_filled) { // Nothing to do. The BOT is hard-wired to be part of the HeapRegion, and we cannot // retrieve it here since this would cause firing of several asserts. The code // executed after commit of a region already needs to do some re-initialization of diff --git a/src/share/vm/gc_implementation/g1/g1CardCounts.cpp b/src/share/vm/gc_implementation/g1/g1CardCounts.cpp index f9fd47cfb..76bf8509a 100644 --- a/src/share/vm/gc_implementation/g1/g1CardCounts.cpp +++ b/src/share/vm/gc_implementation/g1/g1CardCounts.cpp @@ -33,7 +33,10 @@ PRAGMA_FORMAT_MUTE_WARNINGS_FOR_GCC -void G1CardCountsMappingChangedListener::on_commit(uint start_idx, size_t num_regions) { +void G1CardCountsMappingChangedListener::on_commit(uint start_idx, size_t num_regions, bool zero_filled) { + if (zero_filled) { + return; + } MemRegion mr(G1CollectedHeap::heap()->bottom_addr_for_region(start_idx), num_regions * HeapRegion::GrainWords); _counts->clear_range(mr); } diff --git a/src/share/vm/gc_implementation/g1/g1CardCounts.hpp b/src/share/vm/gc_implementation/g1/g1CardCounts.hpp index 4252cba09..7dcad458b 100644 --- a/src/share/vm/gc_implementation/g1/g1CardCounts.hpp +++ b/src/share/vm/gc_implementation/g1/g1CardCounts.hpp @@ -42,7 +42,7 @@ class G1CardCountsMappingChangedListener : public G1MappingChangedListener { public: void set_cardcounts(G1CardCounts* counts) { _counts = counts; } - virtual void on_commit(uint start_idx, size_t num_regions); + virtual void on_commit(uint start_idx, size_t num_regions, bool zero_filled); }; // Table to track the number of times a card has been refined. Once diff --git a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp index c4f97d842..858359c9b 100644 --- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp +++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp @@ -385,7 +385,9 @@ void G1RegionMappingChangedListener::reset_from_card_cache(uint start_idx, size_ OtherRegionsTable::invalidate(start_idx, num_regions); } -void G1RegionMappingChangedListener::on_commit(uint start_idx, size_t num_regions) { +void G1RegionMappingChangedListener::on_commit(uint start_idx, size_t num_regions, bool zero_filled) { + // The from card cache is not the memory that is actually committed. So we cannot + // take advantage of the zero_filled parameter. reset_from_card_cache(start_idx, num_regions); } diff --git a/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp b/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp index b1110da9e..0e869f5b6 100644 --- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp +++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp @@ -172,7 +172,7 @@ class G1RegionMappingChangedListener : public G1MappingChangedListener { private: void reset_from_card_cache(uint start_idx, size_t num_regions); public: - virtual void on_commit(uint start_idx, size_t num_regions); + virtual void on_commit(uint start_idx, size_t num_regions, bool zero_filled); }; class G1CollectedHeap : public SharedHeap { diff --git a/src/share/vm/gc_implementation/g1/g1RegionToSpaceMapper.cpp b/src/share/vm/gc_implementation/g1/g1RegionToSpaceMapper.cpp index da0976ca9..41eb0265d 100644 --- a/src/share/vm/gc_implementation/g1/g1RegionToSpaceMapper.cpp +++ b/src/share/vm/gc_implementation/g1/g1RegionToSpaceMapper.cpp @@ -69,7 +69,7 @@ class G1RegionsLargerThanCommitSizeMapper : public G1RegionToSpaceMapper { virtual void commit_regions(uintptr_t start_idx, size_t num_regions) { _storage.commit(start_idx * _pages_per_region, num_regions * _pages_per_region); _commit_map.set_range(start_idx, start_idx + num_regions); - fire_on_commit(start_idx, num_regions); + fire_on_commit(start_idx, num_regions, true); } virtual void uncommit_regions(uintptr_t start_idx, size_t num_regions) { @@ -115,12 +115,14 @@ class G1RegionsSmallerThanCommitSizeMapper : public G1RegionToSpaceMapper { assert(!_commit_map.at(i), err_msg("Trying to commit storage at region "INTPTR_FORMAT" that is already committed", i)); uintptr_t idx = region_idx_to_page_idx(i); uint old_refcount = _refcounts.get_by_index(idx); + bool zero_filled = false; if (old_refcount == 0) { _storage.commit(idx, 1); + zero_filled = true; } _refcounts.set_by_index(idx, old_refcount + 1); _commit_map.set_bit(i); - fire_on_commit(i, 1); + fire_on_commit(i, 1, zero_filled); } } @@ -139,9 +141,9 @@ class G1RegionsSmallerThanCommitSizeMapper : public G1RegionToSpaceMapper { } }; -void G1RegionToSpaceMapper::fire_on_commit(uint start_idx, size_t num_regions) { +void G1RegionToSpaceMapper::fire_on_commit(uint start_idx, size_t num_regions, bool zero_filled) { if (_listener != NULL) { - _listener->on_commit(start_idx, num_regions); + _listener->on_commit(start_idx, num_regions, zero_filled); } } diff --git a/src/share/vm/gc_implementation/g1/g1RegionToSpaceMapper.hpp b/src/share/vm/gc_implementation/g1/g1RegionToSpaceMapper.hpp index 5f614b7a8..6b3420649 100644 --- a/src/share/vm/gc_implementation/g1/g1RegionToSpaceMapper.hpp +++ b/src/share/vm/gc_implementation/g1/g1RegionToSpaceMapper.hpp @@ -33,7 +33,9 @@ class G1MappingChangedListener VALUE_OBJ_CLASS_SPEC { public: // Fired after commit of the memory, i.e. the memory this listener is registered // for can be accessed. - virtual void on_commit(uint start_idx, size_t num_regions) = 0; + // Zero_filled indicates that the memory can be considered as filled with zero bytes + // when called. + virtual void on_commit(uint start_idx, size_t num_regions, bool zero_filled) = 0; }; // Maps region based commit/uncommit requests to the underlying page sized virtual @@ -51,7 +53,7 @@ class G1RegionToSpaceMapper : public CHeapObj { G1RegionToSpaceMapper(ReservedSpace rs, size_t commit_granularity, size_t region_granularity, MemoryType type); - void fire_on_commit(uint start_idx, size_t num_regions); + void fire_on_commit(uint start_idx, size_t num_regions, bool zero_filled); public: MemRegion reserved() { return _storage.reserved(); } diff --git a/src/share/vm/gc_implementation/g1/g1SATBCardTableModRefBS.cpp b/src/share/vm/gc_implementation/g1/g1SATBCardTableModRefBS.cpp index 26c88293b..d1b9bd4c6 100644 --- a/src/share/vm/gc_implementation/g1/g1SATBCardTableModRefBS.cpp +++ b/src/share/vm/gc_implementation/g1/g1SATBCardTableModRefBS.cpp @@ -124,7 +124,8 @@ void G1SATBCardTableModRefBS::verify_g1_young_region(MemRegion mr) { } #endif -void G1SATBCardTableLoggingModRefBSChangedListener::on_commit(uint start_idx, size_t num_regions) { +void G1SATBCardTableLoggingModRefBSChangedListener::on_commit(uint start_idx, size_t num_regions, bool zero_filled) { + // Default value for a clean card on the card table is -1. So we cannot take advantage of the zero_filled parameter. MemRegion mr(G1CollectedHeap::heap()->bottom_addr_for_region(start_idx), num_regions * HeapRegion::GrainWords); _card_table->clear(mr); } diff --git a/src/share/vm/gc_implementation/g1/g1SATBCardTableModRefBS.hpp b/src/share/vm/gc_implementation/g1/g1SATBCardTableModRefBS.hpp index 80c402e99..1fbe45faa 100644 --- a/src/share/vm/gc_implementation/g1/g1SATBCardTableModRefBS.hpp +++ b/src/share/vm/gc_implementation/g1/g1SATBCardTableModRefBS.hpp @@ -136,7 +136,7 @@ class G1SATBCardTableLoggingModRefBSChangedListener : public G1MappingChangedLis void set_card_table(G1SATBCardTableLoggingModRefBS* card_table) { _card_table = card_table; } - virtual void on_commit(uint start_idx, size_t num_regions); + virtual void on_commit(uint start_idx, size_t num_regions, bool zero_filled); }; // Adds card-table logging to the post-barrier. -- GitLab