ForkJoinPool.managedBlock

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

ForkJoinPool.managedBlock

JSR166 Concurrency mailing list
Experts,

The Javadoc for ForkJoinPool does not specify whether ForkJoinPool.managedBlock() is reentrant or not. I would expect a task which is already being managed to have nested managedblock()s be no-ops.

Thoughts?

--
Cheers,

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: ForkJoinPool.managedBlock

JSR166 Concurrency mailing list
On 04/13/2018 06:56 AM, Viktor Klang via Concurrency-interest wrote:

> The Javadoc for ForkJoinPool does not specify whether
> ForkJoinPool.managedBlock() is reentrant or not. I would expect a task
> which is already being managed to have nested managedblock()s be no-ops.
>
No, it is currently not reentrant, which is the cause of the
behavior in your last post. I've been sitting on this for a few
days trying to decide whether this is a bug or a feature.
I'm currently thinking feature, as an analog of standard advice
never to call arbitrary methods while holding locks.
If a block() method does anything except block, and in particular
adds or removes tasks, then it becomes very hard to reason about or
control. On the other hand, FJP does not currently even maintain
bookkeeping that would allow reliable detection of this case, which
probably should be added.

Other thoughts welcome.

-Doug
_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: ForkJoinPool.managedBlock

JSR166 Concurrency mailing list
Thanks for the very rapid response, Doug!

On Fri, Apr 13, 2018 at 1:09 PM, Doug Lea via Concurrency-interest <[hidden email]> wrote:
On 04/13/2018 06:56 AM, Viktor Klang via Concurrency-interest wrote:

> The Javadoc for ForkJoinPool does not specify whether
> ForkJoinPool.managedBlock() is reentrant or not. I would expect a task
> which is already being managed to have nested managedblock()s be no-ops.
>
No, it is currently not reentrant, which is the cause of the
behavior in your last post.

That sounds highly unlikely because my example is not reentrant?
Or could you explain further how it would create a situation of reentrancy?
 
I've been sitting on this for a few
days trying to decide whether this is a bug or a feature.
I'm currently thinking feature, as an analog of standard advice
never to call arbitrary methods while holding locks.

According to my understanding, managedBlock is a demarcation of a segment of code which is likely to block—it's basically (conditional) IO—even if it could be signalled purely across the memory bus.
Is that understanding flawed?

As a sidenote, I implemented a proof-of-concept of supporting the equivalent of ManagedBlocking using ThreadPoolExecutor: https://github.com/akka/akka/commit/5e5e38747449ad1798f8aae4a1b5e5989cac5eda#diff-410abf8aac6a89a01b05db2cbcf1765cR71
 
If a block() method does anything except block, and in particular
adds or removes tasks, then it becomes very hard to reason about or
control.

If it is not advised to execute arbitrary code under managedBlock() then in my mind the feature is of very little value,
being able to signal to the pool that a worker will be unlikely to be able to execute more work for a while is a *very* useful feature.

 
On the other hand, FJP does not currently even maintain
bookkeeping that would allow reliable detection of this case, which
probably should be added.

I've added the detection within Scala (since a long time), to make sure that there is no reentrancy, but it seemed like something which should either
be clear in the managedBlocker Javadoc, or should be made reentrant. I'd prefer the second alternative, following the principle of least surprise—the performance impact is only when actually block()-ing, which is a performance hit anyway.
 

Other thoughts welcome.

-Doug
_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest



--
Cheers,

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: ForkJoinPool.managedBlock

JSR166 Concurrency mailing list
On 04/13/2018 07:53 AM, Viktor Klang wrote:

> Thanks for the very rapid response, Doug!
>
> On Fri, Apr 13, 2018 at 1:09 PM, Doug Lea via Concurrency-interest
> <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     On 04/13/2018 06:56 AM, Viktor Klang via Concurrency-interest wrote:
>
>     > The Javadoc for ForkJoinPool does not specify whether
>     > ForkJoinPool.managedBlock() is reentrant or not. I would expect a task
>     > which is already being managed to have nested managedblock()s be no-ops.
>     >
>     No, it is currently not reentrant, which is the cause of the
>     behavior in your last post.

