From b70f62a64a455ab0ef6f1df02ef14ae52388950b Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Wed, 8 Jan 2025 15:04:59 +0100 Subject: [PATCH 1/3] Use Crystal::PointerLinkedList instead of Deque in Mutex 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). --- src/fiber/waiting.cr | 13 +++++++++++++ src/mutex.cr | 20 ++++++++++++-------- src/wait_group.cr | 16 +++------------- 3 files changed, 28 insertions(+), 21 deletions(-) create mode 100644 src/fiber/waiting.cr diff --git a/src/fiber/waiting.cr b/src/fiber/waiting.cr new file mode 100644 index 000000000000..8c4d0138a9f1 --- /dev/null +++ b/src/fiber/waiting.cr @@ -0,0 +1,13 @@ +class Fiber + # :nodoc: + struct Waiting + include Crystal::PointerLinkedList::Node + + def initialize(@fiber : Fiber) + end + + def enqueue : Nil + @fiber.enqueue + end + end +end diff --git a/src/mutex.cr b/src/mutex.cr index 780eac468201..ad9bc1ac2ca5 100644 --- a/src/mutex.cr +++ b/src/mutex.cr @@ -1,3 +1,4 @@ +require "fiber/waiting" require "crystal/spin_lock" # A fiber-safe mutex. @@ -22,9 +23,9 @@ class Mutex @state = Atomic(Int32).new(UNLOCKED) @mutex_fiber : Fiber? @lock_count = 0 - @queue = Deque(Fiber).new + @queue = Crystal::PointerLinkedList(Fiber::Waiting).new @queue_count = Atomic(Int32).new(0) - @lock = Crystal::SpinLock.new + @spin = Crystal::SpinLock.new enum Protection Checked @@ -59,7 +60,9 @@ class Mutex loop do break if try_lock - @lock.sync do + waiting = Fiber::Waiting.new(Fiber.current) + + @spin.sync do @queue_count.add(1) if @state.get(:relaxed) == UNLOCKED @@ -71,7 +74,7 @@ class Mutex end end - @queue.push Fiber.current + @queue.push pointerof(waiting) end Fiber.suspend @@ -116,17 +119,18 @@ class Mutex return end - fiber = nil - @lock.sync do + waiting = nil + @spin.sync do if @queue_count.get == 0 return end - if fiber = @queue.shift? + if waiting = @queue.shift? @queue_count.add(-1) end end - fiber.enqueue if fiber + + waiting.try(&.value.enqueue) end def synchronize(&) diff --git a/src/wait_group.cr b/src/wait_group.cr index cf1ca8900e8f..efa19f15e1b4 100644 --- a/src/wait_group.cr +++ b/src/wait_group.cr @@ -1,4 +1,5 @@ require "fiber" +require "fiber/waiting" require "crystal/spin_lock" require "crystal/pointer_linked_list" @@ -31,17 +32,6 @@ require "crystal/pointer_linked_list" # wg.wait # ``` class WaitGroup - private struct Waiting - include Crystal::PointerLinkedList::Node - - def initialize(@fiber : Fiber) - end - - def enqueue : Nil - @fiber.enqueue - end - end - # Yields a `WaitGroup` instance and waits at the end of the block for all of # the work enqueued inside it to complete. # @@ -59,7 +49,7 @@ class WaitGroup end def initialize(n : Int32 = 0) - @waiting = Crystal::PointerLinkedList(Waiting).new + @waiting = Crystal::PointerLinkedList(Fiber::Waiting).new @lock = Crystal::SpinLock.new @counter = Atomic(Int32).new(n) end @@ -128,7 +118,7 @@ class WaitGroup def wait : Nil return if done? - waiting = Waiting.new(Fiber.current) + waiting = Fiber::Waiting.new(Fiber.current) @lock.sync do # must check again to avoid a race condition where #done may have From 098cb66f7fced244de48040196cd5187bd3ba29e Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Tue, 14 Jan 2025 09:13:55 +0100 Subject: [PATCH 2/3] fix: revert `@lock` to `@spin` rename --- src/mutex.cr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mutex.cr b/src/mutex.cr index ad9bc1ac2ca5..956485a4cb3c 100644 --- a/src/mutex.cr +++ b/src/mutex.cr @@ -25,7 +25,7 @@ class Mutex @lock_count = 0 @queue = Crystal::PointerLinkedList(Fiber::Waiting).new @queue_count = Atomic(Int32).new(0) - @spin = Crystal::SpinLock.new + @lock = Crystal::SpinLock.new enum Protection Checked @@ -62,7 +62,7 @@ class Mutex waiting = Fiber::Waiting.new(Fiber.current) - @spin.sync do + @lock.sync do @queue_count.add(1) if @state.get(:relaxed) == UNLOCKED @@ -120,7 +120,7 @@ class Mutex end waiting = nil - @spin.sync do + @lock.sync do if @queue_count.get == 0 return end From 895345b963df341077585a3323a9866c7de95c57 Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Tue, 14 Jan 2025 09:16:47 +0100 Subject: [PATCH 3/3] Rename Fiber::Waiting as Fiber::PointerLinkedListNode --- src/fiber/{waiting.cr => pointer_linked_list_node.cr} | 4 +++- src/mutex.cr | 6 +++--- src/wait_group.cr | 7 +++---- 3 files changed, 9 insertions(+), 8 deletions(-) rename src/fiber/{waiting.cr => pointer_linked_list_node.cr} (71%) diff --git a/src/fiber/waiting.cr b/src/fiber/pointer_linked_list_node.cr similarity index 71% rename from src/fiber/waiting.cr rename to src/fiber/pointer_linked_list_node.cr index 8c4d0138a9f1..45994fe5c489 100644 --- a/src/fiber/waiting.cr +++ b/src/fiber/pointer_linked_list_node.cr @@ -1,6 +1,8 @@ +require "crystal/pointer_linked_list" + class Fiber # :nodoc: - struct Waiting + struct PointerLinkedListNode include Crystal::PointerLinkedList::Node def initialize(@fiber : Fiber) diff --git a/src/mutex.cr b/src/mutex.cr index 956485a4cb3c..14d1aedf7923 100644 --- a/src/mutex.cr +++ b/src/mutex.cr @@ -1,4 +1,4 @@ -require "fiber/waiting" +require "fiber/pointer_linked_list_node" require "crystal/spin_lock" # A fiber-safe mutex. @@ -23,7 +23,7 @@ class Mutex @state = Atomic(Int32).new(UNLOCKED) @mutex_fiber : Fiber? @lock_count = 0 - @queue = Crystal::PointerLinkedList(Fiber::Waiting).new + @queue = Crystal::PointerLinkedList(Fiber::PointerLinkedListNode).new @queue_count = Atomic(Int32).new(0) @lock = Crystal::SpinLock.new @@ -60,7 +60,7 @@ class Mutex loop do break if try_lock - waiting = Fiber::Waiting.new(Fiber.current) + waiting = Fiber::PointerLinkedListNode.new(Fiber.current) @lock.sync do @queue_count.add(1) diff --git a/src/wait_group.cr b/src/wait_group.cr index efa19f15e1b4..003921bd9f46 100644 --- a/src/wait_group.cr +++ b/src/wait_group.cr @@ -1,7 +1,6 @@ require "fiber" -require "fiber/waiting" +require "fiber/pointer_linked_list_node" require "crystal/spin_lock" -require "crystal/pointer_linked_list" # Suspend execution until a collection of fibers are finished. # @@ -49,7 +48,7 @@ class WaitGroup end def initialize(n : Int32 = 0) - @waiting = Crystal::PointerLinkedList(Fiber::Waiting).new + @waiting = Crystal::PointerLinkedList(Fiber::PointerLinkedListNode).new @lock = Crystal::SpinLock.new @counter = Atomic(Int32).new(n) end @@ -118,7 +117,7 @@ class WaitGroup def wait : Nil return if done? - waiting = Fiber::Waiting.new(Fiber.current) + waiting = Fiber::PointerLinkedListNode.new(Fiber.current) @lock.sync do # must check again to avoid a race condition where #done may have