RFR(S): 8071822: [TEST_BUG] Adapt collectorPolicy internal tests to support 64K pages

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Fri Feb 20 21:15:42 UTC 2015


Thanks a lot!!

Best regards,
  Goetz.

From: Jon Masamitsu [mailto:jon.masamitsu at oracle.com]
Sent: Friday, February 20, 2015 8:21 PM
To: Lindenmaier, Goetz; 'hotspot-gc-dev at openjdk.java.net'; 'Dmitry Fazunenko'
Subject: Re: RFR(S): 8071822: [TEST_BUG] Adapt collectorPolicy internal tests to support 64K pages


On 2/20/2015 9:28 AM, Lindenmaier, Goetz wrote:
Hi Jon and Dmitry,

Thanks for looking at this!

renamed to over_size and removed ;m:
http://cr.openjdk.java.net/~goetz/webrevs/8071822-jtregColPol/webrev.04/<http://cr.openjdk.java.net/%7Egoetz/webrevs/8071822-jtregColPol/webrev.04/>

I please need a sponsor.

I'll sponsor.

Jon



Best regards,
  Goetz.





From: Jon Masamitsu [mailto:jon.masamitsu at oracle.com]
Sent: Friday, February 20, 2015 5:21 PM
To: Lindenmaier, Goetz; hotspot-gc-dev at openjdk.java.net<mailto:hotspot-gc-dev at openjdk.java.net>
Subject: Re: RFR(S): 8071822: [TEST_BUG] Adapt collectorPolicy internal tests to support 64K pages


On 2/18/2015 1:52 AM, Lindenmaier, Goetz wrote:
Hi Jon,

> Please revert to the original order of these lines
> Could you define a variable to hold the heap alignment?
Done.

Thanks.




> Should the 30*M be aligned?
It works as is, so I didn't want to change it. Also, OldSize is not aligned to heap_alignment.
It ends up being 10M, while heap_alignment is 32M on the machine with 64K pages.

Ok.




> What is the 20*m?
The tests wants MaxNewSize + OldSize > MaxHeapSize, see comment.  Before, the
flag_value was just chosen accordingly (170M).  Now I add +20M  to MaxNewSize
to assure this, as I have to  compute MaxNewSize.
Is 'overlap' a good name?

Would it be accurate to change overlay (doesn't seem quite) right to
over_size and to change the comment to

989     // We intentionally set MaxNewSize + OldSize > MaxHeapSize (see over_size).

Either way (overlap or over_size) is acceptable so you can decide.
Consider this reviewed.  I don't want to delay you even longer than I
have.

Jon




Here's the new webrev:
http://cr.openjdk.java.net/~goetz/webrevs/8071822-jtregColPol/webrev.03/<http://cr.openjdk.java.net/%7Egoetz/webrevs/8071822-jtregColPol/webrev.03/>

Thanks and best regards,
  Goetz.

From: Jon Masamitsu [mailto:jon.masamitsu at oracle.com]
Sent: Dienstag, 17. Februar 2015 19:57
To: Lindenmaier, Goetz; hotspot-gc-dev at openjdk.java.net<mailto:hotspot-gc-dev at openjdk.java.net>
Subject: Re: RFR(S): 8071822: [TEST_BUG] Adapt collectorPolicy internal tests to support 64K pages


On 2/16/15 8:33 AM, Lindenmaier, Goetz wrote:
Hi Jon,

thanks for looking at this change!

It's not that easy as  just aligning to vm_page_size, but I can use
CollectorPolicy::compute_heap_alignment():
http://cr.openjdk.java.net/~goetz/webrevs/8071822-jtregColPol/webrev.02/<http://cr.openjdk.java.net/%7Egoetz/webrevs/8071822-jtregColPol/webrev.02/>

Please revert to the original order of these lines unless it
makes a difference.

 990     set_basic_flag_values();

 991     flag_value = 30 * M;

Could you define a variable to hold the heap alignment?

size_t heap_alignment =  CollectorPolicy::compute_heap_alignment()

just to make it a little more readable.

Should the 30*M be aligned?

What is the 20*m?  Could you assign it to a variable with a descriptive
name?

Jon




Best regards,
  Goetz.




From: hotspot-gc-dev [mailto:hotspot-gc-dev-bounces at openjdk.java.net] On Behalf Of Jon Masamitsu
Sent: Donnerstag, 12. Februar 2015 00:35
To: hotspot-gc-dev at openjdk.java.net<mailto:hotspot-gc-dev at openjdk.java.net>
Subject: Re: RFR(S): 8071822: [TEST_BUG] Adapt collectorPolicy internal tests to support 64K pages

Goetz,

Could you have used page alignment to determine the
new expected sizes?

For example, instead of

 987     // Calculate what we expect the flag to be.

 988     flag_value = adapted_InitialHeapSize - MaxNewSize;

 989     verify_old_initial(flag_value);
expected_old_initial = align_size_up(flag_value, os::vm_page_size())
verify_old_initial(expected_old_initial)

Jon
On 2/10/2015 7:43 AM, Lindenmaier, Goetz wrote:
Hi,

could someone please have a look at this change?
It's really not that big.

GC-folks maybe?

Thanks,
  Goetz.

From: Lindenmaier, Goetz
Sent: Freitag, 30. Januar 2015 10:36
To: hotspot-dev Source Developers
Subject: RFR(S): 8071822: [TEST_BUG] Adapt collectorPolicy internal tests to support 64K pages

Hi,

this fixes a final problem with 64-bit page size in the jtreg tests.
It addresses the internal VM tests of the collector policies.

Those tests set a row of fixed flag values, and one special value to test.
Then they call the heap ergonomics and check the expected result of the
special value.

With 64K page size, the heap ergonomics adapt more values, breaking the tests.
Wrt. the adapted flag values the tests are correct though.

This change makes the expected values depend on the adapted flags, and the
tests pass. They only depend on adapted flags that are not subject of the
corresponding test.

Please review this test.  I please need a sponsor.

http://cr.openjdk.java.net/~goetz/webrevs/8071822-jtregColPol/webrev.01/<http://cr.openjdk.java.net/%7Egoetz/webrevs/8071822-jtregColPol/webrev.01/src/share/vm/memory/collectorPolicy.cpp.udiff.html>
https://bugs.openjdk.java.net/browse/JDK-8071822


Best regards,
  Goetz.





-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/attachments/20150220/17435c4e/attachment.html>


More information about the hotspot-gc-dev mailing list