RFR: 8194312: Support parallel and concurrent JNI global handle processing

[coleen.phillimore at oracle.com]

On 1/10/18 4:10 PM, Kim Barrett wrote:
>> On Jan 10, 2018, at 8:56 AM, coleen.phillimore at oracle.com wrote:
>>
>>
>>
>> On 1/8/18 10:44 PM, Kim Barrett wrote:
>>>> I feel like BasicParState should be in an oopStorage.inline.hpp file to be included by the GCs that need it.   Runtime code only needs to include oopStorage.hpp and doesn't need this code.
>>> I forgot to deal with your pre-review suggestion of moving some stuff
>>> to oopStorage.inline.hpp.  I'd like to defer making such a change
>>> until this review is (mostly) done, rather than clutter up diffs.
>>>
>>> I think all (or very nearly all) of the inline definitions in
>>> oopStorage.hpp could be moved. I think they're all related to
>>> iteration, and I think most clients (other than GC) won't be using
>>> iteration at all.
>>>
>>> I think having the parallel iteration support wrapped with
>>> #ifdef INCLUDE_ALL_GCS (as is already the case) deals with the
>>> conditional GC usage; I think that all except Serial will eventually
>>> use it.
>>>
>>> The parallel iteration support could be split out into another file,
>>> so places that only use serial iteration (like jvmti) wouldn't include
>>> it, but I think the number of such places doesn't warrant another
>>> file.
>> I would like that it be split out into another file.  A lot of places in the runtime will only create and add oops to OopStorage and shouldn't care about iteration.  I'm fine with making this change as a further improvement.
> I'm unclear on what you are asking for.  Do you want
>
> A. Two files:
>
> (1) oopStorage.hpp - contains just class OopStorage.
>
> (2) oopStorage.inline.hpp - contains all the inline definitions
> presently in oopStorage.hpp.
>
> or
>
> B. Three files:
>
> (1) oopStorage.hpp - contains just class OopStorage.
>
> (2) oopStorage.inline.hpp - contains all the inline definitions
> presently in oopStorage.hpp, except for the ParState stuff.
>
> (3) oopStorageParState.inline.hpp - contains the ParState stuff.
>
> or
>
> C. Two files:
>
> (1) oopStorage.hpp - contains class OopStorage and all the inline
> definitions presently in oopStorage.hpp, except for the ParState
> stuff.
>
> (2) oopStorage.inline.hpp - contains the ParState stuff.
>
> I'm fine with option (A).  Option (B) is unusual, but plausible; among
> other things, inclusion of the ParState header by non-GC code is
> suspicious.  I'm not sure that's worth another file.  Option (C)
> doesn't seem like what you are asking for, but included for
> completeness.
>

I think B.  I'd have to see it.   Put this in the RFE.

Coleen