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

Thread Pool Updates #255

Merged
merged 39 commits into from
Mar 7, 2015
Merged

Thread Pool Updates #255

merged 39 commits into from
Mar 7, 2015

Conversation

jdantonio
Copy link
Member

Addresses #248, #227, and #149.

Changelog

  • Added at_exit handler to Ruby thread pools (already in Java thread pools)
    • Ruby handler stores the object id and retrieves from ObjectSpace
    • JRuby disables ObjectSpace by default so that handler stores the object reference
  • Added a :stop_on_exit option to thread pools to enable/disable at_exit handler
  • Updated thread pool docs to better explain shutting down thread pools
  • Simpler :executor option syntax for all abstractions which support this option
  • Added Executor#auto_terminate? predicate method (for thread pools)
  • Added at_exit handler to TimerSet
  • Simplified auto-termination of the global executors
    • Can now disable auto-termination of global executors
    • Added shutdown/kill/wait_for_termination variants for global executors
  • Can now disable auto-termination for all executors (the nuclear option)
  • Simplified auto-termination of the global executors
  • Deprecated terms "task pool" and "operation pool"
    • New terms are "io executor" and "fast executor"
    • New functions added with new names
    • Deprecation warnings added to functions referencing old names
  • Moved all thread pool related functions from Concurrent::Configuration to Concurrent
    • Old functions still exist with deprecation warnings
    • New functions have updated names as appropriate
  • All high-level abstractions default to the "io executor"
  • Fixed bug in Actor causing it to prematurely warm global thread pools on gem load
    • This also fixed a RejectedExecutionError bug when running with minitest/autorun via JRuby
  • Added LazyReference, a simpler and faster varition of Delay
    • Updated most internal uses of Delay with LazyReference
  • Moved global logger up to the Concurrent namespace and refactored the code

To-Do

From #248

  • Add a new option to the constructor of the Java-backed thread pools that controls the creation of the at_exit handler
    • The default will be to enable the at_exit handler
    • This will support the "simple and easy" base case
    • And will ensure backward compatibility with the current release
  • Create an auto_shutdown? (or similarly named) predicate method to our Java-backed thread pools that indicates whether the at_exit handler has been set
  • Update the pure-Ruby thread pools with the exact same at_exit behavior as the Java-backed thread pools
    • It was an oversight that our pure-Ruby thread pools are not 100% compatible with our Java-backed thread pools
    • However, we need to make sure that this behavior is backward compatible with the current release
  • Update the global config to ensure only one at_exit handler ever exists for each global thread pool
    • Our default global thread pools should be created without their own at_exit handlers
    • [ ] Let the at_exit handlers created within the global config handle the global thread pools
  • Update the behavior of the global configuration auto_terminate feature
    • Remove unnecessary respond_to? checks
    • [ ] Ensure proper handing of at_exit handler creation for all thread pools automatically created within the global config
    • [ ] Ensure proper handling of at_exit when the user explicitly sets a global thread pool
  • Create a Configuration.shutdown_global_thread_pools (or similarly named) method
    • Give advanced users who disable the at_exit handlers the ability to explicitly shutdown the global thread pools
    • Potentially create shutdown/kill/wait methods that mirror the methods on the thread pools themselves
  • Create a global "autotest friendly" configuration option
    • [ ] It should include a require 'concurrent/autotest' feature for simple setup
    • It should prevent any at_exit handlers from being created for all thread pools, regardless of how they are created
    • Yet it should also ensure that the JVM should not hang because the thread pools are not shutdown
    • And it should be as unobtrusive as possible (I hope to avoid coupling thread pools to some global registry if possible)
  • Document, document, document

From #149

  • keep them lazy initialized
  • rename: task -> short-non-blocking, operation -> long-blocking
  • [ ] add more global executors like an immediate executor
  • remove configuration object, move global pools to Concurrent space
  • possibly improve parsing of :executor option
    • [ ] to also handle agent two pools
    • to understand :immediate too
    • etc.
  • what to do with logging?

@jdantonio jdantonio added in progress enhancement Adding features, adding tests, improving documentation. labels Feb 27, 2015
@jdantonio jdantonio added this to the 0.8.1Release milestone Feb 27, 2015
@jdantonio jdantonio self-assigned this Feb 27, 2015
@jdantonio jdantonio modified the milestones: 0.9.0 Release, 0.8.1Release Feb 28, 2015
@jdantonio jdantonio mentioned this pull request Mar 1, 2015
34 tasks
@jdantonio
Copy link
Member Author

ALL: With the exception of documentation, I believe this PR to be complete. I plan to merge it Tuesday night after work. All feedback is welcome.

@pitr-ch
Copy link
Member

pitr-ch commented Mar 2, 2015

@jdantonio I was on PTO for a week so I am still catching up on the conversations. Could you give me little more time to go over it?

@jdantonio
Copy link
Member Author

@pitr-ch Absolutely!

* Renamed #set_shutdown_hook to #enable_at_exit_handler!
* Moved #enable_at_exit_handler! to the Executor module
* Adde call to #enable_at_exit_handler! to Ruby thread pools
* Created new global pools within the Concurrent namespace
* Renamed global task pool to 'fast executor'
* Renamed global operations pool to 'io executor'
* Adjusted global thread pool attributes
* Deprecated old method calls
  - Global thread pool getters get new thread pools
  - Thread pool factories create pools with new attributes
  - Deprecation warnings for all old functions
* Auto terminate feature has more descriptive name
* Updated test helper for resetting global thread pools
* Updated tests that changed the global thread pools
* Commented out a problematic actor test
jdantonio added a commit that referenced this pull request Mar 7, 2015
Global configuration and thread pool updates.
@jdantonio jdantonio merged commit 235f061 into master Mar 7, 2015
@jdantonio jdantonio deleted the fix/thread-pool-shutdown branch March 7, 2015 17:19
@@ -26,4 +26,4 @@ matrix:
- rvm: jruby-head
- rvm: 1.9.3

script: "rake compile && bundle exec rspec --color --backtrace --tag ~unfinished --seed 1 --format documentation ./spec"
script: "rake compile && bundle exec rspec --color --backtrace --tag ~unfinished --seed 1 ./spec"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the --format documentation was there to see the last test where it was hanging. Is there another way?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if there's another way, but --format documentation has been in .travis.yml since June of last year. I only removed it a couple of weeks ago--then immediately added it back when we had a hanging test. I think we should keep it for now. Once we stabilize our tests we can remove it, but the full output is pretty valuable right now.

@pitr-ch
Copy link
Member

pitr-ch commented Mar 12, 2015

Thanks for all the work on executors! And sorry that it took me so long to get to it :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adding features, adding tests, improving documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants