RFR: 8230398: Remove NULL checks before FREE_C_HEAP_ARRAY

[Leo Korinth]

On 02/09/2019 22:38, Kim Barrett wrote:
>> On Sep 2, 2019, at 9:06 AM, Leo Korinth <leo.korinth at oracle.com> wrote:
>>
>> Hi!
>>
>> This fixup removes NULL checks before FREE_C_HEAP_ARRAY. This is unnecessary as FREE_C_HEAP_ARRAY does also do this NULL check. This is a follow up to 8227168: Cleanup usage of NEW_C_HEAP_ARRAY.
>>
>> deallocate_counters() in src/hotspot/os/windows/os_perf_windows.cpp does (in addition to NULL the pointer) also set a counter to zero. Although my change "looks" correct, I tried follow the usage of deallocate_counters(), and could not find a call site. Should I create a bug on this, or am I missing something here?
> 
> Your modification to deallocate_counters looks fine.  I don't think
> you are missing anything, I think it's not called.  That does look
> like it might be a (pre-existing) bug.  Maybe they are immortal?  But
> that isn't obvious from a quick skim.
> 
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8230398
>>
>> Webrev:
>> http://cr.openjdk.java.net/~lkorinth/8230398
>>
>> Testing:
>> mach5 tier1-3
>>
>> Thanks,
>> Leo
> 
> ------------------------------------------------------------------------------
> 
> [pre-existing]
> There are some destructors that are unnecessarily nulling out members
> after deleting their values, such as here:
> 
> src/hotspot/share/classfile/classLoader.cpp
>   387   FREE_C_HEAP_ARRAY(const char, _name);
>   388   _name = NULL;
> 
> src/hotspot/share/gc/g1/sparsePRT.cpp
> 106 RSHashTable::~RSHashTable() {
> 
> src/hotspot/share/gc/shared/cardTableRS.cpp
> 626 CardTableRS::~CardTableRS() {
> 
> src/hotspot/share/runtime/thread.cpp
> 1330 NamedThread::~NamedThread() {
> 
> Your call whether you want to do anything about these with this change.
> 
> ------------------------------------------------------------------------------
> 
> Other than that, looks good.  I don't need another webrev if you
> decide to remove the above assignments to NULL near code you were
> already changing.  A more extensive pass for such should perhaps be a
> separate change.
> 
I will remove NULL assignments of fields in destructors and keep them 
otherwise.

I will also add a comment (// handles NULL pointers) to FreeHeap and 
os::free to not only describe behaviour, but also to establish a contract.

Also fixed indentation miss in os_perf_windows.cpp

Incremental:
http://cr.openjdk.java.net/~lkorinth/8230398/00_01

Full:
http://cr.openjdk.java.net/~lkorinth/8230398/01


running tests...