RFR: 8227168: Cleanup usage of NEW_C_HEAP_ARRAY

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Fri Sep 6 13:15:58 UTC 2019


Can you send a webrev of the final version of this change?
thanks,
Coleen


On 9/4/19 10:44 AM, Leo Korinth wrote:
>
>
> On 04/09/2019 15:51, coleen.phillimore at oracle.com wrote:
>>
>>
>> On 9/4/19 7:16 AM, Leo Korinth wrote:
>>>
>>>
>>> On 03/09/2019 20:07, Kim Barrett wrote:
>>>>> On Sep 3, 2019, at 1:19 PM, coleen.phillimore at oracle.com wrote:
>>>>> On 9/3/19 12:12 PM, Leo Korinth wrote:
>>>>>> I have tried to use FREE_C_HEAP_OBJ correctly, and I did revert 
>>>>>> one usage of NEW_C_HEAP_OBJ to NEW_C_HEAP_ARRAY as it seemed to 
>>>>>> be able to also be an array type when allocated in another place.
>>>>>
>>>>> […]  But I don't agree.  I don't think NEW_C_HEAP_ARRAY should be 
>>>>> used to allocate and FREE_C_HEAP_OBJ should be used to deallocate. 
>>>>> That's worse.
>>>>
>>>> That wasn’t what was suggested.  The suggestion was to always be 
>>>> consistent with the pairing,
>>>> e.g. use NEW_C_HEAP_ARRAY and FREE_C_HEAP_ARRAY for an object, or 
>>>> use NEW_C_HEAP_OBJ
>>>> and FREE_C_HEAP_OBJ, but never mix the pairings for a given object. 
>>>> I think that’s what the
>>>> updated change is doing.
>>>
>>> That was what I tried to do, your explanation is much better than 
>>> mine, thanks Kim.
>>>>
>>>>> Actually, I think neither NEW_C_HEAP_OBJ or FREE_C_HEAP_OBJ should 
>>>>> exist.  They should just use new Type(), where Type is inherited 
>>>>> from CHeapObj<mtAppropriate>
>>> It is my goal to remove /all/ allocation macros, but I could not fit 
>>> it into this change ;-)
>>
>> How are you going to remove the NEW_C_HEAP_ARRAY macros?  Is that 
>> somewhere in this thread?  Everything got cut off.  Can you put all 
>> this in the CR?
>
> I am looking into replacing them with operator new, but I would like 
> to see that I have a working sane version before proposing anything.
>
> It is not related to this in any way. I feel these changes improve the 
> current situation, even though NEW_C_HEAP_OBJ might not be the best 
> solution, it is still /better/ than faking allocation of an array. I 
> also believe it is better to remove bad usage before it is copied than 
> doing nothing because the improvement is not perfect.
>
>> What is the latest version of this patch?
>
> I have no working code for removing those macros, but I am looking 
> into it. Hopefully it will work, but lets not waste time on my ideas.
>
> Thanks,
> Leo
>
>>
>> Thanks,
>> Coleen
>>
>>
>>>
>>> Thanks, Leo
>>>>
>>>> I was expecting that to have widespread impact, but it seems there 
>>>> are far fewer uses of NEW_C_HEAP_OBJ
>>>> than I was expecting.  But I suggest that doing anything about that 
>>>> is out of scope for this change.
>>>>
>>>> I’m also not really fond of the allocation base class approach; 
>>>> C++11 allows some alternatives.
>>>>
>>



More information about the hotspot-dev mailing list