RFR: JDK-8016309 - assert(eden_size > 0 && survivor_size > 0) failed

Tue Oct 22 10:18:22 UTC 2013

Jesper,

On Tuesday 22 October 2013 08.57.53 Jesper Wilhelmsson wrote:
> Hi,
> 
> I managed to break out a few more parts from the main bugfix that can be
> pushed separately. The bugfix patch is still fairly large but I hope that
> it is clean enough to be reviewable at this point.
> 
> Parts broken out:
> 
> [1] Remove unnecessary code in GenRemSet
>      https://bugs.openjdk.java.net/browse/JDK-8026851
>      webrev: http://cr.openjdk.java.net/~jwilhelm/8026851/

Code change looks good.

> 
> [2] Use restricted_align_down in collector policy code
>      https://bugs.openjdk.java.net/browse/JDK-8026852
>      webrev: http://cr.openjdk.java.net/~jwilhelm/8026852/

Code change looks good.

/Mikael

> 
> [3] Prepare GC code for collector policy regression fix
>      https://bugs.openjdk.java.net/browse/JDK-8026853
>      webrev: http://cr.openjdk.java.net/~jwilhelm/8026853/
> 
> The remaining bugfix:
> 
> [4] assert(eden_size > 0 && survivor_size > 0) failed: just checking
>      https://bugs.openjdk.java.net/browse/JDK-8016309
>      webrev: http://cr.openjdk.java.net/~jwilhelm/8016309/Bugfix/webrev.2/
> 
> Thanks!
> /Jesper
> 
> Jesper Wilhelmsson skrev 3/10/13 1:33 PM:
> > Hi,
> > 
> > I've got the necessary reviews for the cleanup part so I will start
> > pushing it. It has been split into smaller pieces and a couple of new
> > RFEs have been created for this:
> > 
> > JDK-8025852 - Remove unnecessary setters in collector policy classes
> > JDK-8025853 - Remove unnecessary uses of GenerationSizer
> > JDK-8025854 - Use "young gen" instead of "eden"
> > JDK-8025855 - Simplify GenRemSet code slightly
> > JDK-8025856 - Fix typos in the GC code
> > 
> > These are all minor cleanups that happened as a result of the major
> > cleanup of the collector policy classes. There are a few cleanups in the
> > big cleanup patch that are not covered by these RFEs. Another RFE may be
> > created to cover them.
> > 
> > Thanks,
> > /Jesper
> > 
> > Jesper Wilhelmsson skrev 30/9/13 10:23 PM:
> >> Hi Thomas,
> >> 
> >> Thanks for looking at this!
> >> 
> >> I brought the repository up to date and have a new webrev including your
> >> suggestions here:
> >> 
> >> http://cr.openjdk.java.net/~jwilhelm/8016309/Cleanup/webrev.1/
> >> http://cr.openjdk.java.net/~jwilhelm/8016309/Bugfix/webrev.1/
> >> 
> >> Comments inline.
> >> 
> >> Thomas Schatzl skrev 27/9/13 1:29 PM:
> >>> Hi,
> >>> 
> >>> On Thu, 2013-09-19 at 23:38 +0200, Jesper Wilhelmsson wrote:
> >>>> Hi,
> >>>> 
> >>>> Could I have a couple of reviews of this cleanup/bugfix.
> >>>> 
> >>>> The bug I intended to fix is JDK-8016309 [1] in which the eden size
> >>>> could
> >>>> end up
> >>>> with a zero size due to minimum alignment and a too small young gen
> >>>> size.
> >>>> The alignment was increased in the fix for JDK-6725714 [2] which caused
> >>>> this bug.
> >>>> 
> >>>> Fixing this in the current collector policies was quite messy.
> >>>> 
> >>>> Attempts to fix this issue is actually already implemented in two
> >>>> different
> >>>> places,
> >>>> none of them results in a properly sized eden. Since Thomas had a patch
> >>>> to cleanup the collector policies to fix JDK-7057939 [3], I based my
> >>>> bugfix on his patch
> >>>> instead of fighting with the mess in the current collector policies. As
> >>>> Thomas wasn't actively working on finishing his cleanup patch I also
> >>>> continued that work to finish it.
> >>> 
> >>> Thanks a lot for continuing the work!
> >> 
> >> Thanks for starting it ;-)
> >> 
> >>>> This cleanup fixes both bugs (JDK-8016309 and JDK-7057939). To extract
> >>>> them to different patches would be too much work and result in an
> >>>> intermediate state between the two patches where the ergonomics
> >>>> wouldn't work properly. I have however split the webrev into two
> >>>> parts:
> >>>> 
> >>>> The first patch contains cosmetic changes (name changes, being
> >>>> consequent in using local instance variables vs using getters/setters,
> >>>> fixing typos in comments etc.).
> >>>> 
> >>>> http://cr.openjdk.java.net/~jwilhelm/8016309/Cleanup/webrev/
> >>>> 
> >>>> The second patch contains the major part of the cleanup work in the
> >>>> collector policies. The goal has been to make the code easier to
> >>>> follow and pull policy decisions that had leaked into other code back
> >>>> to the collector policy classes.
> >>>> 
> >>>> http://cr.openjdk.java.net/~jwilhelm/8016309/Bugfix/webrev/
> >>>> 
> >>>> Please note that the default value of NewSize has changed from 1M to
> >>>> 2M. Since the minimum alignment was increased to 512K, the minimal
> >>>> possible young gen size
> >>> 
> >>> ... on 64 bit machines, 256k on 32 bit (previously 64k for at least
> >>> serial gc I guess)?
> >>> 
> >>> Shouldn't it be possible to still have a MaxHeapSize (or did you really
> >>> mean NewSize here?) of 1M for 32bit (I think this has been the previous
> >>> limit), i.e. each of eden, from, to and old gen a 256k "page"?
> >> 
> >> I did mean NewSize. The MaxNewSize defaults to max_uintx and is later set
> >> ergonomically. I didn't want to make the NewSize default value platform
> >> dependent, but 2M may be a bit big on 32bit platforms..?  I'll check with
> >> embedded to see how they feel about this.
> >> 
> >>>> increased to 1.5M. Having a smaller default would be misleading as the
> >>>> ergonomics would increase it every time.
> >>> 
> >>> I think we should talk to the embedded people about this, but I think
> >>> this 1.5M is a limitation for 64 bit only.
> >> 
> >> Yes.
> >> 
> >>>> New tests has been added and all existing jtreg arguments tests has
> >>>> been
> >>>> used to
> >>>> verify that heap sizing ergonomics behaves as expected.
> >>>> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20131022/76b35be2/attachment.htm>