gmane.comp.java.openjdk.hotspot.gc.devel

Re: RFR(XS): 7143858: G1: Back to back young GCs with the second GChavinga minimally sized eden

Charlie Hunt — 2012-05-23T17:17:27

Looks great John!

I knew you'd get this (annoying behavior) figured out.

charlie ...

On May 21, 2012, at 6:12 PM, John Cuthbertson wrote:

Re: RFR(XS): 7143858: G1: Back to back young GCs with the second GChaving a minimally sized eden

Bengt Rutisson — 2012-05-23T14:28:30

John,

Looks good!

Nice catch.

Bengt

22 maj 2012 kl. 01:12 skrev John Cuthbertson <john.cuthbertson-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org>:

RFR(XS): 7143858: G1: Back to back young GCs with the second GC havinga minimally sized eden

John Cuthbertson — 2012-05-21T23:12:36

Hi Everyone,

Can I have a couple of volunteers review the fix for this CR? The webrev 
can be found at: http://cr.openjdk.java.net/~johnc/7143858/webrev.0/

Description:

In some recent G1 logs we have seen some evacuation pauses that looked 
premature, i.e. the eden occupancy was much less than the target 
capacity - usually only one or two regions. These premature pauses 
always (almost immediately) followed a normal evacuation pause with 
little or no application activity in-between. Bengt's recent changes to 
display the GC cause indicated that the premature pauses were always 
GCLocker Initiated GCs.

What seemed to have been happening was that as the last left a JNI 
critical region, before it scheduled the GCLocker Initiated GC, another 
thread would attempt an allocation. The allocating thread would see that 
GCLocker was no longer active and successfully schedule the evacuation 
pause. As part of this GC operation, a mutator alloc region would get 
allocated and the object allocation request would be satisfied. After, 
the thread initating the GCLocker GC would schedule its GC and the eden 
occupancy would be fairly minimal.

Inserting a 500ms sleep just before scheduling the GCLocker initated GC 
was able to reproduce the problem with Dacapo fairly frequently.

The solution implemented in this webrev is to stall the allocating 
thread until the GCLocker Initiated GC is performed and then retry the 
allocation.

Testing: GC Test Suite and Kitchensink with the additional sleep call 
and verify that no pauses occurred during the sleep, GC Test suite and 
jprt with the additional sleep.

I have been unable to reproduce the issue with the other Hotspot 
collectors but I can't see why they wouldn't be vulnerable. I changed 
the G1 slow case allocation code to match the other collectors and still 
saw the issue.

Thanks,

JohnC

hg: hsx/hotspot-gc/hotspot: 7168848: Add test to check that humongousobject allocation path also checks the heap occupancy.

2012-05-21T16:04:11

Changeset: 9a344d88dc22
Author:    mnunez
Date:      2012-05-21 14:59 +0200
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/9a344d88dc22

7168848: Add test to check that humongous object allocation path also checks the heap occupancy.
Summary: Added test that checks humongous object allocation path also check the heap occupancy and initiate a marking cycle when / if needed.
Reviewed-by: brutisso, jwilhelm

+ test/gc/7168848/HumongousAlloc.java

hg: hsx/hotspot-gc/hotspot: 7169062: CMS: Assertion failed with-XX:+ObjectAlignmentInBytes=64

2012-05-21T05:18:40

Changeset: c92a79900986
Author:    brutisso
Date:      2012-05-17 21:53 +0200
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/c92a79900986

7169062: CMS: Assertion failed with -XX:+ObjectAlignmentInBytes=64
Summary: Removed the assert in CompactibleFreeListSpace::check_free_list_consistency() since it was too strict. Simplified CompactibleFreeListSpace::set_cms_values() to reducde the need for asserts.
Reviewed-by: jcoomes, stefank

! src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp

hg: hsx/hotspot-gc/hotspot: 9 new changesets

2012-05-19T07:30:42

