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.

Sign up for GitHub

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

Jump to bottom

Remove some synchronization that seems no longer necessary #190

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Remove some synchronization that seems no longer necessary #190

wants to merge 1 commit into from

Conversation

jcarreira
Copy link
Owner

@jcarreira jcarreira commented Sep 13, 2017 •
edited
Loading

Because we now use a lock free queue

@jcarreira
Copy link
Owner Author

@devloop0 Can you review this PR?

@jcarreira jcarreira requested a review from devloop0 September 13, 2017 21:58
@jcarreira jcarreira self-assigned this Sep 13, 2017
devloop0
devloop0 approved these changes Sep 17, 2017
View reviewed changes
Copy link
Collaborator

@devloop0 devloop0 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Why do we no longer need synchronization for the send queue? Other than that, looks good to me.
Edit: Perhaps I should rephrase my question; why can we now use a lock-free queue? Or was this always possible and we just now took advantage of it?

@jcarreira
Copy link
Owner Author

It was always possible. Mostly using a lock-free queue makes the code slightly simpler.

@devloop0
Copy link
Collaborator

Sounds good.
This looks pr looks ready to merge but some of the changes here seem to conflict with #181 so we should wait to see which pr we should merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@devloop0 devloop0 devloop0 approved these changes

Assignees

@jcarreira jcarreira

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jcarreira @devloop0