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

[Thomas Schatzl]

On Tue, 2013-05-21 at 15:35 +0200, Bengt Rutisson wrote:
> 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/
> 

Some further minor changes that could be done:

- another commented out line in freeList.hpp:179 which could be removed
- copyright dates could be adapted ;)

> 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.

Looks good otherwise.

Thanks,
  Thomas