RFR (S): 8014971: Minor code cleanup of the freelist management

Jesper Wilhelmsson jesper.wilhelmsson at oracle.com
Tue May 21 15:25:44 UTC 2013


Looks good.
Ship it!
/Jesper

Bengt Rutisson skrev 21/5/13 3:35 PM:
>
> Hi all,
>
> While fixing 'JDK-7066063: CMS: "Conservation Principle" assert failed' I made
> some code cleanups in the code I was browsing. I didn't want to mix these
> changes in with the bug fix, so I'm submitting this review for those changes.
> Could I have a couple of reviews of this small cleanup?
>
> http://cr.openjdk.java.net/~brutisso/8014971/webrev.00/
>
> Here is what I did:
>
> - Removed the unused constructors for FreeList and AdaptiveFreeList
> - Removed unnecessary NULL check after call to global operator new. The new
> operator already checks the allocation and exits the VM if it fails.
> - Removed unused old_end variable in ConcurrentMarkSweepGeneration::grow_by().
> - Fixed a possible bug in
> CompactibleFreeListSpace::addChunkToFreeListsAtEndRecordingStats()
>
> The last fix may require some explanation. In the constructor for
> CompactibleFreeListSpace() we set up the _indexedFreeListParLocks array. But we
> only set it up if ParallelGCThreads > 0, so if we are single threaded the locks
> are unintialized (not even NULL, just random values). In
> CompactibleFreeListSpace::addChunkToFreeListsAtEndRecordingStats() we use these
> locks but we don't check for ParallelGCThreads > 0. We only check the chunk size
> to see if there is a chance that the chunk may end up in the smaller buckets.
> This will fail if ParallelGCThreads = 0.
>
> This bug is benign since
> CompactibleFreeListSpace::addChunkToFreeListsAtEndRecordingStats() is only used
> when we grow or shrink the generation. This is always done with
> os::vm_allocation_granularity() which is larger than SmallForDictionary so we
> will never enter the dangerous code path. However, if someone in the future
> either increases the value of SmallForDictionary or uses a smaller alignment for
> the generation sizing we might hit this problem. So, I figured it is worth
> leaving the code but making sure that it works properly.
>
> Thanks,
> Bengt



More information about the hotspot-gc-dev mailing list