RFR: 8212879: Make JVMTI TagMap table concurrent [v3]

Coleen Phillimore coleenp at openjdk.java.net
Tue Nov 3 21:51:03 UTC 2020


On Tue, 3 Nov 2020 13:45:57 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   More review comments from Stefan and ErikO
>
> src/hotspot/share/gc/shared/weakProcessorPhases.hpp line 50:
> 
>> 48: };
>> 49: 
>> 50: typedef uint WeakProcessorPhase;
> 
> This was originally written with the idea that WeakProcessorPhases::Phase (and WeakProcessorPhase) should be a scoped enum (but we didn't have that feature yet). It's possible there are places that don't cope with a scoped enum, since that feature wasn't available when the code was written, so there might have be mistakes.
> 
> But because of that, I'd prefer to keep the WeakProcessorPhases::Phase type and the existing definition of WeakProcessorPhase. Except this proposed change is breaking that at least here:
> 
> src/hotspot/share/gc/shared/weakProcessor.inline.hpp
> 116     uint oopstorage_index = WeakProcessorPhases::oopstorage_index(phase);
> 117     StorageState* cur_state = _storage_states.par_state(oopstorage_index);
> =>
> 103     StorageState* cur_state = _storage_states.par_state(phase);
> 
> I think eventually (as in some future RFE) this could all be collapsed to something provided by OopStorageSet.
> enum class : uint WeakProcessorPhase {}; 
> 
> ENUMERATOR_RANGE(WeakProcessorPhase,
>                  static_cast<WeakProcessorPhase>(0), 
>                  static_cast<WeakProcessorPhase>(OopStorageSet::weak_count));
> and replacing all uses of WeakProcessorPhases::Iterator with EnumIterator<WeakProcessorPhase> (which involves more than a type alias).
> 
> Though it might be possible to go even further and eliminate WeakProcessorPhases as a thing separate from OopStorageSet.

Ok, so I'm not sure what to do with this:

 enum Phase { 
    // Serial phase.
    JVMTI_ONLY(jvmti)
    // Additional implicit phase values follow for oopstorages.
  `};`

I've removed the only thing in this enum.

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

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



More information about the build-dev mailing list