Changeset: 26423ef693ac
Author:    katleman
Date:      2012-05-17 06:20 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/26423ef693ac

Added tag jdk8-b39 for changeset 96a403721094

! .hgtags

Changeset: 35e504cb49a6
Author:    collins
Date:      2012-05-11 11:30 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/35e504cb49a6

7167625: Adjustments for SE-Embedded build process
Summary: Simple change to the SE-Embedded build rules that should not affect any other OpenJDK users.
Reviewed-by: kvn, dholmes

! make/linux/makefiles/vm.make
! src/share/vm/runtime/arguments.cpp

Changeset: fada85d11d92
Author:    jprovino
Date:      2012-05-16 13:33 -0400
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/fada85d11d92

Merge


Changeset: 8f972594effc
Author:    kvn
Date:      2012-05-14 09:36 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/8f972594effc

6924259: Remove String.count/String.offset
Summary: Allow a version of String class that doesn't have count and offset fields.
Reviewed-by: never, coleenp

! src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp
! src/cpu/x86/vm/c1_LIRAssembler_x86.cpp
! src/share/vm/classfile/javaClasses.cpp
! src/share/vm/classfile/javaClasses.hpp
! src/share/vm/classfile/systemDictionary.cpp
! src/share/vm/classfile/vmSymbols.hpp
! src/share/vm/memory/dump.cpp
! src/share/vm/opto/graphKit.cpp
! src/share/vm/opto/graphKit.hpp
! src/share/vm/opto/library_call.cpp
! src/share/vm/opto/stringopts.cpp
! src/share/vm/opto/stringopts.hpp

Changeset: de0cc3dd9f10
Author:    kvn
Date:      2012-05-17 09:50 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/de0cc3dd9f10

Merge


Changeset: cdeda3fd141e
Author:    jcoomes
Date:      2012-05-18 10:27 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/cdeda3fd141e

Merge

! src/share/vm/runtime/arguments.cpp

Changeset: 14b0e07ab9a6
Author:    amurillo
Date:      2012-05-18 14:50 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/14b0e07ab9a6

Merge


Changeset: ff9decc8235d
Author:    amurillo
Date:      2012-05-18 14:50 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/ff9decc8235d

Added tag hs24-b11 for changeset 14b0e07ab9a6

! .hgtags

Changeset: 03d61caacd1e
Author:    amurillo
Date:      2012-05-18 14:57 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/03d61caacd1e

7170006: new hotspot build - hs24-b12
Reviewed-by: jcoomes

! make/hotspot_version

Re: Request for review (S): 7169062 CMS: Assertion failed with-XX:+ObjectAlignmentInBytes=64

Bengt Rutisson — 2012-05-18T08:24:02

John,

Thanks for the review!

Stefan also looked at this, so I'm all set to push.

Thanks!
Bentg

On 2012-05-18 01:07, John Coomes wrote:

hg: hsx/hotspot-gc/langtools: Added tag jdk8-b39 for changeseta9f547c218d9

2012-05-18T03:57:03

Changeset: 8b869afd2eb4
Author:    katleman
Date:      2012-05-17 06:20 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/langtools/rev/8b869afd2eb4

Added tag jdk8-b39 for changeset a9f547c218d9

! .hgtags

hg: hsx/hotspot-gc/jdk: 4 new changesets

2012-05-18T03:54:28

Changeset: c2d9166f3284
Author:    ihse
Date:      2012-05-11 08:21 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/jdk/rev/c2d9166f3284

7168208: Change use of < at > in one sed command involving paths to different character
Reviewed-by: ohair

! make/common/Release.gmk

Changeset: 8d665b69ebf1
Author:    mfang
Date:      2012-05-15 11:46 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/jdk/rev/8d665b69ebf1

7157855: jvisualvm.1 not included in binaries
Reviewed-by: katleman, thurka

! make/common/Release.gmk

Changeset: b6f529117521
Author:    katleman
Date:      2012-05-16 22:07 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/jdk/rev/b6f529117521

