May 2013 - hotspot-gc-dev

hg: hsx/hotspot-gc/hotspot: 8014971: Minor code cleanup of the freelist management
by bengt.rutisson＠oracle.com 22 May '13

22 May '13

Changeset: 7c5a1b62f53d Author: brutisso Date: 2013-05-22 08:04 +0200 URL: http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/7c5a1b62f53d 8014971: Minor code cleanup of the freelist management Reviewed-by: jwilhelm, jmasa, tschatzl ! src/share/vm/gc_implementation/concurrentMarkSweep/adaptiveFreeList.cpp ! src/share/vm/gc_implementation/concurrentMarkSweep/adaptiveFreeList.hpp ! src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp ! src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp ! src/share/vm/memory/freeList.cpp ! src/share/vm/memory/freeList.hpp

1 0

Re: Request for review: 8007762: Rename a bunch of methods in size policy across collectors
by Tao Mao 22 May '13

22 May '13

Oops...sent to open list. Thanks. Tao On 3/4/2013 4:38 PM, Tao Mao wrote: > This is the proposed final webrev, with copyright dated appropriately. > http://cr.openjdk.java.net/~tamao/8007762/webrev.02/ > > Jon, > A patch is rendered below. Please take the patch and help push the > changeset. > http://cr.openjdk.java.net/~tamao/8007762/webrev.02/8007762RenameMethodsInS… > > Thank you. > Tao > > On 2/26/13 8:37 AM, Jon Masamitsu wrote: >> Thanks. >> >> Looks good. >> >> Jon >> >> On 02/25/13 11:09, Tao Mao wrote: >>> A new webrev is updated. >>> Thanks. >>> Tao >>> >>> On 2/18/2013 7:46 AM, Jon Masamitsu wrote: >>>> Tao, >>>> >>>> Why this change from resize_old_gen() to >>>> resize_tenured_gen()? There are many >>>> variable names and comments referring to >>>> "old gen" still in ParallelScavengeHeap. >>> Reverted. >>>> >>>> http://cr.openjdk.java.net/~tamao/8007762/webrev.00/src/share/vm/gc_impleme… >>>> >>>> Jon >>>> >>>> On 02/14/13 11:18, Tao Mao wrote: >>>>> 8007762: Rename a bunch of methods in size policy across collectors >>>>> https://jbs.oracle.com/bugs/browse/JDK-8007762 >>>>> >>>>> webrev: >>>>> http://cr.openjdk.java.net/~tamao/8007762/webrev.00/ >>>>> >>>>> changeset: >>>>> The motivation of this change is associated with CR 7098155 >>>>> (https://jbs.oracle.com/bugs/browse/JDK-7098155) For this, we >>>>> need to split compute_generations_free_space into two routine (one >>>>> for compute_eden_space_size and the other for >>>>> compute_tenured_generation_free_space) in order to save some >>>>> overhead when we want to compute only one of the two. The next >>>>> step is to split >>>>> PSAdaptiveSizePolicy::compute_generations_free_space(), which will >>>>> be resolved in CR 8007763. >>>>> >>>>> To unify the naming, a bunch of routines are renamed here. >>>>> (1) compute_*: >>>>> compute_survivor_space_size_and_threshold() >>>>> compute_generations_free_space() = compute_eden_space_size() >>>>> + compute_tenured_generation_free_space() >>>>> (2) resize_*_gen: >>>>> resize_young_gen() >>>>> resize_tenured_gen() >>>>> (3) rename judging routine resize_geneneration() to need_resize() >>>>> to avoid ambiguity >>>>> >>>>> testing: >>>>> purely renaming; passed JPRT >>>>> >>>

5 12

RFR (S): 8014971: Minor code cleanup of the freelist management
by Bengt Rutisson 21 May '13

21 May '13

