<html>
<head>
<meta http-equiv="content-type" content="text/html; charset=ISO-8859-1">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
Hi everyone,<br>
<br>
Could I have a couple of reviews for this small change?<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~brutisso/7066063/webrev.00/">http://cr.openjdk.java.net/~brutisso/7066063/webrev.00/</a><br>
<br>
Background :<br>
<br>
We hit this assert in the allocation statistics for CMS:<br>
<br>
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
assert(prevSweep() + splitBirths() + coalBirths() >=
splitDeaths() + coalDeaths() + (ssize_t)count) failed: Conservation
Principle
<br>
<br>
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
The problem is in CompactibleFreeListSpace::reset(). That method
calls FreeList<Chunk>::return_chunk_at_head(), which calls
increment_count().
<br>
<br>
All other call sites that increment the count also updates either
the split birth or coal birth statistics, but
CompactibleFreeListSpace::reset() does not do that. <br>
<br>
My reproducer hits the assert every time I run it. If I add a call
to coalBirth() in CompactibleFreeListSpace::reset() it never hits
the assert.
<br>
<br>
Here are the other calls to return_chunk_at_head and how they update
the statistics:
<br>
<br>
CompactibleFreeListSpace:: par_get_chunk_of_blocks()
<br>
-> calls set_split_births()
<br>
<br>
CompactibleFreeListSpace::returnChunkToFreeList()
<br>
-> coalBirth() in
CompactibleFreeListSpace::addChunkToFreeListsAtEndRecordingStats()
<br>
-> coalBirth() in SweepClosure::flush_cur_free_chunk()
<br>
-> split_birth() in
CompactibleFreeListSpace::getChunkFromLinearAllocBlock()
<br>
<br>
CompactibleFreeListSpace::splitChunkAndReturnRemainder()
<br>
-> split_birth() in CompactibleFreeListSpace::split()
<br>
<br>
<br>
So, all other calls than CompactibleFreeListSpace::reset() either
increment the split birth or coal birth counters. When we set up the
heap we eventually end up in the
CompactibleFreeListSpace::addChunkToFreeListsAtEndRecordingStats()
which increments the coal birth.
<br>
<br>
Since CompactibleFreeListSpace::reset() is called from
CompactibleFreeListSpace::reset_after_compaction() when the freed up
memory from the compaction is added to the free list it seems
reasonable to update the same counter as when we first set up the
heap. Thus, incrementing the coal birth seem more correct than
incrementing the split birth count.
<br>
<br>
Thanks,<br>
Bengt<br>
</body>
</html>