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

Fri Feb 20 19:21:11 UTC 2015

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
> *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: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150220/ff0a8eb3/attachment.htm>