From e724ad622b83a122fba7577e85288a57066a3909 Mon Sep 17 00:00:00 2001 From: Philipp Doerner Date: Fri, 15 Dec 2023 13:15:41 +0100 Subject: [PATCH 1/5] Correct doc mistakes The doc comments implied that tryRecv and trySend do not block. That is incorrect. They do not block *waiting for data*. They *do* block trying to acquire the lock to the channel. Also some adjustments for better uniformity in the docs: - Messages renamed to items - Some doc comments were made more similar to one another in text style --- threading/channels.nim | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/threading/channels.nim b/threading/channels.nim index 7904ad7..04e3790 100644 --- a/threading/channels.nim +++ b/threading/channels.nim @@ -27,7 +27,7 @@ ## the underlying resources and synchronization. It has to be initialized using ## the `newChan` proc. Sending and receiving operations are provided by the ## blocking `send` and `recv` procs, and non-blocking `trySend` and `tryRecv` -## procs. Send operations add messages to the channel, receiving operations +## procs. Send operations add items to the channel, receiving operations ## remove them. ## ## See also: @@ -270,11 +270,13 @@ proc `=copy`*[T](dest: var Chan[T], src: Chan[T]) = dest.d = src.d proc trySend*[T](c: Chan[T], src: sink Isolated[T]): bool {.inline.} = - ## Tries to send a message to a channel. + ## Tries to send an item `src` to a channel `c`. ## - ## The memory `src` is moved, not copied. Doesn't block. + ## The memory of `src` is moved, not copied. + ## Returns immediately once the lock to the channel was acquired and an + ## attempt to send it was made. ## - ## Returns `false` if the message was not sent because the number of pending + ## Returns `false` if the item was not sent because the number of pending ## items in the channel exceeded its capacity. var data = src.extract result = channelSend(c.d, data.unsafeAddr, sizeof(T), false) @@ -286,19 +288,23 @@ template trySend*[T](c: Chan[T], src: T): bool = trySend(c, isolate(src)) proc tryRecv*[T](c: Chan[T], dst: var T): bool {.inline.} = - ## Tries to receive a message from the channel `c` and fill `dst` with its value. - ## This returns immediately even if no message is found. Doesn't block. + ## Tries to receive an item from the channel `c` and fill `dst` with its value. ## - ## This can fail for all sort of reasons, including a lack of messages in the channel - ## to receive or contention. + ## Returns immediately once the lock on the Channel is acquired and an attempt + ## at fetching an item from it was made. + ## The proc will not wait for items to appear. + ## + ## This will fail if the channel is empty. ## ## If it fails it returns `false`. Otherwise it returns `true`. channelReceive(c.d, dst.addr, sizeof(T), false) proc send*[T](c: Chan[T], src: sink Isolated[T]) {.inline.} = - ## Sends item to the channel. + ## Sends item `src` to the channel `c`. ## This blocks the sending thread until the item was successfully sent. ## + ## The memory of `src` is moved, not copied. + ## ## If the channel is already full with items this will block the thread until ## items from the channel are removed. var data = src.extract @@ -312,8 +318,8 @@ template send*[T](c: Chan[T]; src: T) = send(c, isolate(src)) proc recv*[T](c: Chan[T], dst: var T) {.inline.} = - ## Receives an item from the channel. - ## Fills `dist` with the item. + ## Receives an item from the channel `c` and fill `dst` with its value. + ## ## This blocks the receiving thread until an item was successfully received. ## ## If the channel does not contain any items this will block the thread until From 7bf6fad7cd36e07f6baff7cabddccf75fc4acbf1 Mon Sep 17 00:00:00 2001 From: Philipp Doerner Date: Fri, 15 Dec 2023 17:24:07 +0100 Subject: [PATCH 2/5] #52 Rename item to message --- threading/channels.nim | 44 +++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/threading/channels.nim b/threading/channels.nim index 04e3790..ac5e000 100644 --- a/threading/channels.nim +++ b/threading/channels.nim @@ -27,7 +27,7 @@ ## the underlying resources and synchronization. It has to be initialized using ## the `newChan` proc. Sending and receiving operations are provided by the ## blocking `send` and `recv` procs, and non-blocking `trySend` and `tryRecv` -## procs. Send operations add items to the channel, receiving operations +## procs. Send operations add messages to the channel, receiving operations ## remove them. ## ## See also: @@ -270,14 +270,14 @@ proc `=copy`*[T](dest: var Chan[T], src: Chan[T]) = dest.d = src.d proc trySend*[T](c: Chan[T], src: sink Isolated[T]): bool {.inline.} = - ## Tries to send an item `src` to a channel `c`. + ## Tries to send a message `src` to the channel `c`. ## ## The memory of `src` is moved, not copied. ## Returns immediately once the lock to the channel was acquired and an ## attempt to send it was made. ## - ## Returns `false` if the item was not sent because the number of pending - ## items in the channel exceeded its capacity. + ## Returns `false` if the message was not sent because the number of pending + ## messages in the channel exceeded its capacity. var data = src.extract result = channelSend(c.d, data.unsafeAddr, sizeof(T), false) if result: @@ -288,11 +288,11 @@ template trySend*[T](c: Chan[T], src: T): bool = trySend(c, isolate(src)) proc tryRecv*[T](c: Chan[T], dst: var T): bool {.inline.} = - ## Tries to receive an item from the channel `c` and fill `dst` with its value. + ## Tries to receive an message from the channel `c` and fill `dst` with its value. ## ## Returns immediately once the lock on the Channel is acquired and an attempt - ## at fetching an item from it was made. - ## The proc will not wait for items to appear. + ## at fetching a message from it was made. + ## The proc will not wait for messages to appear. ## ## This will fail if the channel is empty. ## @@ -300,13 +300,13 @@ proc tryRecv*[T](c: Chan[T], dst: var T): bool {.inline.} = channelReceive(c.d, dst.addr, sizeof(T), false) proc send*[T](c: Chan[T], src: sink Isolated[T]) {.inline.} = - ## Sends item `src` to the channel `c`. - ## This blocks the sending thread until the item was successfully sent. + ## Sends message `src` to the channel `c`. + ## This blocks the sending thread until the message was successfully sent. ## ## The memory of `src` is moved, not copied. ## - ## If the channel is already full with items this will block the thread until - ## items from the channel are removed. + ## If the channel is already full with messages this will block the thread until + ## messages from the channel are removed. var data = src.extract when defined(gcOrc) and defined(nimSafeOrcSend): GC_runOrc() @@ -318,35 +318,35 @@ template send*[T](c: Chan[T]; src: T) = send(c, isolate(src)) proc recv*[T](c: Chan[T], dst: var T) {.inline.} = - ## Receives an item from the channel `c` and fill `dst` with its value. + ## Receives a message from the channel `c` and fill `dst` with its value. ## - ## This blocks the receiving thread until an item was successfully received. + ## This blocks the receiving thread until a message was successfully received. ## - ## If the channel does not contain any items this will block the thread until - ## items get sent to the channel. + ## If the channel does not contain any messages this will block the thread until + ## a message get sent to the channel. discard channelReceive(c.d, dst.addr, sizeof(T), true) proc recv*[T](c: Chan[T]): T {.inline.} = - ## Receives an item from the channel. - ## A version of `recv`_ that returns the item. + ## Receives a message from the channel. + ## A version of `recv`_ that returns the message. discard channelReceive(c.d, result.addr, sizeof(result), true) proc recvIso*[T](c: Chan[T]): Isolated[T] {.inline.} = - ## Receives an item from the channel. - ## A version of `recv`_ that returns the item and isolates it. + ## Receives a message from the channel. + ## A version of `recv`_ that returns the message and isolates it. var dst: T discard channelReceive(c.d, dst.addr, sizeof(T), true) result = isolate(dst) func peek*[T](c: Chan[T]): int {.inline.} = - ## Returns an estimation of the current number of items held by the channel. + ## Returns an estimation of the current number of messages held by the channel. numItems(c.d) proc newChan*[T](elements: Positive = 30): Chan[T] = ## An initialization procedure, necessary for acquiring resources and ## initializing internal state of the channel. ## - ## `elements` is the capacity of the channel and thus how many items it can hold - ## before it refuses to accept any further items. + ## `elements` is the capacity of the channel and thus how many messages it can hold + ## before it refuses to accept any further messages. assert elements >= 1, "Elements must be positive!" result = Chan[T](d: allocChannel(sizeof(T), elements)) From ea741e7ddee36d0b1388a22e7a7353dfd97234c8 Mon Sep 17 00:00:00 2001 From: Philipp Doerner Date: Fri, 15 Dec 2023 17:42:47 +0100 Subject: [PATCH 3/5] Reformulate tryRecv/trySend Includes @ZoomRc's suggestions from PR #54 but splits them up into smaller sentences to make them more digestable. --- threading/channels.nim | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/threading/channels.nim b/threading/channels.nim index ac5e000..e343e20 100644 --- a/threading/channels.nim +++ b/threading/channels.nim @@ -273,8 +273,12 @@ proc trySend*[T](c: Chan[T], src: sink Isolated[T]): bool {.inline.} = ## Tries to send a message `src` to the channel `c`. ## ## The memory of `src` is moved, not copied. - ## Returns immediately once the lock to the channel was acquired and an - ## attempt to send it was made. + ## Doesn't block waiting for space in the channel to become available. + ## Instead returns after an attempt to send a message was made. + ## + ## .. warning:: Blocking is still possible if another thread uses the blocking + ## version of `send`/`recv` and waits for the data/space to appear in the + ## channel, thus holding the internal lock to channel's buffer. ## ## Returns `false` if the message was not sent because the number of pending ## messages in the channel exceeded its capacity. @@ -290,18 +294,19 @@ template trySend*[T](c: Chan[T], src: T): bool = proc tryRecv*[T](c: Chan[T], dst: var T): bool {.inline.} = ## Tries to receive an message from the channel `c` and fill `dst` with its value. ## - ## Returns immediately once the lock on the Channel is acquired and an attempt - ## at fetching a message from it was made. - ## The proc will not wait for messages to appear. + ## Doesn't block waiting for messages in the channel to become available. + ## Instead returns after an attempt to receive a message was made. ## - ## This will fail if the channel is empty. - ## - ## If it fails it returns `false`. Otherwise it returns `true`. + ## .. warning:: Blocking is still possible if another thread uses the blocking + ## version of `send`/`recv` and waits for the data/space to appear in the + ## channel, thus holding the internal lock to channel's buffer. + ## + ## Returns `false` and does not change `dist` if no message was received. channelReceive(c.d, dst.addr, sizeof(T), false) proc send*[T](c: Chan[T], src: sink Isolated[T]) {.inline.} = ## Sends message `src` to the channel `c`. - ## This blocks the sending thread until the message was successfully sent. + ## This blocks the sending thread until `src` was successfully sent. ## ## The memory of `src` is moved, not copied. ## From cf2a7f62184e3895577d77b62c03dd2269da674f Mon Sep 17 00:00:00 2001 From: Philipp Doerner Date: Fri, 15 Dec 2023 17:57:28 +0100 Subject: [PATCH 4/5] Add some doc links --- threading/channels.nim | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/threading/channels.nim b/threading/channels.nim index e343e20..a79df93 100644 --- a/threading/channels.nim +++ b/threading/channels.nim @@ -277,8 +277,9 @@ proc trySend*[T](c: Chan[T], src: sink Isolated[T]): bool {.inline.} = ## Instead returns after an attempt to send a message was made. ## ## .. warning:: Blocking is still possible if another thread uses the blocking - ## version of `send`/`recv` and waits for the data/space to appear in the - ## channel, thus holding the internal lock to channel's buffer. + ## version of the `send proc`_ / `recv proc`_ and waits for the + ## data/space to appear in the channel, thus holding the internal lock to + ## channel's buffer. ## ## Returns `false` if the message was not sent because the number of pending ## messages in the channel exceeded its capacity. @@ -298,8 +299,8 @@ proc tryRecv*[T](c: Chan[T], dst: var T): bool {.inline.} = ## Instead returns after an attempt to receive a message was made. ## ## .. warning:: Blocking is still possible if another thread uses the blocking - ## version of `send`/`recv` and waits for the data/space to appear in the - ## channel, thus holding the internal lock to channel's buffer. + ## version of the `send proc`_ / `recv proc`_ and waits for the data/space to + ## appear in the channel, thus holding the internal lock to channel's buffer. ## ## Returns `false` and does not change `dist` if no message was received. channelReceive(c.d, dst.addr, sizeof(T), false) From ae3903780befaa5243af7267b06585d477dde9dc Mon Sep 17 00:00:00 2001 From: Philipp Doerner Date: Fri, 15 Dec 2023 18:00:32 +0100 Subject: [PATCH 5/5] #52 Made usage of articles consistent Used specific article "the" for sending messages as you know the mssage being sent. Used unspecific article "a" for receiving a message as you do not know what you receive. --- threading/channels.nim | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/threading/channels.nim b/threading/channels.nim index a79df93..cbfe50a 100644 --- a/threading/channels.nim +++ b/threading/channels.nim @@ -270,7 +270,7 @@ proc `=copy`*[T](dest: var Chan[T], src: Chan[T]) = dest.d = src.d proc trySend*[T](c: Chan[T], src: sink Isolated[T]): bool {.inline.} = - ## Tries to send a message `src` to the channel `c`. + ## Tries to send the message `src` to the channel `c`. ## ## The memory of `src` is moved, not copied. ## Doesn't block waiting for space in the channel to become available. @@ -293,7 +293,7 @@ template trySend*[T](c: Chan[T], src: T): bool = trySend(c, isolate(src)) proc tryRecv*[T](c: Chan[T], dst: var T): bool {.inline.} = - ## Tries to receive an message from the channel `c` and fill `dst` with its value. + ## Tries to receive a message from the channel `c` and fill `dst` with its value. ## ## Doesn't block waiting for messages in the channel to become available. ## Instead returns after an attempt to receive a message was made. @@ -306,7 +306,7 @@ proc tryRecv*[T](c: Chan[T], dst: var T): bool {.inline.} = channelReceive(c.d, dst.addr, sizeof(T), false) proc send*[T](c: Chan[T], src: sink Isolated[T]) {.inline.} = - ## Sends message `src` to the channel `c`. + ## Sends the message `src` to the channel `c`. ## This blocks the sending thread until `src` was successfully sent. ## ## The memory of `src` is moved, not copied.