Merge


Changeset: 47cd90bf0f66
Author:    katleman
Date:      2012-05-17 06:20 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/jdk/rev/47cd90bf0f66

Added tag jdk8-b39 for changeset b6f529117521

! .hgtags

hg: hsx/hotspot-gc/jaxws: Added tag jdk8-b39 for changeset7f6b44fd3034

2012-05-18T03:54:14

Changeset: 09a0ddda03cb
Author:    katleman
Date:      2012-05-17 06:20 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/jaxws/rev/09a0ddda03cb

Added tag jdk8-b39 for changeset 7f6b44fd3034

! .hgtags

hg: hsx/hotspot-gc/jaxp: Added tag jdk8-b39 for changeset f95fdbe525c8

2012-05-18T03:54:01

Changeset: 9ecfdbd6aed4
Author:    katleman
Date:      2012-05-17 06:20 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/jaxp/rev/9ecfdbd6aed4

Added tag jdk8-b39 for changeset f95fdbe525c8

! .hgtags

hg: hsx/hotspot-gc/corba: Added tag jdk8-b39 for changeset785af00e2827

2012-05-18T03:53:54

Changeset: 56d030e5035f
Author:    katleman
Date:      2012-05-17 06:20 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/corba/rev/56d030e5035f

Added tag jdk8-b39 for changeset 785af00e2827

! .hgtags

hg: hsx/hotspot-gc: 4 new changesets

2012-05-18T03:53:49

Changeset: 955a3e8ed4f0
Author:    ohair
Date:      2012-05-10 08:26 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/rev/955a3e8ed4f0

7167593: Changed get_source.sh to allow for getting full oracle jdk repo forest
Reviewed-by: erikj, asaha, chegar, sla, dholmes, mbykov, coleenp

! get_source.sh
! make/scripts/hgforest.sh

Changeset: 8a4e92c10a9a
Author:    ohair
Date:      2012-05-11 17:52 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/rev/8a4e92c10a9a

7167976: Fix broken get_source.sh script
Reviewed-by: tbell

! make/scripts/hgforest.sh

Changeset: 8927dd68aee3
Author:    katleman
Date:      2012-05-16 22:06 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/rev/8927dd68aee3

Merge


Changeset: a2b2d435f1d2
Author:    katleman
Date:      2012-05-17 06:20 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/rev/a2b2d435f1d2

Added tag jdk8-b39 for changeset 8927dd68aee3

! .hgtags

Re: Request for review (S): 7169062 CMS: Assertion failed with-XX:+ObjectAlignmentInBytes=64

John Coomes — 2012-05-17T23:07:00

Looks good to me.

-John

Request for review (S): 7169062 CMS: Assertion failed with-XX:+ObjectAlignmentInBytes=64

Bengt Rutisson — 2012-05-17T20:25:39

Hi all,

Can I have a couple of reviews for this change?
http://cr.openjdk.java.net/~brutisso/7169062/webrev.00/

Background
In CompactibleFreeListSpace::check_free_list_consistency() there were 
some asserts to verify that the instance variables IndexSetStart, 
IndexSetStart and IndexSetSize were set up correctly. These asserts were 
too strong and did not handle different object alignments. The asserts 
are also not necessary to do everytime we call 
check_free_list_consistency(). It makes more sense to only check it when 
we updated the values in CompactibleFreeListSpace::set_cms_values().

I replaced the asserts in check_free_list_consistency() with two asserts 
to verify that have set the values up at all. Instead I intended to add 
some asserts to set_cms_values(), but after simplifying (at least in my 
mind) the calculations there I don't think any asserts are needed. 
Thanks Stefan for helping out with the setup code.


Testing
Ran 64 bit VM with an assert comparing the old calculation of 
MinChunkSize with the new one. Tried all values of  
ObjectAlignmentInBytes (8, 16, 32, 64, 128 and 256). Also ran the 32 bit 
VM with the assert that compared old and new value in place.