(Sorry; not quite the same issues, as explained in my other post.)

>
> According to my understanding, managedBlock is a demarcation of a
> segment of code which is likely to block—it's basically (conditional)
> IO—even if it could be signalled purely across the memory bus.
> Is that understanding flawed?

The operational meaning of managedBlock to FJP is:

1. Ensure that there are enough threads to maintain minimal liveness
(default 1) even if this thread is not active; or return
early if isReleasable; or proceed anyway (at the risk of
liveness failure) if saturate predicate (jdk9+) says OK,
or throw exception if would exceed thread limit.

2. if (1) succeeded, record that there is one fewer thread available
before calling block(), and restore on exit from block().

It is possible to skip most mechanics if the calling thread is
already blocked and saturate() permits it, with increased risk of
liveness failures. Considering that we now allow users to
tell us about tolerance via saturate predicate, I'm not completely
against doing this. It will be hard to explain though.

But it might be more productive to explore the main behavioral
effects you are trying to achieve?

-Doug



_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: ForkJoinPool.managedBlock

JSR166 Concurrency mailing list


On Sun, Apr 15, 2018 at 11:21 PM, Doug Lea via Concurrency-interest <[hidden email]> wrote:
On 04/13/2018 07:53 AM, Viktor Klang wrote:
> Thanks for the very rapid response, Doug!
>
> On Fri, Apr 13, 2018 at 1:09 PM, Doug Lea via Concurrency-interest
> <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     On 04/13/2018 06:56 AM, Viktor Klang via Concurrency-interest wrote:
>
>     > The Javadoc for ForkJoinPool does not specify whether
>     > ForkJoinPool.managedBlock() is reentrant or not. I would expect a task
>     > which is already being managed to have nested managedblock()s be no-ops.
>     >
>     No, it is currently not reentrant, which is the cause of the
>     behavior in your last post.

(Sorry; not quite the same issues, as explained in my other post.)

>
> According to my understanding, managedBlock is a demarcation of a
> segment of code which is likely to block—it's basically (conditional)
> IO—even if it could be signalled purely across the memory bus.
> Is that understanding flawed?

The operational meaning of managedBlock to FJP is:

1. Ensure that there are enough threads to maintain minimal liveness
(default 1) even if this thread is not active;

The problem I'm seeing is that the workers are getting stuck in awaitWork.
 
or return
early if isReleasable; or proceed anyway (at the risk of
liveness failure) if saturate predicate (jdk9+) says OK,
or throw exception if would exceed thread limit. 

2. if (1) succeeded, record that there is one fewer thread available
before calling block(), and restore on exit from block().

It is possible to skip most mechanics if the calling thread is
already blocked and saturate() permits it, with increased risk of
liveness failures. Considering that we now allow users to
tell us about tolerance via saturate predicate, I'm not completely
against doing this. It will be hard to explain though.

But it might be more productive to explore the main behavioral
effects you are trying to achieve?

ForkJoinWorkerThreadFactory does not signal whether it is creating new threads to maintain liveness due to ManagedBlocking, or otherwise, so the only way to deal with the situation is to have a limit on max number of concurrent workers.
 
I have an idea for a possible workaround. I'll see if I can get that up and running ASAP.


-Doug



_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest



--
Cheers,

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: ForkJoinPool.managedBlock

JSR166 Concurrency mailing list


On Mon, Apr 16, 2018 at 2:01 PM, Viktor Klang <[hidden email]> wrote:


