Request for review - 8011268: NPG: Free unused VirtualSpaceNodes

[Jon Masamitsu]

Thanks.

Jon

On 4/16/2013 12:58 PM, Coleen Phillimore wrote:
>
> 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
>>>>
>