RFR: 8202634: Metaspace: simplify SpaceManager lists

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Tue May 8 15:53:44 UTC 2018


This change looks great.  Reviewed.

On 5/7/18 11:29 AM, Thomas Stüfe wrote:
> Hi all,
>
> may I please have reviews for this small simplification.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8202634
> webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8202634-simplify-spacemanager/webrev.00/webrev/
>
> This replaces the four separate in-use-chunk-lists in SpaceManager
> with a single one. The reasoning behind this is that we do not need to
> keep the chunks sorted by chunk size, a single list containing all
> chunks of all sizes in use by this SpaceManager is enough, and it
> simplifies the coding.
>
> There is only one place where we really did use the former
> by-chunktype lists - that is to count how many chunks per chunk type
> there are.
>
> However, arguably that is better done with running counters, so this
> is what I did. The added benefit is that in
> SpaceManager::calc_chunk_size() we do not need to walk the lists
> anymore to sum up chunks.
>
> Changes in detail:
>
> - removed the "ChunkIndex" argument in
> ChunkManager::return_single_chunk() and
> Chunkmanager::return_chunk_list() - that argument was unnecessary,
> since the Metachunk we return knows its index.

good.
>
> - Unfolded SpaceManager::initialize into Spacemanager constructor (I
> prefer initializing members in ctor init lists)

I agree!
>
> - replaces SpaceManager::sum_count_chunks_in_use() with running counters.
>
> - I dumbed down the logging inside ChunkManager::return_chunk_list()
> somewhat. Should anyone miss the former details, I can reinstate it.

I don't think we'll miss the implementation details in the logging. 
Knowing how much space is returned seems most useful.

thanks,
Coleen
>
> Thank you,
>
> Thomas



More information about the hotspot-runtime-dev mailing list