RFR: 8227168: Cleanup usage of NEW_C_HEAP_ARRAY
leo.korinth at oracle.com
Wed Sep 4 14:44:00 UTC 2019
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
>>> 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
>>> 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