Hi all, While fixing 'JDK-7066063: CMS: "Conservation Principle" assert failed' I made some code cleanups in the code I was browsing. I didn't want to mix these changes in with the bug fix, so I'm submitting this review for those changes. Could I have a couple of reviews of this small cleanup? http://cr.openjdk.java.net/~brutisso/8014971/webrev.00/ Here is what I did: - Removed the unused constructors for FreeList and AdaptiveFreeList - Removed unnecessary NULL check after call to global operator new. The new operator already checks the allocation and exits the VM if it fails. - Removed unused old_end variable in ConcurrentMarkSweepGeneration::grow_by(). - Fixed a possible bug in CompactibleFreeListSpace::addChunkToFreeListsAtEndRecordingStats() The last fix may require some explanation. In the constructor for CompactibleFreeListSpace() we set up the _indexedFreeListParLocks array. But we only set it up if ParallelGCThreads > 0, so if we are single threaded the locks are unintialized (not even NULL, just random values). In CompactibleFreeListSpace::addChunkToFreeListsAtEndRecordingStats() we use these locks but we don't check for ParallelGCThreads > 0. We only check the chunk size to see if there is a chance that the chunk may end up in the smaller buckets. This will fail if ParallelGCThreads = 0. This bug is benign since CompactibleFreeListSpace::addChunkToFreeListsAtEndRecordingStats() is only used when we grow or shrink the generation. This is always done with os::vm_allocation_granularity() which is larger than SmallForDictionary so we will never enter the dangerous code path. However, if someone in the future either increases the value of SmallForDictionary or uses a smaller alignment for the generation sizing we might hit this problem. So, I figured it is worth leaving the code but making sure that it works properly. Thanks, Bengt

3 3

Request for review: 8015007: Incorrect print format in error message for VM cannot allocate the requested heap
by Tao Mao 21 May '13

21 May '13

Hi all, Can I have a couple of quick reviews of this trivial change? Thanks. Tao webrev: http://cr.openjdk.java.net/~tamao/8015007/webrev.00/ changeset: 1. print format: %d -> SIZE_FORMAT for type of size_t (variable total_reserved) 2. *minor: clean up error message call in check_alignment

3 2

hg: hsx/hotspot-gc/hotspot: 8014405: G1: PerRegionTable::fl_mem_size() calculates size of the free list using wrong element sizes
by bengt.rutisson＠oracle.com 21 May '13

21 May '13

Changeset: 6702da6b6082 Author: tschatzl Date: 2013-05-21 11:30 +0200 URL: http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/6702da6b6082 8014405: G1: PerRegionTable::fl_mem_size() calculates size of the free list using wrong element sizes Summary: Instead of using a simple sizeof(), ask the PerRegionTable class about its size when iterating over the free list. Reviewed-by: jwilhelm, brutisso ! src/share/vm/gc_implementation/g1/heapRegionRemSet.cpp ! src/share/vm/gc_implementation/g1/heapRegionRemSet.hpp ! src/share/vm/prims/jni.cpp

1 0

hg: hsx/hotspot-gc/hotspot: 2 new changesets
by bengt.rutisson＠oracle.com 21 May '13

21 May '13

Changeset: 28e53b8db94f Author: brutisso Date: 2013-05-21 08:50 +0200 URL: http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/28e53b8db94f 7066063: CMS: "Conservation Principle" assert failed Summary: Add call to coalBirth() in CompactibleFreeListSpace::reset() Reviewed-by: ysr, jmasa ! src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp Changeset: 5ed122fbd0ef Author: brutisso Date: 2013-05-21 10:39 +0200 URL: http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/5ed122fbd0ef Merge

1 0

CMS: "premature" minor GCs w/ ParNew
by Vladimir Ivanov 21 May '13

21 May '13

