RFR: 8257676: Simplify WeakProcessorPhase

Albert Mingkun Yang ayang at openjdk.java.net
Thu Dec 10 09:46:36 UTC 2020


On Thu, 10 Dec 2020 09:23:07 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> 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.

I see; thanks for the explanation.

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

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



More information about the hotspot-gc-dev mailing list