Hi Peter,
On 5/19/2015 4:34 AM, Peter Levart wrote:
Hi Roger,
On 05/18/2015 11:49 PM, Roger Riggs wrote:
The earlier spec specifically used the common pool but from a
specification point of view
it allows more flexibility in the implementation not to bind onExit
to the commonPool.
Its not clear that the attributes of threads used to handle output or
managing Processes
are the same as those attributed to the commonPool.
The default sizing of the commonPool is based on the number of
processors
and does not grow (except via the ManagedBlockers) mechanism.
The number of threads waiting for processes is unrelated to the
number of CPUs.
It is simpler to maintain to use a separate pool and not depend on
consistent
assumptions between Process handling and the commonPool.
I'm not advocating to use commonPool for waiting on the process
exit(s). We have special low-stack-size reaper threads for this
purpose and in future there could be just one thread maybe. I'm also
not advocating to use commonPool in default Process.onExit()
implementation where the thread must wait for process to exit. I'm
just questiong the use of commonPool vs. AsyncExecutor in
ProcessImpl.onExit() and ProcessHandleImpl.onExit() where there is an
async CompletableFuture stage inserted to shield from exposing
internal low-stack-size threads.
The code that is executed by commonPool (or AsyncExecutor) by onExit()
continuations doesn't wait for process to exit. The number of
processes spawned does not matter directly. Only the rate of process
exits does. The code executed will be a cleanup-type /
post-exit-actions-type of code which seems to be typical code for
commonPool. Code dealing with process input/output will typically be
executed concurrently with the Process while the process is still
running and not by onExit() continuations.
makes sense
On 05/18/2015 11:49 PM, Roger Riggs wrote:
One alternative is for onExit to return a CF subclass that maps
synchronous methods
to the corresponding async methods. Each call to onExit would return
a new proxy
that would delegate to a new CF created from the
ExitCompletion.handle(...).
For all requests that involved calling app code it would delegate to
the corresponding
async method. For example,
CompletableFuture<ProcessHandle> cf =
ProcessHandleImpl.completion(getPid(), false)
.handle((exitStatus, unusedThrowable) -> this);
return new CompletionProxy(cf);
The CompletionProxy can ensure that all CFs that call application
code are Async.
It would override the syncronous methods and redirect to the
corresponding Async method.
public <U> CompletableFuture<U> thenApply(Function<? super
ProcessHandle, ? extends U> fn) {
return thenApplyAsync(fn);
}
The CompletionProxy can also supply the Executor as needed unless
supplied by the application
It adds a simple delegation but removed the extra task dispatch
except where needed.
Does that work as you would expect?
Thanks, Roger
I thought about that too. Unfortunately, there are also xxxAsync(...,
Executor) methods that appear to attach asynchronous continuations.
And they do, if passed-in Executor dispatches to separate threads. But
users can pass in a "SynchronousExecutor" like the following:
public class SynchronousExecutor implements Executor {
@Override
public void execute(Runnable command) {
command.run();
}
}
...which makes xxxAsync(..., Executor) methods equivalent to
synchronous-continuation-attaching methods.
It might be a bit interventionist, but its possible check the concrete
type of the executors
and allow direct use of those that are built-in; or to replace unknown
ones with a built-in.
It would be lighter weight than always queuing a task and potentially
starting up a new thread.
It would be nice to have an API that guaranteed division of threads
between internal-code and user-code threads, but such API would have
to be constructed around something different than Executor interface.
I think what we have with insertion of an async stage is the best we
can do with CompletableFuture API.
ok, the implementation can be updated if something new becomes available.
The only question remaining is whether to use AsyncExecutor in
ProcessImpl and ProcessHandleImpl .onExit() methods for executing the
inserted async stage (and any synchronous continuation of it), or only
in default implementation of Process.onExit() for waiting on process
exit. The AsyncExecutor properties are such that it doesn't pre-start
any threads and if not used, doesn't represent any system overhead.
This is good if it is normally not used and would make it a good
candidate for default Process.onExit() implementation which is used
only in uncommon custom Process implementations. Using it in
ProcessImpl and ProcessHandleImpl .onExit() methods makes it being
used after each onExit() call, albeit potentially only briefly, so
there's a chance that only one thread will ever be started and will
die after every 60s of inactivity. commonPool() is no different from
that perspective, but there is a chance, being a common pool, that it
is used for other purposes too, so potentially less thread starts and
stops can be achieved.
Makes sense
There's also the lack of uniformity if AsyncExecutor is used which can
have surprising effects. For example, a user switches from using
synchronous onExit() continuation to asynchronous one and as a result
the executor changes which can results in different behavior.
That argument would inhibit changing the implementation since there is
an implicit dependency
on the implementation of the Executor.
I would be *for* using the AsyncExecutor in Process[Handle]Impl if it
could be set as default Executor for any continuation born from
returned CompletableFuture transitively. There were some changes
announced from jsr166 team that would allow that (subclassing CF so
that the methods returned a subclass of CF), so this could perhaps be
the way to go...
A factory for the new CF's that are returned from CF would be quite
useful if/when that comes to pass.
I've updated the implementation to use the commonPool for
ProcessHandleImpl and the ProcessImpls
and retaining the asyncExecutor pool for Process.onExit.
Thanks, Roger
Regards, Peter
