RFR: 8277824: Remove empty RefProcSubPhasesWorkerTimeTracker destructor

Kim Barrett kbarrett at openjdk.java.net
Mon Nov 29 09:36:09 UTC 2021


On Thu, 25 Nov 2021 09:29:36 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

> Simple change removing effectively dead code, and some general cleanup.
> 
> Test: hotspot_gc

Changes requested by kbarrett (Reviewer).

src/hotspot/share/gc/shared/referenceProcessorPhaseTimes.hpp line 113:

> 111: public:
> 112:   RefProcWorkerTimeTracker(WorkerDataArray<double>* worker_time, uint worker_id);
> 113:   ~RefProcWorkerTimeTracker();

The usual guideline is that the destructor for a baseclass should be either
public and virtual or protected and non-virtual. This prevents destructor
slicing by `delete`. That the base class in question is a StackObj mitigates
against that kind of error.

And indeed, if I recall correctly, because it's now a StackObj its destructor
can't be virtual, because that would run afoul of the induced "deleting
destructor".

There are some relevant warning options that I occasionally look at and
remember why I think they aren't right for us, so we can't ever enable them
(-Wdelete-non-virtual-dtor and -Wnon-virtual-dtor).

But all that is kind of irrelevant.  The real design problem here is that
RefProcSubPhasesWorkerTimeTracker is derived from RefProcWorkerTimeTracker.
It should instead *have* a RefProceWorkerTimeTracker.  That would eliminate
all the discussion about base class destructors, and would also make
RefProcWorkerTimeTracker no longer need to perform double-duty as both a base
class and as a concrete class (which is often problematic).

-------------

PR: https://git.openjdk.java.net/jdk/pull/6555



More information about the hotspot-gc-dev mailing list