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

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 
MinChunkS

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/genMarkSw

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:

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

2012-05-15T06:04:40

Changeset: 78a1b285cda8
Author:    mikael
Date:      2012-05-15 00:56 +0200
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/78a1b285cda8

7158457: division by zero in adaptiveweightedaverage
Summary: Add ceiling to AdaptiveWeightedAverage
Reviewed-by: ysr, iveresov

! src/share/vm/gc_implementation/shared/gcUtil.cpp
! src/share/vm/gc_implementation/shared/gcUtil.hpp

Changeset: 33e366609904
Author:    johnc
Date:      2012-05-14 21:07 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/33e366609904

Merge

hg: hsx/hotspot-gc/hotspot: 7161545: G1: Minor cleanups to the G1logging

2012-05-15T04:03:52

Changeset: 3a22b77e755a
Author:    brutisso
Date:      2012-05-14 17:32 +0200
URL:       http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/3a22b77e755a

7161545: G1: Minor cleanups to the G1 logging
Summary: Rename "to-space-overflow" to "to-space-exhausted", Introduce one decimal point in the size format, Add Sum to the aggregate and re-order the entries, Add number of GC workers to the log output
Reviewed-by: johnc, jwilhelm

! src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
! src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp
! src/share/vm/gc_implementation/g1/g1CollectorPolicy.hpp
! src/share/vm/utilities/globalDefinitions.hpp

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

John Cuthbertson — 2012-05-14T19:22:58

Hi Everyone,

I have a new webrev based upon a suggestion from Bengt and Mikael Gerdin 
which can be found here: http://cr.openjdk.java.net/~johnc/7158682/webrev.1/

Testing:
* GC test suite with G1 and CM (both with and without biased locking 
enabled).

JohnC

On 04/25/12 11:08, John Cuthbertson wrote:

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

John Cuthbertson — 2012-05-14T19:00:10

Hi Bengt,

Overall this looks good to me, but I do have a couple of minor comments 
and questions....

arguments.cpp - Typo < at > line 3096

gcCause.hpp - fields in the new class should have a leading underscore.

g1CollectedHeap.cpp - < at > line 3600. IMO using a local to hold the GC 
string and pass that into the TraceTime constructor would improve the 
readability significantly.

g1CollectorPolicy.cpp - < at > line 888. If you have a log level == finer, 
where is the print of the date/timestamp prefix now?

Other than that, it looks good?

JohnC

On 05/14/12 00:46, Bengt Rutisson wrote:

Re: Request for review (S): 7161545: G1: Minor cleanups to the G1logging

Bengt Rutisson — 2012-05-14T11:58:00

Thanks Jesper and John! All set to pus this now.

Bengt

On 2012-05-14 13:54, Jesper Wilhelmsson wrote:

Re: Request for review (S): 7161545: G1: Minor cleanups to the G1logging

Jesper Wilhelmsson — 2012-05-14T11:54:31

Looks good. Ship it!
/Jesper

On 2012-05-14 13:32, Bengt Rutisson wrote: