RFR 9: 8077350 Process API Updates Implementation Review

[Roger Riggs]

Hi Peter,

yes, your patch does reduce the code quite a bit except where the 
complexity is needed.

The commonPool does have a soft limit (256/system property) on the 
number of
compensating processes it will start.
That limit would only come into play for subclasses of Process that did
not properly override onExit to delegate to the underlying builtin 
Process implementation.
So, probably not a big risk.

It might be desirable to put the loop and exception handling inside the 
ManagedBlocker
but the difference is only in the creation of additional inner class 
instances in the case
of InterruptedExceptions.

Thanks, Roger



On 5/20/2015 4:39 AM, Peter Levart wrote:
> Hi Roger,
>
> I looked at Martin's idea and I think that we don't need the 
> AsyncExecutor at all (it already sounds like I hate it ;-). Using 
> ManagedBlocker, a ForkJoinPoll can compensate and grow it's pool as 
> needed when Process.waitFor() blocks. So we could leverage this 
> feature and simplify things even further:
>
> http://cr.openjdk.java.net/~plevart/jdk9-sandbox/JDK-8046092-branch/webrev.03/ 
>
>
> Passing a commonPool() to xxxAsync() methods is unneeded as the 
> default is exactly the same. If CompletableFuture ever gets a feature 
> to specify a default Executor for all it's descendants, then we can 
> revisit this if needed.
>
> What do you think?
>
> Regards, Peter
>
> On 05/19/2015 10:15 PM, Roger Riggs wrote:
>> The webrev, javadoc, and specdiffs have been updated to address 
>> recent recommendations:
>>
>> Please review and comment:
>>
>> Webrev:
>> http://cr.openjdk.java.net/~rriggs/webrev-ph/ (May 19)
>>
>> javadoc:
>> http://cr.openjdk.java.net/~rriggs/ph-apidraft/ (May 19)
>>
>> Diffs of the spec/javadoc from previous draft:
>> http://cr.openjdk.java.net/~rriggs/ph-diffs-2015-05-19/overview-summary.html 
>>
>>
>> Thanks, Roger
>