Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

TaskQueue

#TaskQueue I'mI'm not particularly fond of the name m_done. I would prefer it semantically inverted and called m_enabled. Also it looks like m_done is never set from inside of the queue and you could use std::atomic<bool> for it and avoid a bunch of locking when you query the queue. It seems you already handle it changing dynamically and mid processing.

WorkStealingQueueThreadPool

#WorkStealingQueueThreadPool TheThe name is a bit convoluted and too many words. I'd look for something shorter.

#TaskQueue I'm not particularly fond of the name m_done. I would prefer it semantically inverted and called m_enabled. Also it looks like m_done is never set from inside of the queue and you could use std::atomic<bool> for it and avoid a bunch of locking when you query the queue. It seems you already handle it changing dynamically and mid processing.

#WorkStealingQueueThreadPool The name is a bit convoluted and too many words. I'd look for something shorter.

TaskQueue

I'm not particularly fond of the name m_done. I would prefer it semantically inverted and called m_enabled. Also it looks like m_done is never set from inside of the queue and you could use std::atomic<bool> for it and avoid a bunch of locking when you query the queue. It seems you already handle it changing dynamically and mid processing.

WorkStealingQueueThreadPool

The name is a bit convoluted and too many words. I'd look for something shorter.

deleted 171 characters in body
Source Link
Emily L.
  • 16.7k
  • 1
  • 39
  • 89

Did you mean to use std::min here:

explicit WorkStealingQueueThreadPool(size_t threadCount = 
 std::max(2u, std::thread::hardware_concurrency()));

?

The variable m_queueCount is useless. Remove it and simply use m_queues.size().

Did you mean to use std::min here:

explicit WorkStealingQueueThreadPool(size_t threadCount = 
 std::max(2u, std::thread::hardware_concurrency()));

?

The variable m_queueCount is useless. Remove it and simply use m_queues.size().

The variable m_queueCount is useless. Remove it and simply use m_queues.size().

added 170 characters in body
Source Link
Emily L.
  • 16.7k
  • 1
  • 39
  • 89

This isn't necessary:

for (auto& thread : m_threads)
 thread.join();

because std::thread is automatically joined on destruction.

(削除) This isn't necessary:

for (auto& thread : m_threads)
 thread.join();

because std::thread is automatically joined on destruction. (削除ここまで) My memory has failed me,if the thread isn't joined on destruction std::terminate is called .

This isn't necessary:

for (auto& thread : m_threads)
 thread.join();

because std::thread is automatically joined on destruction.

(削除) This isn't necessary:

for (auto& thread : m_threads)
 thread.join();

because std::thread is automatically joined on destruction. (削除ここまで) My memory has failed me,if the thread isn't joined on destruction std::terminate is called .

Source Link
Emily L.
  • 16.7k
  • 1
  • 39
  • 89
Loading
lang-cpp

AltStyle によって変換されたページ (->オリジナル) /