2016-01-25 44 views
7

まず最初に、このWebサイトの他のトピックを見て、I/O操作やスレッド作成のオーバーヘッドを使用している人たちを主に扱っているため、私の問題には関係しないことがわかりました。私の問題は、私のスレッドプールやワーカータスク構造の実装が(この場合)シングルスレッドよりもかなり遅いということです。私はこれで本当に混乱していて、ThreadPool、タスクそのもの、どうやってテストするのか、スレッドの性質などはわかりません。 VS2013プロファイラとC++:シングルスレッドよりもスレッドプールが遅い?

// Sorry for the long code 
#include <vector> 
#include <queue> 

#include <thread> 
#include <mutex> 
#include <future> 

#include "task.hpp" 

class ThreadPool 
{ 
public: 
    ThreadPool() 
    { 
     for (unsigned i = 0; i < std::thread::hardware_concurrency() - 1; i++) 
      m_workers.emplace_back(this, i); 

     m_running = true; 
     for (auto&& worker : m_workers) 
      worker.start(); 
    } 
    ~ThreadPool() 
    { 
     m_running = false; 
     m_task_signal.notify_all(); 
     for (auto&& worker : m_workers) 
      worker.terminate(); 
    } 

    void add_task(Task* task) 
    { 
     { 
      std::unique_lock<std::mutex> lock(m_in_mutex); 
      m_in.push(task); 
     } 
     m_task_signal.notify_one(); 
    } 
private: 
    class Worker 
    { 
    public: 
     Worker(ThreadPool* parent, unsigned id) : m_parent(parent), m_id(id) 
     {} 
     ~Worker() 
     { 
      terminate(); 
     } 

     void start() 
     { 
      m_thread = new std::thread(&Worker::work, this); 
     } 
     void terminate() 
     { 
      if (m_thread) 
      { 
       if (m_thread->joinable()) 
       { 
        m_thread->join(); 
        delete m_thread; 
        m_thread = nullptr; 
        m_parent = nullptr; 
       } 
      } 
     } 
    private: 
     void work() 
     { 
      while (m_parent->m_running) 
      {    
       std::unique_lock<std::mutex> lock(m_parent->m_in_mutex); 
       m_parent->m_task_signal.wait(lock, [&]() 
       { 
        return !m_parent->m_in.empty() || !m_parent->m_running; 
       }); 

       if (!m_parent->m_running) break; 
       Task* task = m_parent->m_in.front(); 
       m_parent->m_in.pop(); 
       // Fixed the mutex being locked while the task is executed 
       lock.unlock(); 

       task->execute();    
      } 
     } 
    private: 
     ThreadPool* m_parent = nullptr; 
     unsigned m_id = 0; 

     std::thread* m_thread = nullptr; 
    }; 
private: 
    std::vector<Worker> m_workers; 

    std::mutex m_in_mutex; 
    std::condition_variable m_task_signal; 
    std::queue<Task*> m_in; 

    bool m_running = false; 
}; 

class TestTask : public Task 
{ 
public: 
    TestTask() {} 
    TestTask(unsigned number) : m_number(number) {} 

    inline void Set(unsigned number) { m_number = number; } 

    void execute() override 
    { 
     if (m_number <= 3) 
     { 
      m_is_prime = m_number > 1; 
      return; 
     } 
     else if (m_number % 2 == 0 || m_number % 3 == 0) 
     { 
      m_is_prime = false; 
      return; 
     } 
     else 
     { 
      for (unsigned i = 5; i * i <= m_number; i += 6) 
      { 
       if (m_number % i == 0 || m_number % (i + 2) == 0) 
       { 
        m_is_prime = false; 
        return; 
       } 
      } 
      m_is_prime = true; 
      return; 
     } 
    } 
public: 
    unsigned m_number = 0; 
    bool m_is_prime = false; 
}; 

int main() 
{ 
    ThreadPool pool; 

    unsigned num_tasks = 1000000; 
    std::vector<TestTask> tasks(num_tasks); 
    for (auto&& task : tasks) 
     task.Set(randint(0, 1000000000)); 

    auto s = std::chrono::high_resolution_clock::now(); 
    #if MT 
    for (auto&& task : tasks) 
     pool.add_task(&task); 
    #else 
    for (auto&& task : tasks) 
     task.execute(); 
    #endif 
    auto e = std::chrono::high_resolution_clock::now(); 
    double seconds = std::chrono::duration_cast<std::chrono::nanoseconds>(e - s).count()/1000000000.0; 
} 

ベンチマーク:そのような答えで

10,000,000 tasks: 
    MT: 
     13 seconds of wall clock time 
     93.36% is spent in msvcp120.dll 
     3.45% is spent in Task::execute() // Not good here 
    ST: 
     0.5 seconds of wall clock time 
     97.31% is spent with Task::execute() 
+3

スタートあなたが測定を行う方法を、あなたの「時間がかかる」コードを示す、そしてどのようにあなたがそれをコンパイルすると:

は、ここで「ロックなし」の実装です。重要である可能性があります。 – deviantfan

+0

@deviantfan私はあまりにも遅れてそのミスをキャッチしました。改訂された回答。 – Jarann

+0

コアの数はいくつですか?マルチスレッド化されたコードが1つだけであれば、単一のコードよりも簡単に遅くなる可能性があります。 –

答えて

6

通常の免責事項:確かに伝えるための唯一の方法プロファイラツールでそれを測定することです。

