ForkJoin refresh

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

ForkJoin refresh

JSR166 Concurrency mailing list

Now committed in jsr166 are extensions of the AQS refresh of last fall
to also cover ForkJoin; among other things removing reliance on builtin
monitors  The extra few months were mainly a result of noticing that the
initial way I did this increased performance differences across garbage
collectors. These are now reduced (not increased), but still worth
noting: FJ programs are relatively sensitive to GC choice because they
entail work-stealing duels between program and collector. On hotspot,
usually the best throughput for divide-and-conquer style usages
(including parallelStreams) is with -XX:+UseParallelGC. For unstructured
computations with lots of little tasks (for example little
CompletableFutures), UseShenandoahGC (and/or if needing more than 32GB
heap, UseZGC) are often better choices.  UseParallelGC meshes well with
highly generational structured parallelism, but is prone to
card-mark-contention with small tasks (even with -XX:+UseCondCardMark)
and long pauses. The default UseG1GC tends to be somewhere in the
middle. Also, on linux, using -XX:+UseTransparentHugePages seems to
always be a good idea for programs using FJ. (Other switches seem to
have less consistent positive effects.)

These days, it is almost impossible to quantify exactly how much better
performance is, but it's likely that your usages are faster. For
example, about half of the programs in the recent Renaissance Suite
(https://renaissance.dev/) somehow use FJ, and most of these have
improved scores on most machines, most JVMs (including OpenJ9 and
Graal), most collectors, most phases of the moon....

It would be great to get feedback from other usages before integrating
into OpenJDK. As usual, you can try it with any JDK11+ JVM by grabbing
http://gee.cs.oswego.edu/dl/concurrent/dist/jsr166.jar and running "java
--patch-module java.base="$DIR/jsr166.jar", where DIR is the full file
prefix. (Although beware that using --patch-module slows down startup,
and occasionally entire test runs. For other options, see
http://gee.cs.oswego.edu/dl/concurrency-interest/index.html).

-Doug

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

Re: ForkJoin refresh

JSR166 Concurrency mailing list
Thanks Doug - how different is this to the Java 14-EA version?  I
noticed the ManagedBlocker implementation today whilst showing the
Condition.await() implementation to a class I was teaching and even
tweeted about it: https://twitter.com/heinzkabutz/status/1218204292626243585

Funny coincidence :-)

Regards

Heinz
--
Dr Heinz M. Kabutz (PhD CompSci)
Author of "The Java™ Specialists' Newsletter" - www.javaspecialists.eu
Java Champion - www.javachampions.org
JavaOne Rock Star Speaker
Tel: +30 69 75 595 262
Skype: kabutz

On 2020/01/17 16:39, Doug Lea via Concurrency-interest wrote:

> Now committed in jsr166 are extensions of the AQS refresh of last fall
> to also cover ForkJoin; among other things removing reliance on builtin
> monitors  The extra few months were mainly a result of noticing that the
> initial way I did this increased performance differences across garbage
> collectors. These are now reduced (not increased), but still worth
> noting: FJ programs are relatively sensitive to GC choice because they
> entail work-stealing duels between program and collector. On hotspot,
> usually the best throughput for divide-and-conquer style usages
> (including parallelStreams) is with -XX:+UseParallelGC. For unstructured
> computations with lots of little tasks (for example little
> CompletableFutures), UseShenandoahGC (and/or if needing more than 32GB
> heap, UseZGC) are often better choices.  UseParallelGC meshes well with
> highly generational structured parallelism, but is prone to
> card-mark-contention with small tasks (even with -XX:+UseCondCardMark)
> and long pauses. The default UseG1GC tends to be somewhere in the
> middle. Also, on linux, using -XX:+UseTransparentHugePages seems to
> always be a good idea for programs using FJ. (Other switches seem to
> have less consistent positive effects.)
>
> These days, it is almost impossible to quantify exactly how much better
> performance is, but it's likely that your usages are faster. For
> example, about half of the programs in the recent Renaissance Suite
> (https://renaissance.dev/) somehow use FJ, and most of these have
> improved scores on most machines, most JVMs (including OpenJ9 and
> Graal), most collectors, most phases of the moon....
>
> It would be great to get feedback from other usages before integrating
> into OpenJDK. As usual, you can try it with any JDK11+ JVM by grabbing
> http://gee.cs.oswego.edu/dl/concurrent/dist/jsr166.jar and running "java
> --patch-module java.base="$DIR/jsr166.jar", where DIR is the full file
> prefix. (Although beware that using --patch-module slows down startup,
> and occasionally entire test runs. For other options, see
> http://gee.cs.oswego.edu/dl/concurrency-interest/index.html).
>
> -Doug
>
> _______________________________________________
> Concurrency-interest mailing list
> [hidden email]
> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: ForkJoin refresh

JSR166 Concurrency mailing list
In reply to this post by JSR166 Concurrency mailing list
Thanks for sharing!

Probably a naive question...
How these results are been obtained?
There is any Open-Source repository that shows the source of the tests performed?


Thanks,
Francesco

Il ven 17 gen 2020, 22:40 Doug Lea via Concurrency-interest <[hidden email]> ha scritto:

Now committed in jsr166 are extensions of the AQS refresh of last fall
to also cover ForkJoin; among other things removing reliance on builtin
monitors  The extra few months were mainly a result of noticing that the
initial way I did this increased performance differences across garbage
collectors. These are now reduced (not increased), but still worth
noting: FJ programs are relatively sensitive to GC choice because they
entail work-stealing duels between program and collector. On hotspot,
usually the best throughput for divide-and-conquer style usages
(including parallelStreams) is with -XX:+UseParallelGC. For unstructured
computations with lots of little tasks (for example little
CompletableFutures), UseShenandoahGC (and/or if needing more than 32GB
heap, UseZGC) are often better choices.  UseParallelGC meshes well with
highly generational structured parallelism, but is prone to
card-mark-contention with small tasks (even with -XX:+UseCondCardMark)
and long pauses. The default UseG1GC tends to be somewhere in the
middle. Also, on linux, using -XX:+UseTransparentHugePages seems to
always be a good idea for programs using FJ. (Other switches seem to
have less consistent positive effects.)

These days, it is almost impossible to quantify exactly how much better
performance is, but it's likely that your usages are faster. For
example, about half of the programs in the recent Renaissance Suite
(https://renaissance.dev/) somehow use FJ, and most of these have
improved scores on most machines, most JVMs (including OpenJ9 and
Graal), most collectors, most phases of the moon....

It would be great to get feedback from other usages before integrating
into OpenJDK. As usual, you can try it with any JDK11+ JVM by grabbing
http://gee.cs.oswego.edu/dl/concurrent/dist/jsr166.jar and running "java
--patch-module java.base="$DIR/jsr166.jar", where DIR is the full file
prefix. (Although beware that using --patch-module slows down startup,
and occasionally entire test runs. For other options, see
http://gee.cs.oswego.edu/dl/concurrency-interest/index.html).

-Doug

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

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

Re: ForkJoin refresh

JSR166 Concurrency mailing list
On 1/17/20 5:11 PM, Francesco Nigro wrote:

> How these results are been obtained?

I'm not set up to report performance on a published benchmark suite, so
just informally characterized results. Checking the Renaissance suite is
helpful in avoiding over-reliance on microbenchmarks that might not
capture typical uses. But coming up with results people could readily
compare with others is a lot of work. It would be great if anyone wants
to take this on.

Also currently, some programs in the suite (version 0.10) don't work
with JDK12+. To run the ones that appear to work (although in two cases
with warnings that might bear on results), and use FJ (in a few cases
not all that much) try:

java --patch-module java.base=jsr166.jar [... gc settings ...] \
  -jar renaissance-gpl-0.10.0.jar \

fj-kmeans,future-genetic,scrabble,reactors,par-mnemonics,neo4j-analytics,finagle-http,finagle-chirper

With --csv option for results, and/or use their instructions to run
under JMH at https://github.com/renaissance-benchmarks/renaissance/

>
> Thanks,
> Francesco
>
> Il ven 17 gen 2020, 22:40 Doug Lea via Concurrency-interest
> <[hidden email]
> <mailto:[hidden email]>> ha scritto:
>
>
>     Now committed in jsr166 are extensions of the AQS refresh of last fall
>     to also cover ForkJoin; among other things removing reliance on builtin
>     monitors  The extra few months were mainly a result of noticing that the
>     initial way I did this increased performance differences across garbage
>     collectors. These are now reduced (not increased), but still worth
>     noting: FJ programs are relatively sensitive to GC choice because they
>     entail work-stealing duels between program and collector. On hotspot,
>     usually the best throughput for divide-and-conquer style usages
>     (including parallelStreams) is with -XX:+UseParallelGC. For unstructured
>     computations with lots of little tasks (for example little
>     CompletableFutures), UseShenandoahGC (and/or if needing more than 32GB
>     heap, UseZGC) are often better choices.  UseParallelGC meshes well with
>     highly generational structured parallelism, but is prone to
>     card-mark-contention with small tasks (even with -XX:+UseCondCardMark)
>     and long pauses. The default UseG1GC tends to be somewhere in the
>     middle. Also, on linux, using -XX:+UseTransparentHugePages seems to
>     always be a good idea for programs using FJ. (Other switches seem to
>     have less consistent positive effects.)
>
>     These days, it is almost impossible to quantify exactly how much better
>     performance is, but it's likely that your usages are faster. For
>     example, about half of the programs in the recent Renaissance Suite
>     (https://renaissance.dev/) somehow use FJ, and most of these have
>     improved scores on most machines, most JVMs (including OpenJ9 and
>     Graal), most collectors, most phases of the moon....
>
>     It would be great to get feedback from other usages before integrating
>     into OpenJDK. As usual, you can try it with any JDK11+ JVM by grabbing
>     http://gee.cs.oswego.edu/dl/concurrent/dist/jsr166.jar and running "java
>     --patch-module java.base="$DIR/jsr166.jar", where DIR is the full file
>     prefix. (Although beware that using --patch-module slows down startup,
>     and occasionally entire test runs. For other options, see
>     http://gee.cs.oswego.edu/dl/concurrency-interest/index.html).
>
>     -Doug
>
>     _______________________________________________
>     Concurrency-interest mailing list
>     [hidden email]
>     <mailto:[hidden email]>
>     http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>


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

Re: ForkJoin refresh

JSR166 Concurrency mailing list
In reply to this post by JSR166 Concurrency mailing list
On 1/17/20 4:54 PM, Dr Heinz M. Kabutz wrote:
> Thanks Doug - how different is this to the Java 14-EA version?  I

One thing you might notice is the lack of performance oddities when
tasks unblock but GC hasn't yet unbiased locks. Also better behaved when
a bunch of exceptions are thrown by tasks. Also less flailing with
relatively slow streams of incoming tasks. Plus the other improvements I
mentioned

> noticed the ManagedBlocker implementation today whilst showing the
> Condition.await() implementation to a class I was teaching and even
> tweeted about it: https://twitter.com/heinzkabutz/status/1218204292626243585
>

Right. I had forgotten that because few people use non-LTS JDKs, most
haven't seen that blocking on BlockingQueues etc, along with most other
j.u.c locking/sync work better especially in FJ programs.

-Doug


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

Re: ForkJoin refresh

JSR166 Concurrency mailing list
In reply to this post by JSR166 Concurrency mailing list

I mentioned a few weeks ago that most uses of
Executors.newFixedThreadPool(n) (based on TPE ThreadPoolExecutor) should
instead use "new ForkJoinPool(n)", This led to more systematic removal
of FJP-TPE differences in new FJ update. Among the issues are default
interrupt policies for ForkJoinTasks that wrap Callables and Runnables.
The ExecutorService invokeAny and invokeAll methods for arrays of
Callables are specified in a way that makes users expect that cancelled
tasks are interrupted. So they now are. Further, in addition to
internally forcing this for only these methods, there is now a way that
anyone can obtain this effect for any wrapped Callable (exposing a hack
that I've suggested in response to desperate queries). New method
adaptInterruptible(Callable) has class-level description as follows:

 *
 * <p>By default, method {@link #cancel} ignores its {@code
 * mayInterruptIfRunning} argument, separating task cancellation from
 * the interruption status of threads running tasks. However, the
 * method is overridable to accommodate cases in which running tasks
 * must be cancelled using interrupts. This may arise when adapting
 * Callables that cannot check {@code isCancelled()} task status.
 * Tasks constructed with the (@link #adaptInterruptible) adaptor
 * track and interrupt the running thread upon {@code
 * cancel(true)}. Reliable usage requires awareness of potential
 * consequences: Method bodies should ignore stray interrupts to cope
 * with the inherent possibility that a late interrupt issued by
 * another thread after a given task has completed may (inadvertently)
 * interrupt some future task. Further, interruptible tasks should not
 * in general create subtasks, because an interrupt intended for a
 * given task may be consumed by one of its subtasks, or vice versa.
 *


(Formatted in context at:
http://gee.cs.oswego.edu/dl/jsr166/dist/docs/java.base/java/util/concurrent/ForkJoinTask.html)

The intent of this description is to also help explain why most
ForkJoinTasks should not and do not interrupt when cancelling. Doing
otherwise would lead to constant surprises about both "missed" and
"spurious" interrupts, without any sure way to fix these problems. But
adaptInterruptible can still be useful when restricted to those
occasional cases (often legacy code) where such things can be tolerated.

Beyond these, the only difference between fixed-pool TPE and FJP that
could conceivably matter is that FJP.shutdownNow does not return a
collection of tasks that did not run because of pool termination.
(Because there is no central queue, it does not know which tasks are
cancelled due to termination vs otherwise.)

Comments and suggestions are welcome.

On 1/17/20 4:39 PM, Doug Lea via Concurrency-interest wrote:
> It would be great to get feedback from other usages before integrating
> into OpenJDK. As usual, you can try it with any JDK11+ JVM by grabbing
> http://gee.cs.oswego.edu/dl/concurrent/dist/jsr166.jar and running "java
> --patch-module java.base="$DIR/jsr166.jar", where DIR is the full file
> prefix. (Although beware that using --patch-module slows down startup,
> and occasionally entire test runs. For other options, see
> http://gee.cs.oswego.edu/dl/concurrency-interest/index.html).
>

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

Re: ForkJoin refresh

JSR166 Concurrency mailing list
Hi,

On 2/4/20 8:52 PM, Doug Lea via Concurrency-interest wrote:
>   * Tasks constructed with the (@link #adaptInterruptible) adaptor
>   * track and interrupt the running thread upon {@code
>   * cancel(true)}. Reliable usage requires awareness of potential
>   * consequences: Method bodies should ignore stray interrupts to cope
>   * with the inherent possibility that a late interrupt issued by
>   * another thread after a given task has completed may (inadvertently)
>   * interrupt some future task. Further, interruptible tasks should not
>   * in general create subtasks, because an interrupt intended for a
>   * given task may be consumed by one of its subtasks, or vice versa.

Would something like the following be more "precise" in targeting the
task and only the task and not any other code that executes after the
task in the same thread? At least stray interrupts would not be possible
as a consequence of cancel(true) for some unrelated task. Should anyone
interrupt the pool thread outside of normal task execution, the
interrupted status would not propagate to task code nor would it be lost
after the task has finished.

The hot path cost of this would be a CAS instead of a volatile write to
the runner field at the end of each task.

     static final class AdaptedInterruptibleCallable<T> extends
ForkJoinTask<T>
         implements RunnableFuture<T> {
         @SuppressWarnings("serial") // Conditionally serializable
         final Callable<? extends T> callable;
         @SuppressWarnings("serial") // Conditionally serializable
         transient volatile Thread runner;
         @SuppressWarnings("serial") // Conditionally serializable
         transient volatile boolean cancelInterrupted;
         T result;

         AdaptedInterruptibleCallable(Callable<? extends T> callable) {
             if (callable == null) throw new NullPointerException();
             this.callable = callable;
         }

         public final T getRawResult() { return result; }

         public final void setRawResult(T v) { result = v; }

         public final boolean exec() {
             // save the pre-task interrupted status and clear it for task
             boolean wasInterrupted = Thread.interrupted();
             Thread t = Thread.currentThread();
             runner = t;
             try {
                 result = callable.call();
                 return true;
             } catch (RuntimeException rex) {
                 throw rex;
             } catch (Exception ex) {
                 throw new RuntimeException(ex);
             } finally {
                 if (!RUNNER.compareAndSet(this, t, (Thread) null)) {
                     // someone called cancel(true) and successfully CASed
                     // runner to null - we should wait for it to set the
                     // cancelInterrupted flag...
                     while (!cancelInterrupted) {
                         Thread.onSpinWait();
                     }
                     // ... and then clear the interrupt it set just
before that
                     Thread.interrupted();
                 }
                 // restore pre-task interrupted status
                 if (wasInterrupted) {
                     Thread.currentThread().interrupt();
                 }
             }
         }

         public final void run() { invoke(); }

         public final boolean cancel(boolean mayInterruptIfRunning) {
             Thread t;
             boolean stat = super.cancel(false);
             if (mayInterruptIfRunning && (t = runner) != null &&
                 RUNNER.compareAndSet(this, t, (Thread) null)) {
                 try {
                     t.interrupt();
                 } catch (Throwable ignore) {
                 }
                 cancelInterrupted = true;
             }
             return stat;
         }

         public String toString() {
             return super.toString() + "[Wrapped task = " + callable + "]";
         }

         private static final VarHandle RUNNER;
         static {
             try {
                 RUNNER = MethodHandles
                     .lookup()
.findVarHandle(AdaptedInterruptibleCallable.class, "runner", Thread.class);
             } catch (NoSuchFieldException | IllegalAccessException e) {
                 throw new InternalError(e);
             }
         }

         private static final long serialVersionUID = 2838392045355241008L;
     }




Regards, Peter

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

Re: ForkJoin refresh

JSR166 Concurrency mailing list
Ooops,

There's also a race at the beginning of task execution. The task might
already be executing and another thread might call cancel(true) even
before the 'runner' field is set. In this case, the code of task will
not get interrupted. So we should check the cancelation status after
setting the 'runner' field and bail out if it was canceled.

This might do:

     static final class AdaptedInterruptibleCallable<T> extends
ForkJoinTask<T>
         implements RunnableFuture<T> {
         @SuppressWarnings("serial") // Conditionally serializable
         final Callable<? extends T> callable;
         transient volatile Thread runner;
         transient volatile boolean cancelInterrupted;
         @SuppressWarnings("serial") // Conditionally serializable
         T result;

         AdaptedInterruptibleCallable(Callable<? extends T> callable) {
             if (callable == null) throw new NullPointerException();
             this.callable = callable;
         }

         public final T getRawResult() { return result; }

         public final void setRawResult(T v) { result = v; }

         public final boolean exec() {
             Thread t = Thread.currentThread();
             // set runner filed
             runner = t;
             try {
                 // check for concurrent cancel() that just missed the
runner field
                 if (isCancelled()) {
                     throw new CancellationException();
                 } else {
                     result = callable.call();
                 }
                 return true;
             } catch (RuntimeException rex) {
                 throw rex;
             } catch (Exception ex) {
                 throw new RuntimeException(ex);
             } finally {
                 if (!RUNNER.compareAndSet(this, t, (Thread) null)) {
                     // someone called cancel(true) and successfully CASed
                     // runner to null - we should wait for it to set the
                     // cancelInterrupted flag...
                     while (!cancelInterrupted) {
                         Thread.onSpinWait();
                     }
                     // ... and then clear the interrupt it set just
before that
                     Thread.interrupted();
                 }
             }
         }

         public final void run() { invoke(); }

         public final boolean cancel(boolean mayInterruptIfRunning) {
             Thread t;
             boolean stat = super.cancel(false);
             if (mayInterruptIfRunning && (t = runner) != null &&
                 RUNNER.compareAndSet(this, t, (Thread) null)) {
                 try {
                     t.interrupt();
                 } catch (Throwable ignore) {
                 }
                 cancelInterrupted = true;
             }
             return stat;
         }

         public String toString() {
             return super.toString() + "[Wrapped task = " + callable + "]";
         }

         private static final VarHandle RUNNER;
         static {
             try {
                 RUNNER = MethodHandles
                     .lookup()
.findVarHandle(AdaptedInterruptibleCallable.class, "runner", Thread.class);
             } catch (NoSuchFieldException | IllegalAccessException e) {
                 throw new InternalError(e);
             }
         }

         private static final long serialVersionUID = 2838392045355241008L;
     }


Regards, Peter

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

Re: ForkJoin refresh

JSR166 Concurrency mailing list
In reply to this post by JSR166 Concurrency mailing list
On 2/5/20 10:15 AM, Peter Levart wrote:

>
> Would something like the following be more "precise" in targeting the
> task and only the task and not any other code that executes after the
> task in the same thread? At least stray interrupts would not be possible
> as a consequence of cancel(true) for some unrelated task. Should anyone
> interrupt the pool thread outside of normal task execution, the
> interrupted status would not propagate to task code nor would it be lost
> after the task has finished.
>

Thanks for taking a look at this. I'm not sure your suggested changes
are preferable though. Interrupt status for interruptible tasks is
cleared at end, before running next task, which reduces the impact of
other slow threads needlessly interrupting the task. I can't think of
cases where it helps to conditionalize this on whether any occurred?
To better explain though, I'll add to internal documentation that the
only occurrence of Thread.interrupt inside FJ code itself is during
termination, in which case tasks are or will be cancelled anyway.

-Doug

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

Re: ForkJoin refresh

JSR166 Concurrency mailing list


On 2/6/20 1:31 PM, Doug Lea wrote:

> On 2/5/20 10:15 AM, Peter Levart wrote:
>
>> Would something like the following be more "precise" in targeting the
>> task and only the task and not any other code that executes after the
>> task in the same thread? At least stray interrupts would not be possible
>> as a consequence of cancel(true) for some unrelated task. Should anyone
>> interrupt the pool thread outside of normal task execution, the
>> interrupted status would not propagate to task code nor would it be lost
>> after the task has finished.
>>
> Thanks for taking a look at this. I'm not sure your suggested changes
> are preferable though. Interrupt status for interruptible tasks is
> cleared at end, before running next task, which reduces the impact of
> other slow threads needlessly interrupting the task.
It may reduce the impact, but it can't guarantee that it won't happen.
There is a theoretical race in cancel(true):

1455                 if (mayInterruptIfRunning && (t = runner) != null) {
1456                     try {
1457                         t.interrupt();
1458                     } catch (Throwable ignore) {
1459                     }
1460                 }

...between the read of the 'runner' field and the call to interrupt()
the 't' thread may already transition to running another unrelated task
which will get a stray interrupt then...

If stray interrupts are not problematic, then what about the race that
causes a missing interrupt at the start of execution of the task:

1436             public final boolean exec() {
1437                 Thread.interrupted();
1438                 runner = Thread.currentThread();
1439                 try {
1440                     result = callable.call();
1441                     return true;

The task might already be executing (just entering exec method), but has
not yet assigned the 'runner' field. A concurrent thread with access to
the task may call cancel(true) which will not interrupt the task,
because it observes the runner still being null. So the task will
execute user code to the end although user code might have checks to
detect interrupts. In my 2nd message I proposed to add a check after
assigning the runner field:

             Thread.interrupted();
             runner = Thread.currentThread();
             try {
                 // check for concurrent cancel() that just missed the
runner field
                 if (isCancelled()) {
                     throw new CancellationException();
                 } else {
                     result = callable.call();
                 }
                 return true;

The cancelation and the read of 'runner' field in cancel() are reversed,
so this should guarantee that either interrupt is observable by user
code or the task ends execution before it enters user code (Callable):

1452             public final boolean cancel(boolean
mayInterruptIfRunning) {
1453                 Thread t;
1454                 boolean stat = super.cancel(false);
1455                 if (mayInterruptIfRunning && (t = runner) != null) {
1456                     try {
1457                         t.interrupt();


Regards, Peter

>   I can't think of
> cases where it helps to conditionalize this on whether any occurred?
> To better explain though, I'll add to internal documentation that the
> only occurrence of Thread.interrupt inside FJ code itself is during
> termination, in which case tasks are or will be cancelled anyway.
>
> -Doug
>

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

Re: ForkJoin refresh

JSR166 Concurrency mailing list
On 2/6/20 10:56 AM, Peter Levart wrote:
>
> If stray interrupts are not problematic, then what about the race that
> causes a missing interrupt at the start of execution of the task:

Thanks; I agree. It suffices to just recheck status. (The outcome of
CancellationException will be reported without needing to throw it here.)

1436             public final boolean exec() {
1437                 Thread.interrupted();
1438                 runner = Thread.currentThread();
1439                 try {
+                      if (!isDone())
1440                     result = callable.call();
1441                     return true;


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