RFR: 8227168: Cleanup usage of NEW_C_HEAP_ARRAY

Leo Korinth leo.korinth at oracle.com
Tue Sep 3 16:12:45 UTC 2019

On 03/09/2019 01:46, Kim Barrett wrote:
>> On Sep 2, 2019, at 5:59 PM, David Holmes <david.holmes at oracle.com> wrote:
>> On 2/09/2019 10:23 pm, Leo Korinth wrote:
>>> Hi!
>>> After I got caught doing an unnecessary check on the return value of NEW_C_HEAP_ARRAY (a mistake that I copied) I thought it would be good to do a cleanup in the sources so that others would not fall into this trap. This is the result.
>>> I have removed some places where the VM will be shut down after NEW_C_HEAP_ARRAY returns NULL (it never does return NULL, it does instead exit). I have also removed lots of unnecessary casts, that might hide bugs.
>> Those changes all seem fine, as are the perfMemory changes.
>> My only issue is with the places where you can changed allocation of a NEW_C_HEAP_ARRAY of size 1 to a NEW_C_HEAP_OBJ. They are still freed with FREE_C_HEAP_ARRAY which looks odd even though correct (in that it's all os::free under the covers anyway). I'm not sure this change is worth the potential confusion it may cause.
> I noticed that too, and then forgot about it.  I agree that’s not right.  I think those places
> should be changed to use FREE_C_HEAP_OBJ for the deallocation.

Thank you for finding this. I actually considered implementing the macro 
FREE_C_HEAP_OBJ as I could not find it. Now I can not understand how I 
could /miss/ it, though I guess I was looking for a macro with the same 
signature as FREE_C_HEAP_ARRAY (one taking the type as argument) :-(

As a consequence I also deleted some outdated help text:

- //   NEW_RESOURCE_ARRAY(type, size)
- //   NEW_RESOURCE_OBJ(type)
- //   NEW_C_HEAP_ARRAY(type, size)
- //   NEW_C_HEAP_OBJ(type, memflags)
- //   FREE_C_HEAP_ARRAY(type, old)
- //   FREE_C_HEAP_OBJ(objname, type, memflags)
- //   char* AllocateHeap(size_t size, const char* name);
- //   void  FreeHeap(void* p);

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.



rerunning tests...

Thanks, Leo

More information about the hotspot-dev mailing list