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

[Robbin Ehn]

Hi David,

On 5/7/19 12:40 AM, David Holmes wrote:
> 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 processes must be paused. If the processes would be running the linked list 
is also broken since if we unlink and delete a JavaThread and then later SA 
follows that _next pointer.

> 
> 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.

There are already several usages of lambdas in SA code, e.g. 
LinuxDebuggerLocal.java. SA is not a core module, it's an application, there is
not a bootstrap issue or so.

> 
> 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.

That's why I only used the internal iteration where we didn't have early returns.

Thanks, Robbin

> 
> 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