Request for review (small) 8026303: CMS: JVM intermittently crashes with "FreeList of size 258 violates Con,servation Principle" assert

[Jon Masamitsu]

On 11/23/13 2:58 PM, Srinivas Ramakrishna wrote:
>
>
>
> On Fri, Nov 22, 2013 at 11:01 PM, Jon Masamitsu 
> <jon.masamitsu at oracle.com <mailto:jon.masamitsu at oracle.com>> wrote:
>
>     I'm withdrawing this review request until further notice.
>
>
>
> Jon, I know you withdrew to give this further thought, but IIRC, the 
> thing to do here might be
> to record the return as a split birth, rather than to fix up the split 
> death.
> At least that was my mental model for the demand tracking.
Missed this somehow in my mail box.

I agree it would be nice to keep the census update near the
code that removed the chunk from the dictionary but I was
confused about what to do.

I wanted to undo a split-death so adding a coal-birth seemed
like the thing to do but at the point at which the chunk is
being added back, putting in a coal-birth looked wrong
because there really wasn't any coalescing being done.  I could
see myself coming back to that code next week and
wondering why the heck I was adding a coal-birth.  And
when I thought about it, the split-death wasn't right either
since I didn't really know what went on in get_chunk() for
an "atLeast" policy.  In the end I decided two wrongs
don't make a right :-), so did the census update when
I new what was actually being done.  It seemed simpler
to me.

If I was going to fix it, I would add a method
get_chunk_and_update_census() that would
know how to correctly update the census.

Jon

>
> Will look at yr changes when you provide a new webrev. At this time I 
> only read yr descripition below, but
> did not look at the webrev.
>
> thanks!
> -- ramki
>
>
>
>     On 11/22/2013 5:54 PM, Jon Masamitsu wrote:
>
>         8026303: CMS: JVM intermittently crashes with "FreeList of
>         size 258 violates Con
>         servation Principle" assert
>
>         CompactibleFreeListSpace:: par_get_chunk_of_blocks() replenishes
>         the free list of a given size by splitting a larger chunk. The
>         code
>         searched for a block that was large enough to split. If a
>         large enough
>         chunk was found it was removed from the dictionary and a split
>         death
>         was recorded. If the remainder after splitting would be too
>         small, that
>         block was returned to the dictionary but forgot to fix the
>         split death
>         accounting
>
>         The fix was to move the split death accounting to the point where
>         it was known that the chunk would not be put back into the
>         dictionary.
>         There was also code moved which did the accounting for the
>         _unallocated_block (updated it to account for the allocation which
>         could change _unallocated_block).
>
>         The fix
>
>         http://cr.openjdk.java.net/~jmasa/8026303/webrev.00/
>         <http://cr.openjdk.java.net/%7Ejmasa/8026303/webrev.00/>
>
>         A small amount of code refactoring was done and is in
>         a second webrev (along with the fix above).  Both changes
>         will be put back together.
>
>         http://cr.openjdk.java.net/~jmasa/8026303/webrev_cleanup.00/
>         <http://cr.openjdk.java.net/%7Ejmasa/8026303/webrev_cleanup.00/>
>
>         Thanks.
>
>         Jon
>
>
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20131203/193a0bc8/attachment.htm>