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

Use Crystal::PointerLinkedList instead of Deque in Mutex #15330

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

Conversation

ysbaddaden
Copy link
Contributor

Follows WaitGroup that uses a linked list instead of a deque to store the waiting fibers.

The flat array doesn't have much impact on performance: we only reach the head or the tail once to dequeue/dequeue one fiber at a time. This however spares a number of GC allocations since the Deque has to be allocated plus its buffer that will have to be reallocated sometimes (and will only ever grow, never shrink).

Extracts the undocumented Fiber::Waiting struct from WaitGroup that acts as the node in the linked list.

Renames @lock to @spin since the former was a bit confusing, for example with @lock_count that counts the number of reentrancy.

The flat array also doesn't have much impact since it's never traversed:
we only reach the head or the tail once (to dequeue or dequeue one).

This spares a number of GC allocations since the Deque needs to be
allocated and also a buffer that will have to be reallocated sometimes
(and will only ever grow).
@BlobCodes
Copy link
Contributor

This PR seems to remove the need for @queue_count since PointerLinkedList#empty? should be just as fast

@ysbaddaden
Copy link
Contributor Author

Well Deque#empty? would have been just the same.

What we really need is to know whether there's a waiter and it must be atomic, so unlock can safely spare acquiring the spinlock when there are no waiters.

Now, we don't need to count: it could be a simple flag on the spinlock value, set when #lock acquires the spinlock and unset when #unlock releases it and there are no waiters (this is what the nsync library does).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Review
Development

Successfully merging this pull request may close these issues.

2 participants