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