Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements and Simplifications to ThreadPool #3

Merged
merged 5 commits into from
Dec 29, 2021

Conversation

DeveloperPaul123
Copy link
Owner

Improvements/changes largely based on feedback from Code Review and Reddit

Changed

  • Simplified class definition and removed unused typedefs (YAGNI)
  • Also removed Queue template parameter (also YAGNI)
  • Use std::deque instead of std::queue in dp::thread_safe_queue
  • Use std::deque in dp::thread_pool so that usage of std::unique_ptr is no longer needed for internal task objects.

Fixed

  • Issues with dp::thread_safe_queue's usage of lock guards before notifying threads of new data being available.
  • Ensure that all tasks complete before the thread pool is destroyed.

Removed type definition of thread pool for thread_pool_impl. Simplified class by removing queue template parameter. Switched to using std::deque for task_queue objects so that we don't have to use unique_ptrs for those objects. Made pool non-copyable. Enqueue parameters by value for version that returns a future. Removed other unused type definitions.
Fix to push function to properly use the lock guard before notifying with the condition variable. Switched to using std::deque directly instead of std::queue for the underlying data.
Renamed old test and added new test to ensure that work completes before the thread pool gets destroyed.
Added way to ensure that tasks complete before destroying the thread pool.
@DeveloperPaul123 DeveloperPaul123 marked this pull request as ready for review December 29, 2021 16:10
@DeveloperPaul123 DeveloperPaul123 merged commit c1b91fd into master Dec 29, 2021
@DeveloperPaul123 DeveloperPaul123 deleted the feature/minor-improvements branch December 29, 2021 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant