Feedback on the API

[Alan Bateman]

On 21/02/2019 22:17, Volkan Yazıcı wrote:
> Hello,
>
> Even though the current prototype interfaces are not finalized yet, I would
> like to share some feedback in no particular order.
Thanks for taking time to try it out.

>
>     - Fiber.schedule() ergonomics is not aligned with Executor#invoke*() and
>     Executor#submit().
Once we get more confidence with fiber scopes then I think we can 
looking at replacing the Fiber.schedule methods (there is an API note 
about this in the javadoc but might not be obvious).


>     - Maybe it is just me, but I anticipate people will try to reach out for
>     the fiber counterpart of Thread.sleep(). (I am very well aware of
>     Thread.sleep() does not block the fiber scheduler thread.)
TBD, for now just call Thread.sleep (which will park the fiber rather 
than the carrier thread as you have found).


>     - Propagating InterruptedExceptions via
>     Thread.currentThread().interrupt() is a pretty common practice. That said,
>     typing Thread.currentThread().interrupt() in a fiber body feels like I am
>     doing something wrong.
We are exploring cancellation of fibers but with different semantics 
that avoids the error prone catching of InterruptedException and 
re-asserting of the interrupt status. I hope to have something on the 
Loom wiki about this soon.


>     - FiberScope interface feels like a black box. Is it supposed to be
>     extended? Am I constrained to the default provided ones? What does a
>     FiberScope technically deliver? It is not really intention revealing.
>     - Please don't pollute FiberScope with static accessors of particular
>     implementations, e.g., FiberScope.currentDeadline(). FiberScope.current()
>     would be good enough. Then each implementation can introduce their own
>     accessors.
There is deliberately no FiberScope.current() at this time, this is 
partly because the current prototype uses AutoCloseable and we need to 
reduce the temptation to call close in random places.

The currentDeadline() method is to make it easy to get the current 
deadline, which may have been set further up the stack in some enclosing 
scope. More on this below.


>     - Isn't there a garbage-free way to access the result of a Fiber? Do we
>     always need to convert it into a Future?
Did you find the Fiber::join method?


>     - It would be handy if users could pass their own BlockingQueue
>     implementations to FiberScopeImpl. (This is a pain point in
>     ScheduledThreadPoolExecutor, IMHO.)
TerminationQueue is deliberately final at this time - this is because it 
would amount to arbitrary code executing when a fiber terminates. For 
now, the implementation gets away with queuing the terminated fiber from 
the carrier thread where the fiber was mounted when it terminated. There 
are several subtle issues around this that we are avoiding by avoiding BYOQ.

>     - In its current form, FiberScope needs to be explicitly passed to
>     Fiber.schedule().
They were temporary for the initial push, they are gone now. In the 
current prototype you can do this:

try (var scope = FiberScope.withDeadline(...)) {
     Fiber<String> fiber1 = scope.schedule(() -> "foo");
     Fiber<String> fiber2 = scope.schedule(() -> "bar");
}


> If you schedule a fiber within a scope and that fiber
>     directly or indirectly schedules a new fiber discarding the scope in use,
>     your beautiful scope serves no purpose anymore. And I think this leakage of
>     fibers totally contradicts with the goal of structured concurrency. I can
>     understand fixing this incurs a well thought new concept, e.g., special-,
>     FiberScoped-, TaskLocal-, etc. variables. Though unless one can implicitly
>     employ the active scope for the upcoming fiber schedulings, FiberScope is
>     no better than hand crafted deadline facilities, which I believe what we
>     are trying to avoid.
Ah, I suspect you might have missed that they nest. Here's an example 
that might help:

     void foo() {
         try (var scope1 = FiberScope.cancellable()) {
             var fiber1 = scope1.schedule(() -> bar());
         }
     }

     void bar() {
         try (var scope2 = FiberScope.cancellable()) {
             var fiber2 = scope2.schedule(...);
         }
     }

A thread (or fiber) invoking foo enters scope1, schedules fiber1, and 
then exits. Exiting scope1 waits for fiber1 to terminate (if it hasn't 
terminated already)

fiber1, initially in scope1, enters scope2, schedules fiber2, and then 
exits. Existing scope2 waits for fiber2 to terminate, at which point 
fiber1 is back in scope1.

So no leakage and no need to pass the scope around. There will be cases 
where you might need to schedule a fiber that outlives the context where 
it is created. For now this is the "detached" scope, e.g.

FiberScope.detached().schedule(() -> nixer());

"detached" is a lousy name, it's really the primordial scope (which 
cannot be closed/exited). I'm sure we'll converge on something better in 
time.

You mention locals/variables. There isn't anything yet but this is an 
important topic for exploration.


>     - While "scope" term in FiberScope makes perfect sense, I find
>     "ContinuationScope" confusing. It rather feels like a "state" than a
>     "scope".
The continuation scope is important nested continuations. But yes, we 
have too many concepts using the term "scope" right now. I'm sure that 
better names will be found once the various areas of exploration are 
further along.

-Alan