RFR(m): 8223306: Remove threads linked list (use ThreadsList's array in SA)

[David Holmes]

Hi Robbin,

I have a few concerns here.

First I can't see how you are actually integrating the SA with the 
ThreadSMR. You've exposed the _java_thread_list for it to iterate but 
IIRC that list can be updated when threads are added/removed and I'm not 
seeing how the SA is left iterating a valid list - we'd normally using a 
ThreadsListHandle for that ?? (I may need a refresher on how this list 
is actually maintained.)

The conversion from external iteration of the list (the for loop) to 
internal iteration (passing a lambda to JavaThreadsDo) is also 
problematic. First I'd be very wary about introducing lambda expressions 
into the SA code - lambda drags in a lot of supporting code that could 
have an impact on the way SA functions. There are places where we have 
to avoid lambdas due to bootstrapping/initialization issues and I think 
the SA may be an area where we also want to keep the code extremely simple.

Second by using lambda's with internal iteration you've lost the ability 
to terminate the iteration loop! In the existing code if we have a 
return in the for-loop then we not only terminate the loop but the 
enclosing method. With the lambda the "return" just ends the current 
iteration and JavaThreadsDo will then continue with the next thread - so 
we don't even terminate the iteration let alone the method performing 
the iteration. So for places were we want to process one thread, for 
example, we will continue to iterate all remaining threads and just do 
nothing with them. That's very inefficient.

Thanks,
David

On 6/05/2019 5:31 pm, Robbin Ehn wrote:
> Hi, please review.
> 
> Old threads linked list remove and updated SA to use ThreadsList array 
> instead.
> 
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8223306
> Webrev:
> http://cr.openjdk.java.net/~rehn/8223306/webrev/
> 
> Passes t1-3 (which includes SA tests).
> 
> Thanks, Robbin