Hi, I see minor GCs occurring when young gen occupancy is very low, e.g.: [ParNew: 8446K->7381K(943744K), 0.0274200 secs] Does anybody know what provokes such "premature" GCs? There's no CMS concurrent cycle running or any other interesting GC events occurred around that moment - just a sequence of minor GCs with such outlier. Such premature minor GCs occur on a regular basis. JDK version: 6u37, 64-Bit Server VM (20.12-b01, mixed mode) Best regards, Vladimir Ivanov

4 7

hg: hsx/hotspot-gc/hotspot: 8014862: Add fast Metasapce capacity and used per MetadataType
by jon.masamitsu＠oracle.com 21 May '13

21 May '13

Changeset: 2b1a9d972fc2 Author: jmasa Date: 2013-05-20 22:34 -0700 URL: http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/2b1a9d972fc2 8014862: Add fast Metasapce capacity and used per MetadataType Reviewed-by: ehelin, stefank ! src/share/vm/memory/metaspace.cpp ! src/share/vm/memory/metaspace.hpp

1 0

RFR (S): 7066063: CMS: "Conservation Principle" assert failed
by Bengt Rutisson 20 May '13

20 May '13

Hi everyone, Could I have a couple of reviews for this small change? http://cr.openjdk.java.net/~brutisso/7066063/webrev.00/ Background : We hit this assert in the allocation statistics for CMS: assert(prevSweep() + splitBirths() + coalBirths() >= splitDeaths() + coalDeaths() + (ssize_t)count) failed: Conservation Principle The problem is in CompactibleFreeListSpace::reset(). That method calls FreeList<Chunk>::return_chunk_at_head(), which calls increment_count(). All other call sites that increment the count also updates either the split birth or coal birth statistics, but CompactibleFreeListSpace::reset() does not do that. My reproducer hits the assert every time I run it. If I add a call to coalBirth() in CompactibleFreeListSpace::reset() it never hits the assert. Here are the other calls to return_chunk_at_head and how they update the statistics: CompactibleFreeListSpace:: par_get_chunk_of_blocks() -> calls set_split_births() CompactibleFreeListSpace::returnChunkToFreeList() -> coalBirth() in CompactibleFreeListSpace::addChunkToFreeListsAtEndRecordingStats() -> coalBirth() in SweepClosure::flush_cur_free_chunk() -> split_birth() in CompactibleFreeListSpace::getChunkFromLinearAllocBlock() CompactibleFreeListSpace::splitChunkAndReturnRemainder() -> split_birth() in CompactibleFreeListSpace::split() So, all other calls than CompactibleFreeListSpace::reset() either increment the split birth or coal birth counters. When we set up the heap we eventually end up in the CompactibleFreeListSpace::addChunkToFreeListsAtEndRecordingStats() which increments the coal birth. Since CompactibleFreeListSpace::reset() is called from CompactibleFreeListSpace::reset_after_compaction() when the freed up memory from the compaction is added to the free list it seems reasonable to update the same counter as when we first set up the heap. Thus, incrementing the coal birth seem more correct than incrementing the split birth count. Thanks, Bengt

3 4

hg: hsx/hotspot-gc/hotspot: 7186737: Unable to allocate bit maps or card tables for parallel gc for the requested heap
by jon.masamitsu＠oracle.com 20 May '13

20 May '13

Changeset: 10f759898d40 Author: tamao Date: 2013-05-20 10:44 -0700 URL: http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/10f759898d40 7186737: Unable to allocate bit maps or card tables for parallel gc for the requested heap Summary: Print helpful error message when VM aborts due to inability of allocating bit maps or card tables Reviewed-by: jmasa, stefank Contributed-by: tamao <tao.mao(a)oracle.com> ! src/share/vm/gc_implementation/parallelScavenge/parMarkBitMap.cpp ! src/share/vm/gc_implementation/parallelScavenge/parMarkBitMap.hpp ! src/share/vm/gc_implementation/parallelScavenge/psParallelCompact.cpp ! src/share/vm/gc_implementation/parallelScavenge/psParallelCompact.hpp

1 0