RFR (M/L): 8010722 assert: failed: heap size is too big for compressed oops (possibly CR 8009778)

[Leonid Mesnik]

Changed to internal alias since here are mostly our testing process are 
discussed.

I've not found where you check that you run test on 64-bit VM only. Do 
have this check?

Also it is better to set GC as command-line options and read them from 
system property.
The command-line options are part of test metadata which is available to 
jtreg.

   32  * @run main/othervm TestUseCompressedOopsErgo -XX:+UseG1GC
   33  * @run main/othervm TestUseCompressedOopsErgo -XX:+UseParallelOldGC
   34  * @run main/othervm TestUseCompressedOopsErgo -XX:+UseConcMarkSweepGC
   35  * @run main/othervm TestUseCompressedOopsErgo -XX:+UseSerialGC


The ParallelOld is Parallel now. Also there is
-XX:+UseParallelGC -XX:-UseParallelOldGC
tested combination.

You could just use

* @run main/othervm TestUseCompressedOopsErgo

and run tested VM with options set by javaoption. It is a common 
approach for most of tests.


Leonid
On 05/14/2013 06:37 PM, Thomas Schatzl wrote:
> Hi all,
>
>    can I have a review for the following change?
>
> In argument processing related to ergonomically determining compressed
> oops use, there were a few use-before-set issues leading to crashes that
> this fix tries to resolve.
>
> bugs.sun.com
> http://bugs.sun.com/view_bug.do?bug_id=8010722
>
> JIRA:
> https://jbs.oracle.com/bugs/browse/JDK-8010722
>
> webrev:
> http://cr.openjdk.java.net/~tschatzl/8010722/webrev/
>
> testing:
> jprt, test cases
>
> Here's a walkthrough of the changes:
>
> As mentioned, the cause of this CR is that ergonomics for determining
> the maximum java heap size usable for compressed oops uses variables
> that are later changed ergonomically.
>
> It is best to look at the changes beginning from
> Arguments::set_ergonomics_flags(): the idea of this change is to avoid
> later overflow, so the change tries to conservatively estimate sizes of
> the non-java heap parts. The complication is that not even the later
> effective alignment of these heap parts has been determined at this
> point.
>
> So the change first calculates the maximum possible heap alignment by
> calling set_max_heap_alignment(); this size is influenced by OS page
> sizes, so the change needs to initialize large pages by calling
> os::large_page_init() in Arguments::parse(), before the call to
> set_ergonomics_flags(). The maximum possible alignment is then
> calculated by asking the active GC for its maximum alignment, as at this
> point the GC has already been determined, the maximum page size, and
> other requirements, like alignment for card table size etc.
>
> Now the code can calculate the conservative estimate for actual maximum
> heap for compressed oops used in set_use_compressed_oops(), by
> subtracting the conservatively aligned sizes of the other heap parts.
> (In Arguments::max_heap_for_compressed_oops()) The result is the maximum
> possible heap that can use compressed oops, minus the aligned metaspace
> size, minus the aligned null page size.
>
> There is another circular dependency problem here, the metaspace size itself is later ergonomically sized; the change fixes this problem by anticipating that in Arguments::max_heap_for_compressed_oops(), using the same predicate for determining whether to ergonomically size it or not [this is CR8009778 I think].
>
> The other changes are straightforward: the os_* changes result from that
> large page initialization must be done earlier now; the changes in the
> collectors themselves are simply about providing the collector's maximum
> alignment. The change in Universe::reserve_heap() contains one assertion that checks whether the conservative estimate for alignment has been conservative enough earlier.
>
> The test case tests test cases from the CR that work now, and additional
> border cases related to ergonomically deciding heap size for compressed
> oops.
>
> One side effect of this change is that the ergonomically determined heap size is slighly smaller now (so that it always fits :).
>
> Thanks,
>    Thomas
>
>
>
>
>


-- 
Leonid Mesnik
JVM SQE