RFR: 8072498: Multi-thread JNI weak reference processing

[Thomas Schatzl]

Hi,

On Thu, 2018-08-09 at 14:57 -0400, Kim Barrett wrote:
> > On Aug 8, 2018, at 10:27 AM, Kim Barrett <kim.barrett at oracle.com>
> > wrote:
> > 
> > > On Aug 8, 2018, at 10:11 AM, Thomas Schatzl <thomas.schatzl at oracl
> > > e.com> wrote:
> > > 
> > > Hi Kim,
> > > 
> > > On Tue, 2018-07-31 at 15:23 -0400, Kim Barrett wrote:
> > > > […]
> > > > CR:
> > > > https://bugs.openjdk.java.net/browse/JDK-8072498
> > > > 
> > > > Webrev:
> > > > http://cr.openjdk.java.net/~kbarrett/8072498/open.00/
> > > > 
> > > > Testing:
> > > > mach5 tier1-3, hs-tier4-5.
> > > > 
> > > > Local and mach5 testing of TestGCBasherWithG1 and
> > > > TestSystemGCWithG1, with the tests modified to use a smaller
> > > > non-default value of -XX:ReferencesPerThread, to examine the
> > > > logging output and verify multi-threaded execution.
> > > > 
> > > 
> > > - g1ConcurrentMark.cpp:1673: I assume that using the serial
> > > version of weak processing has been used here because of logging
> > > concerns. Is there an upcoming change that fixes that?
> > > One could create a local instance of a WeakProcessorPhaseTimes
> > > and just take the interesting total time taken from it and print
> > > it. Or just print it the same way as for the young gcs - I
> > > understand this is ugly, but as long as there is no other way...
> > > 
> > > - same for g1FullCollector.cpp:217 and g1FullGCAdjustTask:101
> > 
> > These are using the parallel versions.  Maybe overloading the
> > weak_oops_do
> > name with one that takes a workgang is too subtle?
> 
> To be clear, these are calling this overload:
> 
>   template<typename IsAlive, typename KeepAlive>
>   weak_oops_do(WorkGang*, IsAlive*, KeepAlive*, uint log_indent)
> 
> which stack allocates a temporary WeakProcessorPhaseTimes, calls the
> parallel overload taking a phasetime as the fourth argument, and then
> logs the phasetime with the requested indentation.
> 
> New webrevs without the external optimization of run_task:
> full: http://cr.openjdk.java.net/~kbarrett/8072498/open.01/
> incr: http://cr.openjdk.java.net/~kbarrett/8072498/open.01.inc/

  looks good.

Thomas