Single-threaded ForkJoinPool

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

Re: Single-threaded ForkJoinPool

cowwoc
Doug,

As an aside, do you plan on fixing tryAddWorker() so it retries launching a new thread if one is still needed? Your workaround will probably work for me, but the current implementation is still vulnerable to race conditions if the thread factory ever runs null. This could impact people with different use-cases than my own, where it doesn't make sense to join on the existing thread.

Gili

On 2016-02-11 9:49 AM, cowwoc wrote:
Not a bad idea :) I'll give it a try.

Gili

On 2016-02-11 8:46 AM, Doug Lea [via JSR166 Concurrency] wrote:
On 02/11/2016 06:57 AM, Viktor Klang wrote:
> Haven't had time to think about this yet.

In his case, one way to force race outcome is for the factory
to join the existing single thread before returning it. This
is not in general a good practice of course...

-Doug



>
> I think in general it is best to model the task in terms of dependency rather
> than tweaking the execution engine to get the effect you want.
>
> --
> Cheers,
> √
>
> On Feb 8, 2016 11:00 AM, "cowwoc" <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     You're still vulnerable to the same race condition:
>
>      1. [main] Invokes ForkJoinPool.execute(task)
>      2. [WorkerThread] Finishes running a task, reaches the point immediately
>         before deregisterThread() at
>         https://github.com/scala/scala/blob/2.12.x/src/library/scala/concurrent/impl/ExecutionContextImpl.scala#L61
>      3. [main] ForkJoinPool invokes tryAddWorker(),
>         https://github.com/scala/scala/blob/2.12.x/src/library/scala/concurrent/impl/ExecutionContextImpl.scala#L65
>         returns null
>      4. [WorkerThread] Invokes deregisterThread().
>
>     Now we have an enqueued task, no worker threads and no one is trying to spin
>     up a new thread.
>
>     Gili
>
>     On 2016-02-08 4:53 AM, Viktor Klang wrote:
>>     I was thinking something along the lines of this:
>>
>>     https://github.com/scala/scala/blob/2.12.x/src/library/scala/concurrent/impl/ExecutionContextImpl.scala#L65
>>     https://github.com/scala/scala/blob/2.12.x/src/library/scala/concurrent/impl/ExecutionContextImpl.scala#L68
>>
>>     (no need for WeakReferences to Threads, synchronized etc)
>>
>>     On Mon, Feb 8, 2016 at 8:47 AM, cowwoc <[hidden email]
>>     <mailto:[hidden email]>> wrote:
>>
>>         So, it turns out this solution isn't actually safe.
>>
>>         1. I used the following thread factory:
>>
>>                         ForkJoinWorkerThreadFactory factory = new
>>         ForkJoinWorkerThreadFactory()
>>                         {
>>                             private WeakReference<Thread> currentWorker = new
>>         WeakReference<>(null);
>>
>>                             @Override
>>                             public synchronized ForkJoinWorkerThread
>>         newThread(ForkJoinPool pool)
>>                             {
>>                                 // If the pool already has a live thread,
>>         return null.
>>                                 Thread thread = currentWorker.get();
>>                                 if (thread != null && thread.isAlive())
>>                                 {
>>                                     System.out.println("Thread: " +
>>         thread.getName() + " is already alive, returning null.");
>>                                     return null;
>>                                 }
>>                                 ForkJoinWorkerThread result = new
>>         MyForkJoinWorkerThread(pool);
>>                                 currentWorker = new WeakReference<>(result);
>>                                 // According to Doug Lea this will reduce the
>>         probability of short livelocks
>>                                 Thread.yield();
>>                                 return result;
>>                             }
>>                         };
>>
>>         2. I started a debugging session.
>>         3. I suspended all threads long enough for the worker thread to get
>>         flagged as idle (approximately 10 seconds).
>>         4. I allowed all threads to continue execution.
>>         5. Upon resuming, the worker thread shut down and at the same time the
>>         factory printed "Thread: X is already alive, returning null."
>>         6. If I run the above scenario without suspending all threads (only
>>         suspending the "main" thread) then the worker thread shuts down, I
>>         resume execution, and a new worker thread spins up.
>>
>>         In other words, the proposed solution is vulnerable to a race condition.
>>
>>         Gili
>>
>>         On 2016-02-06 9:35 PM, cowwoc wrote:
>>>         Thanks Doug, I'll give this a try.
>>>
>>>         Thanks,
>>>         Gili
>>>
>>>         On 2016-02-06 3:02 PM, Doug Lea [via JSR166 Concurrency] wrote:
>>>>         On 02/06/2016 05:21 AM, Viktor Klang wrote:
>>>>         > What happens if you supply it with a thread factory which only
>>>>         allows a single
>>>>         > thread to be alive at a time, and returns null if it already has
>>>>         returned a
>>>>         > still living thread?
>>>>         >
>>>>
>>>>         Yes, this will work if you are positive that only one thread
>>>>         is required for liveness. FJ sometimes conservatively creates
>>>>         threads when it cannot itself guarantee liveness (for example,
>>>>         when GC or other system load causes stalls). But it will
>>>>         respond to null factory returns by rechecking, not failing.
>>>>         unless a thread really is needed to maintain liveness, in which
>>>>         case the program may livelock. To reduce transient near-livelock,
>>>>         you might want to place a Thread.yield() call before the
>>>>         "return null" in the factory.
>>>>
>>>>         -Doug
>>>>
>>>>
>>>>         > --
>>>>         > Cheers,
>>>>         > √
>>>>         >
>>>>         > On Feb 6, 2016 05:19, "cowwoc" <[hidden email]
>>>>         <http:///user/SendEmail.jtp?type=node&node=13243&i=0>
>>>>         > <mailto:[hidden email]
>>>>         <http:///user/SendEmail.jtp?type=node&node=13243&i=1>>> wrote:
>>>>         >
>>>>         >     Hi,
>>>>         >
>>>>         >     Is this the correct mailing list for discussing ForkJoinPool
>>>>         in JDK9? If
>>>>         >     not, please point me to the right place.
>>>>         >
>>>>         >     I have a feature request for ForkJoinPool which doesn't seem
>>>>         to be possible
>>>>         >     to implement without a JDK change:
>>>>         <http://stackoverflow.com/q/34012134/14731>http://stackoverflow.com/q/34012134/14731
>>>>         >
>>>>         >     Specifically, I need to be able to an application that uses
>>>>         Random and
>>>>         >     ForkJoinPool in a deterministic manner when
>>>>         debugging/profiling but run
>>>>         >     full-speed in normal execution mode. I have all the moving
>>>>         parts nailing
>>>>         >     down except for ForkJoinPool.
>>>>         >
>>>>         >     If I create ForkJoinPool with a parallelism of 1, sometimes I
>>>>         see two worker
>>>>         >     threads getting used. I am guessing that this is caused by
>>>>         >     ForkJoinTask.get() invoking
>>>>         ForkJoinPool.common.externalHelpComplete(), but
>>>>         >     maybe something else is going on.
>>>>         >
>>>>         >     Is there a way for me to guarantee that ForkJoinThread will
>>>>         use exactly 1
>>>>         >     worker thread, no less, no more? Would you like me to file a
>>>>         formal feature
>>>>         >     request?
>>>>         >
>>>>         >     Thank you,
>>>>         >     Gili
>>>>         >
>>>>         >
>>>>         >
>>>>         >     --
>>>>         >     View this message in context:
>>>>         >
>>>>         http://jsr166-concurrency.10961.n7.nabble.com/Single-threaded-ForkJoinPool-tp13232.html
>>>>         >     Sent from the JSR166 Concurrency mailing list archive at
>>>>         Nabble.com.
>>>>         > _______________________________________________
>>>>         >     Concurrency-interest mailing list
>>>>         > [hidden email]
>>>>         <http:///user/SendEmail.jtp?type=node&node=13243&i=2>
>>>>         <mailto:[hidden email]
>>>>         <http:///user/SendEmail.jtp?type=node&node=13243&i=3>>
>>>>         >
>>>>         <http://cs.oswego.edu/mailman/listinfo/concurrency-interest>http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>>>>         >
>>>>         >
>>>>         >
>>>>         > _______________________________________________
>>>>         > Concurrency-interest mailing list
>>>>         > [hidden email] <http:///user/SendEmail.jtp?type=node&node=13243&i=4>
>>>>         > http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>>>>         >
>>>>
>>>>
>>>>
>>>>         _______________________________________________
>>>>         Concurrency-interest mailing list
>>>>         [hidden email] <http:///user/SendEmail.jtp?type=node&node=13243&i=5>
>>>>         http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>>>>
>>>>
>>>>         --------------------------------------------------------------------------------
>>>>         If you reply to this email, your message will be added to the
>>>>         discussion below:
>>>>         http://jsr166-concurrency.10961.n7.nabble.com/Single-threaded-ForkJoinPool-tp13232p13243.html
>>>>
>>>>         To unsubscribe from Single-threaded ForkJoinPool, click here.
>>>>         NAML
>>>>         <http://jsr166-concurrency.10961.n7.nabble.com/template/NamlServlet.jtp?macro=macro_viewer&id=instant_html%21nabble%3Aemail.naml&base=nabble.naml.namespaces.BasicNamespace-nabble.view.web.template.NabbleNamespace-nabble.view.web.template.NodeNamespace&breadcrumbs=notify_subscribers%21nabble%3Aemail.naml-instant_emails%21nabble%3Aemail.naml-send_instant_email%21nabble%3Aemail.naml>
>>>>
>>>
>>
>>
>>         --------------------------------------------------------------------------------
>>         View this message in context: Re: Single-threaded ForkJoinPool
>>         <http://jsr166-concurrency.10961.n7.nabble.com/Single-threaded-ForkJoinPool-tp13232p13250.html>
>>
>>
>>         Sent from the JSR166 Concurrency mailing list archive
>>         <http://jsr166-concurrency.10961.n7.nabble.com/> at Nabble.com.
>>
>>         _______________________________________________
>>         Concurrency-interest mailing list
>>         [hidden email]
>>         <mailto:[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
>



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



If you reply to this email, your message will be added to the discussion below:
http://jsr166-concurrency.10961.n7.nabble.com/Single-threaded-ForkJoinPool-tp13232p13260.html
To unsubscribe from Single-threaded ForkJoinPool, click here.
NAML


Reply | Threaded
Open this post in threaded view
|

Re: Single-threaded ForkJoinPool

Doug Lea

Sorry for the delay...

On 02/11/2016 11:35 PM, cowwoc wrote:

> As an aside, do you plan on fixing tryAddWorker() so it retries launching a new
> thread if one is still needed?

It's a feature (not a bug) also used in ThreadPoolExecutor
to avoid infinitely looping failures (including for example
OutOfMemoryErrors):
A failed (null or exceptional) call to newThread is not
retried until the next opportunity to call it.
We should clarify the documentation, as below.

Not directly relevantly, but I'm also finally getting to think
that we should add some control/tuning parameters to ForkJoinPool.
More on that soon.


     /**
      * Factory for creating new {@link ForkJoinWorkerThread}s.
      * A {@code ForkJoinWorkerThreadFactory} must be defined and used
      * for {@code ForkJoinWorkerThread} subclasses that extend base
      * functionality or initialize threads with different contexts.
      */
     public static interface ForkJoinWorkerThreadFactory {
         /**
          * Returns a new worker thread operating in the given pool.
          * Returning null or throwing an exception may result in tasks
          * never being executed.  If this method throws an exception,
          * it is relayed to the caller of the method (for example
          * {@code execute}) causing attempted thread creation. If this
          * method returns null or throws an exception, it is not
          * retried until the next attempted creation (for example
          * another call to {@code execute}).
          *
          * @param pool the pool this thread works in
          * @return the new worker thread, or {@code null} if the request
          *         to create a thread is rejected.
          * @throws NullPointerException if the pool is null
          */
         public ForkJoinWorkerThread newThread(ForkJoinPool pool);
     }


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

Re: Single-threaded ForkJoinPool

cowwoc
+1 for the updated documentation.

Looking forward to hearing more about the ForkJoinPool control/turning parameters you have in mind.

Thanks,
Gili

On 2016-02-21 3:18 PM, Doug Lea [via JSR166 Concurrency] wrote:

Sorry for the delay...

On 02/11/2016 11:35 PM, cowwoc wrote:

> As an aside, do you plan on fixing tryAddWorker() so it retries launching a new
> thread if one is still needed?

It's a feature (not a bug) also used in ThreadPoolExecutor
to avoid infinitely looping failures (including for example
OutOfMemoryErrors):
A failed (null or exceptional) call to newThread is not
retried until the next opportunity to call it.
We should clarify the documentation, as below.

Not directly relevantly, but I'm also finally getting to think
that we should add some control/tuning parameters to ForkJoinPool.
More on that soon.


     /**
      * Factory for creating new {@link ForkJoinWorkerThread}s.
      * A {@code ForkJoinWorkerThreadFactory} must be defined and used
      * for {@code ForkJoinWorkerThread} subclasses that extend base
      * functionality or initialize threads with different contexts.
      */
     public static interface ForkJoinWorkerThreadFactory {
         /**
          * Returns a new worker thread operating in the given pool.
          * Returning null or throwing an exception may result in tasks
          * never being executed.  If this method throws an exception,
          * it is relayed to the caller of the method (for example
          * {@code execute}) causing attempted thread creation. If this
          * method returns null or throws an exception, it is not
          * retried until the next attempted creation (for example
          * another call to {@code execute}).
          *
          * @param pool the pool this thread works in
          * @return the new worker thread, or {@code null} if the request
          *         to create a thread is rejected.
          * @throws NullPointerException if the pool is null
          */
         public ForkJoinWorkerThread newThread(ForkJoinPool pool);
     }


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



If you reply to this email, your message will be added to the discussion below:
http://jsr166-concurrency.10961.n7.nabble.com/Single-threaded-ForkJoinPool-tp13232p13264.html
To unsubscribe from Single-threaded ForkJoinPool, click here.
NAML

12