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

Implemented synchronized PriorityQueue. #398

Closed
wants to merge 2 commits into from
Closed

Conversation

jdantonio
Copy link
Member

The original priority queue implementation was added as an internal implementation detail of TimerSet. It was not thread-safe because TimerSet handled the synchronization. I recently began experimenting with a Rails ActiveJob adapter built on concurrent-ruby. I began thinking about adding prioritization to the adapter and realized that I might be able to use our priority queue, so I took a stab at creating a thread-safe wrapper around our current implementation.

This is a low-priority item since it doesn't look like my Rails PR is going to be merged. But this update may still be a nice addition to our gem.

This PR does a few things:

  • Renames the original priority queue implementation to better reflect that it is not thread-safe.
  • Removes the unnecessary method aliases from the non-thread-safe priority queue implementations.
  • Implements a synchronized Concurrent::PriorityQueue which leverages Concurrent::Synchronization::Object.
  • Implements the original method aliases on Concurrent::PriorityQueue.

@jdantonio jdantonio added in progress enhancement Adding features, adding tests, improving documentation. labels Aug 16, 2015
@jdantonio jdantonio added this to the 2.0.0 Release milestone Aug 16, 2015
@jdantonio jdantonio self-assigned this Aug 16, 2015
@jdantonio
Copy link
Member Author

I'm going to close this PR until we have an actual use case. I'm going to cherry-pick commit 677ab3e onto master. The classes need renamed to better reflect their not-thread-safe status.

@jdantonio
Copy link
Member Author

Superseded by PR #399

@jdantonio jdantonio closed this Aug 17, 2015
@jdantonio jdantonio deleted the priority-queue branch August 17, 2015 12:39
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.

1 participant