On Sun, Apr 15, 2018 at 11:21 PM, Doug Lea via Concurrency-interest <[hidden email]> wrote:
On 04/13/2018 07:53 AM, Viktor Klang wrote:
> Thanks for the very rapid response, Doug!
>
> On Fri, Apr 13, 2018 at 1:09 PM, Doug Lea via Concurrency-interest
> <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     On 04/13/2018 06:56 AM, Viktor Klang via Concurrency-interest wrote:
>
>     > The Javadoc for ForkJoinPool does not specify whether
>     > ForkJoinPool.managedBlock() is reentrant or not. I would expect a task
>     > which is already being managed to have nested managedblock()s be no-ops.
>     >
>     No, it is currently not reentrant, which is the cause of the
>     behavior in your last post.

(Sorry; not quite the same issues, as explained in my other post.)

>
> According to my understanding, managedBlock is a demarcation of a
> segment of code which is likely to block—it's basically (conditional)
> IO—even if it could be signalled purely across the memory bus.
> Is that understanding flawed?

The operational meaning of managedBlock to FJP is:

1. Ensure that there are enough threads to maintain minimal liveness
(default 1) even if this thread is not active;

The problem I'm seeing is that the workers are getting stuck in awaitWork.
 
or return
early if isReleasable; or proceed anyway (at the risk of
liveness failure) if saturate predicate (jdk9+) says OK,
or throw exception if would exceed thread limit. 

2. if (1) succeeded, record that there is one fewer thread available
before calling block(), and restore on exit from block().

It is possible to skip most mechanics if the calling thread is
already blocked and saturate() permits it, with increased risk of
liveness failures. Considering that we now allow users to
tell us about tolerance via saturate predicate, I'm not completely
against doing this. It will be hard to explain though.

But it might be more productive to explore the main behavioral
effects you are trying to achieve?

ForkJoinWorkerThreadFactory does not signal whether it is creating new threads to maintain liveness due to ManagedBlocking, or otherwise, so the only way to deal with the situation is to have a limit on max number of concurrent workers.
 
I have an idea for a possible workaround. I'll see if I can get that up and running ASAP.

Looks possible to solve (in my case) like this: https://github.com/scala/scala/pull/6529
 


-Doug



_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest



--
Cheers,



--
Cheers,

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: ForkJoinPool.managedBlock

JSR166 Concurrency mailing list
On 04/16/2018 10:20 AM, Viktor Klang wrote:

>     I have an idea for a possible workaround. I'll see if I can get that
>     up and running ASAP.
>
>
> Looks possible to solve (in my case) like
> this: https://github.com/scala/scala/pull/6529

This seems OK as a better approximation of jdk9+ functionality,
but hopefully in the future you can replace with jdk9 version,
which avoids the raciness here of reserving and then unreserving
threads that don't actually materialize. Which can still have the
result of reducing parallelism (but much less so than your original).

-Doug

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: ForkJoinPool.managedBlock

JSR166 Concurrency mailing list


On Tue, Apr 17, 2018 at 12:35 PM, Doug Lea <[hidden email]> wrote:
On 04/16/2018 10:20 AM, Viktor Klang wrote:

>     I have an idea for a possible workaround. I'll see if I can get that
>     up and running ASAP.
>
>
> Looks possible to solve (in my case) like
> this: https://github.com/scala/scala/pull/6529

This seems OK as a better approximation of jdk9+ functionality,
but hopefully in the future you can replace with jdk9 version,
which avoids the raciness here of reserving and then unreserving
threads that don't actually materialize. Which can still have the
result of reducing parallelism (but much less so than your original).

Thanks for the feedback, Doug—my workaround seems to pass all tests I'm throwing at it.
 

-Doug




--
Cheers,

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: ForkJoinPool.managedBlock

JSR166 Concurrency mailing list
Quick followup question:

The javadoc of ManagedBlocker doesn't really mention any thread safety requirements, yet the examples seems to indicate that they are needed (for instance, the volatile `item` in the example code.

Can it be safely assumed that the only thread which will invoke any methods on the instance of the ManagedBlocker is the thread calling managedBlock(blocker)? (i.e. no safe publication needed, and no concurrency control to avoid concurrent invocations of the ManagedBlocker methods—of course assuming I do not submit the same ManagedBlocker instance more than once)

