New STL-only version

[sheljohn]

Hi vit-vit,

As mentioned in the head comments, I think I fixed a few unsafe spots / bugs in your original code, and overall I tried to make it more readable with reformatting and clearer variable names.

For the fixes:

• I think the push functions should be guarded in cases of ongoing interruption (old stop function);
• I think the reason why you had to call clear_queue twice in the original stop function is because the order of the booleans in the test you were using with idle functions was wrong (testing for more elements in the queue first, rather than interruptions flags first).

I also added a restart function that might be useful for temporary interrupts.

The expected output of your example is not straightforward to evaluate, but it compiles and seems to run fine with my new version; I'll let you be the judge of that :)

Let me know,
Cheers,
Jonathan

[vit-vit]

Hi Jonathan,

Sorry for my late response. I am very busy these times.
Thank you for your suggestions. I think they are worthwhile though not
very critical. I must consider them.

Thanks.
Cheers,
Vitaliy

On 23/10/15 2:06 AM, John H wrote:

Hi vit-vit,

As mentioned in the head comments, I think I fixed a few unsafe spots
/ bugs in your original code, and overall I tried to make it more
readable with reformatting and clearer variable names.

For the fixes:

• I think the push functions should be guarded in cases of ongoing
  interruption (old |stop| function);
• I think the reason why you had to call |clear_queue| twice in the
  original stop function is because the order of the booleans in the
  test you were using with idle functions was wrong (testing for
  more elements in the queue first, rather than interruptions flags
  first).

I also added a restart function that might be useful for temporary
interrupts.

The expected output of your example is not straightforward to
evaluate, but it compiles and seems to run fine with my new version;
I'll let you be the judge of that :)

Let me know,
Cheers,
Jonathan

---

    You can view, comment on, or merge this pull request online at:

#3

    Commit Summary

• propose edit of the STL-only version

  File Changes
  

• M ctpl_stl.h
  https://github.com/vit-vit/CTPL/pull/3/files#diff-0 (311)

• M example.cpp
  https://github.com/vit-vit/CTPL/pull/3/files#diff-1 (61)

  Patch Links:
  

• https://github.com/vit-vit/CTPL/pull/3.patch

• https://github.com/vit-vit/CTPL/pull/3.diff

—
Reply to this email directly or view it on GitHub
#3.

[jonathonracz]

I'd just like to chime in and say I've been using this version for a while now and haven't had any issues with it. I really enjoy the enhanced readability, and while I'm no expert with threading I can see where the bugfixes are coming in. Any chance this will get pulled at some point?

[VPaulV]

I think there is a small bug in reset function. Without resize() after reset() further processing will not be possible

index 69c4750..4e69079 100644
--- a/ctpl_stl.h
+++ b/ctpl_stl.h
@@ -81,7 +81,7 @@ namespace ctpl {
     public:

         thread_pool() { this->init(); }
-        thread_pool(size_t nThreads) { this->init(); this->resize(nThreads); }
+        thread_pool(size_t nThreads) { this->init(); m_nThreads = nThreads; this->resize(nThreads); }

         // the destructor waits for all the functions in the queue to be finished
         ~thread_pool() { this->interrupt(false); }
@@ -101,6 +101,7 @@ namespace ctpl {
         {
             this->interrupt(false); // finish all existing tasks but prevent new ones
             this->init(); // reset atomic flags
+            this->resize(m_nThreads);
         }

         // change the number of threads in the pool

@@ -275,6 +276,7 @@ namespace ctpl {

         std::atomic<bool>    ma_interrupt, ma_kill;
         std::atomic<size_t>  ma_n_idle;
+        std::atomic<size_t> m_nThreads;

         std::mutex               m_mutex;
         std::condition_variable  m_cond;

Another approach could be to redesign the interrupt function and don't remove threads there.