Request for review - 8011268: NPG: Free unused VirtualSpaceNodes

[Coleen Phillimore]

Jon,
This looks good.   I like the placement new.
Coleen

On 04/16/2013 03:32 PM, Jon Masamitsu wrote:
>
>
> On 04/16/13 07:46, Mikael Gerdin wrote:
>> 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:
> You're right.  This is better and better.
>
>>   Metachunk* result = ::new (chunk_limit) Metachunk(chunk_word_size, 
>> this);
>> though you might need to "#include <new>" for that.
>
> The solaris and linux builds didn't complain about needing the 
> include.  I'll
> go try windows.
>
>>
>>
>> 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? :)
>
> Deleted the comment.
>
>>
>>
>> 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.
>
> I need to merge and do a few tests.  That will give Coleen a chance to 
> look at it.
> Then I can push.
>
> Thanks for the review.
>
> Jon
>>
>> /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
>>>