RFR: 8257676: Simplify WeakProcessorPhase

[Kim Barrett]

On Wed, 9 Dec 2020 18:22:27 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Please review this reimplementation of WeakProcessorPhase.  It is changed to
>> a scoped enum at namespace scope, and uses the recently added EnumIterator
>> facility to provide iteration, rather than a bespoke iterator class.
>> 
>> This is a step toward eliminating it entirely.  I've split it out into a
>> separate PR to make the review of the follow-up work a bit easier.
>> 
>> As part of this the file weakProcessorPhases.hpp is renamed to
>> weakProcessorPhase.hpp, but git doesn't seem to be recognizing that as a
>> rename and (majorly) edit, instead treating it as a remove and add a new
>> file.
>> 
>> Testing: mach5 tier1
>
> src/hotspot/share/gc/shared/weakProcessor.inline.hpp line 88:
> 
>> 86:     CountingClosure<IsAlive, KeepAlive> cl(is_alive, keep_alive);
>> 87:     WeakProcessorPhaseTimeTracker pt(_phase_times, phase, worker_id);
>> 88:     int state_index = checked_cast<int>(phase_range.index(phase));
> 
> I feel `EnumRange<WeakProcessorPhase>().index(phase)` is better than `phase_range.index(phase)`, since we want to know the "global" index for this phase, not the "local" index within this particular range instance. The two are identical currently. However, if the for-loop is iterating over a subset of all `WeakProcessorPhase`, the two cases will give different results, I believe.

I agree that this isn't entirely ideal as-is. If someone were to change
phase_range to be a subset of the full range, then this line would need to
be examined and probably modified. However, that's not going to happen. This
PR is an intermediate step toward eliminating WeakProcessorPhase entirely,
which will address this issue (among others). I have that followup ready for
review once this one is integrated and I've rebased and retested.

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

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