Hi,
I have the following proposes for Nim channels. The actual implementation is still ongoing (mostly comments are missing from a proper PR). Please give me feedback what you would like to see.
Thank you, Peter
The functions isClosed() and isAlive() are to check whether the channel has been closed. The functions isAlive() and isDead() are to check whether the channel has been closed and empty.
P1: recv() should return a tuple[dataAvailable: bool, message TMsg] instead of just TMsg. The reason is simple: currently it is impossible to say whether a channel[bool] has been closed or someone sent false. With this change tryRecv() and recv() would have the same return type. This change breaks the current API. Example:
var ch: Channel[string]
ch.open()
...
let (avail, msg) = ch.recv()
if avail:
echo "message received: ", msg
Update: a channel can be empty and closed from a different thread, and in that case there is no point waiting for a new message. Currently the default value is returned (for example false for bool). The problem with this approach is that there is no way to say whether the default value was sent, or the channel is closed and empty (dead. if we follow the notation of P0). This is the reason why an extra flag is needed, so we could distinguish these two cases.
P2: There should be overloaded versions of recv(...) and tryRecv(...) which takes an message: var TMsg and returns a bool indicating whether the reception was successful. I understand that in this case the user needs to provide the type when creating the variable. The main usecase:
var ch: Channel[string]
ch.open()
...
var msg: string
while ch.recv(msg):
echo "we just received: ", msg
echo "If we get to this point then the channel 'ch' was already closed and we received everything (if we were the only thread receiving)"
Update: The recv(msg) call can be unsuccessful if the channel was closed and empty (i.e. dead). The tryRecv(msg) call can be unsuccessful if there is no new message waiting (for example it is simple empty or closed as well). I really like the above format while ch.recv(msg):, I think that this is intuitive and clear what it does.
P3: The tryRecv(...) functions should receive a message if there is at least one message even if a short time is needed. Currently these functions returns without reading the message if the channel is occupied (a message is being read or sent). The example usecase:
var ch: Channel[string]
ch.open()
...
var msg: string
while ch.tryRecv(msg):
echo "we just received: ", msg
echo "If we get to this point then there was one moment when the channel 'ch' was empty"
P4: At each time a message is sent to a channel or a channel is closed, an extra mutex is locked, and one counter increased. This is done so that the following code could listen on multiple channel from one thread:
var ch1: Channel[string]
var ch2: Channel[string]
ch1.open()
ch2.open()
...
channelWhile ch1.isAlive() or ch2.isAlive():
echo "this message is shown only if there was channel activity, otherwise no CPU is used"
var m: string
while ch1.tryRecv(m):
echo "ch1 says: ", m
while ch2.tryRecv(m):
echo "ch2 says: ", m
echo "We receive this point if both channels are closed and empty."
Update: Currently there is no way to listen to several channels from a thread. Each channel has a mutex lock and condition, and if you listen to one channel's condition, then you cannot do anything else because it blocks your thread. Currently each time a message is sent we lock this mutex and send a signal using the condition to notify other threads which might wait for a message. The proposal is that each time a message is sent, then additional to the current steps we also lock a global common lock (only for a very short time), increase a global counter and send a broadcast signal. You can think of this global counter as the number of messages sent plus number of channels closed since the start of the program. Given this setup, we can easily listen on two channels, because instead of listening on one of them, we can wait using the global lock and condition until some channel event happens (either a message sent or a channel is closed). The channelWhile statement above works like a while cycle: you can think of it as a loop which runs until the given condition is true. However, each time the loop is stopped until there is a channel event.
P5: There should be a trySend(message: TMsg): bool function, which returns false (instead of throwing an exception) if the channel is closed. This can be used in the following pattern:
var ch: channel[string]
ch.open()
...
while true:
var m = getSampleFromInfiniteFountain()
if not ch.trySend(m):
break
echo "If you reach this line, the channel is closed."
P6: There should be a debugger/profiler mode. It shouldn't be effecting the release compilation. In debug mode if it is not enabled, then we have an extra if check overhead. If in debug mode it is enabled, then you could see the channel activities in your browser: http://pottom.mooo.com/channels.gif
Update: The webserver and all the relevant code is in a separate file, not in channels.nim. However we need to extend channels.nim with a few lines to support this functionality. Obviously there is a cost for this. These extra lines in channels.nim is disabled (by a when statement) in release compilation, so there is no cost at all if you compile your code with -d:release (and obviously you cannot use this in release). It is only effecting debug builds.
P0: Ok, sounds reasonable.
P1: Isn't this done with tryRecv?.
P2: Under what conditions would send not be successful?
P4: Could you explain a bit?
P6: This is far beyond the normal scope of a standard library. It's a neat idea though.
Overall, you raise done good points about additions to the channels module. Don't forget that you can copy the current channels module, make the changes/additions, then publish it on Nimble and petition for its inclusion into the standard library.
@Varriount: Thank you for your feedback. Instead of answering here, I extended the original post above with Update: sections.
Peter
P4: introduces a single global point of contention and so it is really a bad implementation. There are better ways to implement the same feature. Please check (at least) Go's channel implementation.
P1: Currently if you use recv the sentinel is part of the message and that can continue to work if you make recv raise an exception when the channel has been closed already and no message is pending.
I started to read Go's channel implementation. They are using a list of waiters for each channel, who need to be notified when there is a write/read request. So this is how they avoid to have one central global lock. They call it sudog, and there is even some smart cache logic to have a central list of these unused locks. When I reached a function which was replaced by the compiler according to the comment, then I stopped digging. The Go code is not that hard to read, but it uses many special things, like shared memory between threads, and I know so few things about Nim's memory management. Maybe I should start learning that first.
If we make recv(): TMsg raise an exception if the channel is closed then it will behave the same as send(msg: TMsg) on closed channels. That's good. However, I saw many code examples here in this forum when they checked msg==nil to find out whether the channel was closed, so some code will break.