しかし、私はそれなしであなたの結果を説明しようとします。まず、すべてのスレッドに1つのミューテックスがあります。したがって、一度に1つのスレッドだけが何らかのタスクを実行できます。あなたの持つすべての利益を殺します。あなたのスレッドにもかかわらず、あなたのコードは完全にシリアルです。だから少なくとも、ミューテックスからあなたのタスクを実行してください。キューからタスクを取り出すには、ミューテックスをロックする必要があります。タスクが実行されたときに保持する必要はありません。

次に、タスクが非常に簡単であるため、1つのスレッドで短時間で実行できます。あなたはそのような仕事でどんな利益も測定できません。もっと面白い結果を生み出すことができる重いタスクをいくつか作成してください(そのような人為的なものではなく、現実世界に近いものもあります)。

第3のポイント:スレッドは、コストコンテキスト切り替え、ミューテックス競合などがないわけではありません。実際の利益を得るには、前の2つのポイントのように、スレッドが導入するオーバーヘッドよりも時間がかかるタスクが必要です。シリアルにするリソースを待つ代わりに、コードは本当に並列でなければなりません。

UPD:私はコードの間違った部分を見ました。タスクは、十分に大きな数のタスクを作成すれば十分に複雑です。


UPD2:私はあなたのコードで演奏し、MTコードが優れているかを示すために良い素数を発見しました。次の素数を使用してください:1019048297。これは、その違いを示すのに十分な計算量を与えます。

しかし、なぜあなたのコードは良い結果をもたらさないのですか? randint()の実装を見ずにはわかりにくいですが、私はそれをかなりシンプルにしています。半分のケースでは、偶数やその他のケースでも大きな素数を生成しません。そのため、タスクは非常に簡単で、特定の実装やスレッドのコンテキスト切り替えやその他の処理は、一般的に計算より時間がかかります。私が与えた素数を使用すると、タスクは選択肢が与えられず、時間計算に費やされます。数字が大きく、実際に素数であるため簡単な答えはありません。だからこそ、大きな数字があなたに求めている答え、つまりMTコードのより良い時間を与えるのです。

+0

大変なことはありません。 – Jarann

+0

@Jamesについては、更新された回答をご覧ください。私はあなたのコードの間違った部分を見た – ixSci

+0

私はミューテックスの問題を修正し、ベンチマークを更新するためにプロファイラを使用しました – Jarann

2

タスクが実行なっている間は、そうでない場合は、他のスレッドがタスクを取得することができなくなり、ミューテックスを保持するべきではありません。それぞれのフェーズに費やされているどのくらいの時間MTについては、

void work() { 
    while (m_parent->m_running) { 
     Task* currentTask = nullptr;  
     std::unique_lock<std::mutex> lock(m_parent->m_in_mutex); 
     m_parent->m_task_signal.wait(lock, [&]() { 
      return !m_parent->m_in.empty() || !m_parent->m_running; 
     });      
     if (!m_parent->m_running) continue; 
     currentTask = m_parent->m_in.front(); 
     m_parent->m_in.pop();    
     lock.unlock(); //<- Release the lock so that other threads can get tasks 
     currentTask->execute(); 
     currentTask = nullptr; 
    } 
}  
+0

私はこれをixSciの回答 – Jarann

+0

@Jamesから修正しました。聞いてよかったですが、問題を解決するのに役立つ回答を必ずマークしてください。 –

1

を」オーバーヘッド ":std::unique_lock,m_task_signal.wait,front,pop,unlock

3%の有益な結果しか得られていないことから、上記は97%を消費することを意味します。上記の各部分の番号を取得します(各呼び出しの間にタイムスタンプを追加するなど)。

あなたが[単に]次のタスクポインタをデキューするのに使うコードはかなり重いようです。私ははるかに単純なキュー[おそらくロックレス]メカニズムをやっています。または、アトミックを使用して、上記の5つのステップのプロセスではなく、キューにインデックスをバンプすることもできます。たとえば:

void 
work() 
{ 
    while (m_parent->m_running) { 
     // NOTE: this is just an example, not necessarily the real function 
     int curindex = atomic_increment(&global_index); 
     if (curindex >= max_index) 
      break; 

     Task *task = m_parent->m_in[curindex]; 

     task->execute(); 
    } 
} 

はまた、多分あなたの代わりに一つだけの時に[発言] 10ポップ必要があります。

また、メモリにバインドされているか、「タスクスイッチ」にバインドされている可能性があります。 (例えば、)配列にアクセスするスレッドの場合、通常4つ以上のスレッドがメモリバスを飽和させる。 [新しいunlock呼び出しであっても、間接的に]ロックを独占しているためスレッドが枯渇するような重度の競合が発生する可能性があります。

通常、スレッド間ロックは、それらのアウト・オブ・オーダー実行パイプラインを同期させる。

void 
work() 
{ 
    // assume m_id is 0,1,2,... 
    int curindex = m_id; 

    while (m_parent->m_running) { 
     if (curindex >= max_index) 
      break; 

     Task *task = m_parent->m_in[curindex]; 

     task->execute(); 

     curindex += NUMBER_OF_WORKERS; 
    } 
} 
+0

私は多分4(または任意の複数の)タスクをプッシュし、すべての4つのスレッドに通知し、タスクの量を取得する同じアイデアを持っていた。まだ完全には学んでいないので、私はちょっとアトミックを避けていましたが、一見すると自分の設定よりも優れています。私はそれを試してみましょう。 – Jarann

+0

ここにリンクがあります:http://stackoverflow.com/questions/33083270/atomically-increment-two-integers-with-casこれは私が作成したCAS実装です。しかし、もっと重要なのは、内部にcppconで提供されているロックレスに関するビデオトークのリンクがあることです –

関連する問題