Request for review - 8011268: NPG: Free unused VirtualSpaceNodes

Mikael Gerdin mikael.gerdin at oracle.com
Tue Apr 16 14:46:37 UTC 2013


Jon,

On 2013-04-16 06:46, Jon Masamitsu wrote:
> This version incorporates the code review comments and fixes
> some bugs uncovered by the changes.
>
> http://cr.openjdk.java.net/~jmasa/8011268/webrev.01/

I like the usage of placement new, but I don't think you need to add it 
to _ValueObj and Metachunk, you could just have used the global 
placement new instead:
   Metachunk* result = ::new (chunk_limit) Metachunk(chunk_word_size, this);
though you might need to "#include <new>" for that.


VirtualSpaceList::purge has some tabs in it.

The comment in purge:
1000       // Will this work if the entire Virtualspace is used?
Are you going to remove that when you've convinced yourself if it will 
work or not? :)


Besides these small comments I think this is good.

>
> This version does not free the current VirtualSpaceNode.
> I tried it but backed it out to simply the code while debugging.  I
> will try it again later (but not with this set of changes).
>
> This is a webrev based on the changes for 8011173.
>
> http://cr.openjdk.java.net/~jmasa/8011173/webrev.00/

Are you going to push this change? It looks like you've got enough 
reviews to push it.

/Mikael

>
> Jon
>
> On 4/3/2013 9:00 PM, Jon Masamitsu wrote:
>> After class unloading deallocate any VirtualSpaceNodes not being used.
>>
>> When the classes for a dead class loader are unloaded, all the Metachunks
>> that had been used by that class loader are put on the Metachunk
>> free list for later reuse.   If all the Metachunks allocated out of a
>> VirtualSpaceNode are on the Metachunk free list, remove the Metachunks
>>  from the free list and deallocate the VirtualSpaceNode.
>>
>> The constructor for VirtualSpaceNode was moved to be below the
>> definition of
>> SpaceManager.
>>
>> http://cr.openjdk.java.net/~jmasa/8011268/webrev.00/
>>
>> Thanks.
>>
>> Jon
>



More information about the hotspot-gc-dev mailing list