All the code I've looked at when it comes the the managedBlocker implementation looks safe, but I'm thinking about forward compatibility and potentially other JDKs.


On Tue, Apr 17, 2018 at 6:50 PM, Viktor Klang <[hidden email]> wrote:


On Tue, Apr 17, 2018 at 12:35 PM, Doug Lea <[hidden email]> wrote:
On 04/16/2018 10:20 AM, Viktor Klang wrote:

>     I have an idea for a possible workaround. I'll see if I can get that
>     up and running ASAP.
>
>
> Looks possible to solve (in my case) like
> this: https://github.com/scala/scala/pull/6529

This seems OK as a better approximation of jdk9+ functionality,
but hopefully in the future you can replace with jdk9 version,
which avoids the raciness here of reserving and then unreserving
threads that don't actually materialize. Which can still have the
result of reducing parallelism (but much less so than your original).

Thanks for the feedback, Doug—my workaround seems to pass all tests I'm throwing at it.
 

-Doug




--
Cheers,



--
Cheers,

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: ForkJoinPool.managedBlock

JSR166 Concurrency mailing list
On 04/18/2018 10:16 AM, Viktor Klang wrote:
> Quick followup question:
>
> The javadoc of ManagedBlocker
> <https://docs.oracle.com/javase/9/docs/api/java/util/concurrent/ForkJoinPool.ManagedBlocker.html> doesn't
> really mention any thread safety requirements, yet the examples seems to
> indicate that they are needed (for instance, the volatile `item` in the
> example code.

Not sure how to answer...

>
> Can it be safely assumed that the only thread which will invoke any
> methods on the instance of the ManagedBlocker is the thread calling
> managedBlock(blocker)?

FJP.managedBlock() calls ManagedBlocker.block() in the current thread.
The user implementation of block() must deal with other threads or IO.

I can't think of any javadoc that would further clarify?

-Doug


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: ForkJoinPool.managedBlock

JSR166 Concurrency mailing list


On Sun, Apr 22, 2018, 14:59 Doug Lea <[hidden email]> wrote:
On 04/18/2018 10:16 AM, Viktor Klang wrote:
> Quick followup question:
>
> The javadoc of ManagedBlocker
> <https://docs.oracle.com/javase/9/docs/api/java/util/concurrent/ForkJoinPool.ManagedBlocker.html> doesn't
> really mention any thread safety requirements, yet the examples seems to
> indicate that they are needed (for instance, the volatile `item` in the
> example code.

Not sure how to answer...


Apologies, either I was unclear or the answer is so obvious that I missed it :)
 
>
> Can it be safely assumed that the only thread which will invoke any
> methods on the instance of the ManagedBlocker is the thread calling
> managedBlock(blocker)?

FJP.managedBlock() calls ManagedBlocker.block() in the current thread.
The user implementation of block() must deal with other threads or IO.

Is this a quote from the documentation?


I can't think of any javadoc that would further clarify?

The reason I ask is mainly because I didn't want to rely on the implementation as the carrier of the spec, and perhaps I missed it, but couldn't find a clear answer in the doc for managedBlock() or in ManagedBlocker itself.

The situation was further a bit complicated by the two examples in ManagedBlocker where in one "hasLock" is a plain field, and "item" is a volatile field.

As it wouldn't be completely weird to have the ManagedBlocker handed over in tryCompensate for the compensating thread to test periodically to see whether what blocking it is compensating for is done yet (isReleasable()), hence the question. :-)

However—given your answer—I'll assume that it is only ever the calling thread of managedBlock() who will be invoking methods on it.

Also, it seems the contract is that managedBlocker is not "allowed" to call block() if isReleasable() returned true, and it isn't "allowed" to call block() is it has ever returned true. Will these invariants hold by spec rather than implementation? (Just making sure I have the right guards in place in my implementations.)


-Doug



_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest