RFR: 8230398: Remove NULL checks before FREE_C_HEAP_ARRAY

Kim Barrett kim.barrett at oracle.com
Mon Sep 2 20:38:32 UTC 2019

> 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


There are some destructors that are unnecessarily nulling out members
after deleting their values, such as here:

 387   FREE_C_HEAP_ARRAY(const char, _name);
 388   _name = NULL;

106 RSHashTable::~RSHashTable() {

626 CardTableRS::~CardTableRS() {

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.

More information about the hotspot-dev mailing list