WIP: Threading upgrade

[dogancan]

This is work in progress. See #1234 for earlier discussion. Note that this PR includes the changes from #1249 and is mainly for comments and discussion. Threading related changes are at the end of the commit list.

To-do list:

• Reimplement Semaphore and multi-threaded parallelization utilities (MultiThreader, TaskSequencer) using STL thread support library instead of pthread.
• Reorganize the code in src/thread/.
  • Move the code in src/thread/kaldi-task-sequence.h to src/thread/kaldi-thread.h. Move the tests in src/thread/kaldi-task-sequence-test.cc to src/thread/kaldi-thread-test.cc.
  • Move src/thread/kaldi-semaphore.{h,cc}, src/thread/kaldi-thread.{h,cc} and src/thread/kaldi-thread-test.cc to src/util/.
  • Remove the remaning code in src/thread/ (kaldi-mutex.{h,cc}, kaldi-barrier.{h,cc}) that is no longer used/needed.
• Update codebase to use STL thread support library and the new threading code under src/util.
• Update build system to build and link with the new threading code under src/util.
• Improve the implementation of multi-threaded parallelization utilities (MultiThreader, TaskSequencer) ?
• A good amount of pthread related error checking code was removed. Do we need/want to introduce try/catch blocks to handle exceptions thrown by std::thread? Decided not to handle exceptions.
• Regression tests.

@danpovey, @jtrmal: I think this is more or less ready for review.

[danpovey]

@dogancan, are you still planning to work on this?

[dogancan]

@danpovey, @jtrmal: As far as I recall, I left this in a state ready for test/review.

Since threading code can be error prone and some changes in this PR are non-trivial, I would appreciate if someone can do a careful review. I would also appreciate if someone can test the new threading code on example setups.

The other item left on the list is the refactoring/rewrite of the multi-threaded parallelization utilities (MultiThreader, TaskSequencer), which I guess is somewhat orthogonal to the changes in this PR so that can be done later.

[danpovey]

Oh, great. I didn't realize it was ready for review.
I just had a look through it and it looks straightforward.
Would you mind resolving the Makefile conflict? Then I can merge.
It will cause "make" to complain about unmet dependencies, but people need to learn to do "make depend" when that happens.

[dogancan]

Sure, I can take care of the conflicts. Please note that although most of the changes are straightforward, as far as I recall there were some non-trivial changes in this PR.

Also, what do you think about the following items?

• Removing the remaining code in src/thread/ (kaldi-mutex.{h,cc}, kaldi-barrier.{h,cc}) that is no longer used/needed. I don't think there is any point in keeping these around after this change.
• A good amount of pthread related error checking code was removed. Do we need/want to introduce try/catch blocks to handle exceptions thrown by std::thread? I don't think we want to catch those exceptions.

[danpovey]

I agree with those changes.

…

On Wed, May 24, 2017 at 4:10 PM, Dogan Can ***@***.***> wrote: Sure, I can take care of the conflicts. Please note that although most of the changes are straightforward, as far as I recall there were some non-trivial changes in this PR. Also, what do you think about the following items? - Removing the remaining code in src/thread/ (kaldi-mutex.{h,cc}, kaldi-barrier.{h,cc}) that is no longer used/needed. I don't think there is any point in keeping these around after this change. - A good amount of pthread related error checking code was removed. Do we need/want to introduce try/catch blocks to handle exceptions thrown by std::thread? I don't think we want to catch those exceptions. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1350 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADJVuxzYTp4Ocl1PAfon8UX_9pmXsSC1ks5r9I7SgaJpZM4LmnoP> .

[dogancan]

@danpovey I resolved the conflicts and removed the src/thread directory.

[danpovey]

OK, thanks. Merging.