Ran JPRT (the Stockholm queue :-) ).

Coding Style
The instance variables IndexSetStride and IndexSetStart should probably 
be _index_set_stride and _index_set_start. I'll try to keep my change 
small, so I would like to avoid changing naming style etc right now.

hg: hsx/hotspot-gc/hotspot: 7166894: Add gc cause to GC logging forall collectors

2012-05-16T09:48:59

Changeset: 9d679effd28c
Author:    brutisso
Date:      2012-05-15 10:25 +0200
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/9d679effd28c

7166894: Add gc cause to GC logging for all collectors
Reviewed-by: mgerdin, johnc

! src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp
! src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp
! src/share/vm/gc_implementation/concurrentMarkSweep/vmCMSOperations.cpp
! src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
! src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp
! src/share/vm/gc_implementation/parNew/parNewGeneration.cpp
! src/share/vm/gc_implementation/parallelScavenge/psMarkSweep.cpp
! src/share/vm/gc_implementation/parallelScavenge/psParallelCompact.cpp
! src/share/vm/gc_implementation/parallelScavenge/psScavenge.cpp
! src/share/vm/gc_interface/gcCause.hpp
! src/share/vm/memory/defNewGeneration.cpp
! src/share/vm/memory/genCollectedHeap.cpp
! src/share/vm/memory/genMarkSweep.cpp
! src/share/vm/runtime/arguments.cpp
! src/share/vm/runtime/globals.hpp
! src/share/vm/runtime/java.hpp

hg: hsx/hotspot-gc/hotspot: 7169056: Add gigabyte unit toproper_unit_for_byte_size() and byte_size_in_proper_unit()

2012-05-16T07:38:27

Changeset: cdfa5139bd58
Author:    brutisso
Date:      2012-05-15 22:26 +0200
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/cdfa5139bd58

7169056: Add gigabyte unit to proper_unit_for_byte_size() and byte_size_in_proper_unit()
Reviewed-by: jwilhelm, johnc, dholmes

! src/share/vm/utilities/globalDefinitions.hpp

Re: Request for review (S): 7166894 Add gc cause to Full GC loggingfor all collectors

John Cuthbertson — 2012-05-16T00:14:48

Hi Bengt,

Looks good to me. Ship it.

JohnC

On 05/15/12 00:51, Bengt Rutisson wrote:

hg: hsx/hotspot-gc/hotspot: 7168294: G1: Some Full GCs incorrectlyreport GC cause as "No GC"

2012-05-15T19:41:02

Changeset: 1096fc5a52eb
Author:    johnc
Date:      2012-05-15 09:49 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/1096fc5a52eb

7168294: G1: Some Full GCs incorrectly report GC cause as "No GC"
Summary: GC cause was not being set by the VM_G1CollectForAllocation VM operation.
Reviewed-by: jmasa, ysr, brutisso

! src/share/vm/gc_implementation/g1/vm_operations_g1.cpp

Re: Request for review (S): 7166894 Add gc cause to Full GC loggingfor all collectors

Bengt Rutisson — 2012-05-15T07:51:34

Hi John,

Thanks for looking at this!

On 2012-05-14 21:00, John Cuthbertson wrote:

Fixed.


Of course! I keep missing that when I create new classes. Thanks for 
finding it! Fixed.


Fixed.


Good catch! I clearly removed one line too much. Added it back.

Here is an updated webrev:
http://cr.openjdk.java.net/~brutisso/7166894/webrev.06/

Thanks again for looking at this!
Bengt

Re: RFR (XXS): 7158682: G1: Handle leak when running nsk.sysdict tests

Bengt Rutisson — 2012-05-15T06:14:56

John,

Looks good! Thanks for doing this change. It should solve the memory 
leak for CMS as well.

Bengt

On 2012-05-14 21:22, John Cuthbertson wrote: