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

name collision for "take" #5511

Closed
StefanKarpinski opened this issue Jan 24, 2014 · 13 comments
Closed

name collision for "take" #5511

StefanKarpinski opened this issue Jan 24, 2014 · 13 comments
Labels
breaking This change will break code
Milestone

Comments

@StefanKarpinski
Copy link
Member

This is a rather unfortunate name collision:

@JeffBezanson
Copy link
Member

We should probably rename the RemoteRef take to take! and put to put!. That's probably enough to fix this too since the take in Iterators is inherently a pure function.

@StefanKarpinski
Copy link
Member Author

That seems fine to me, although thus far we've only used the ! convention for mutation of arrays, not other things with state (e.g. I/O objects).

@JeffBezanson
Copy link
Member

A RemoteRef acts like a synchronized queue of length 1, so it is just as much a data structure as an I/O thing.

@StefanKarpinski
Copy link
Member Author

Fair enough. Just thought I'd point out that it's stretching the convention further than we have so far.

@JeffBezanson
Copy link
Member

Or, on the theory that this thing is a queue and might someday allow more than one element, we should use some combination of pop!, push!, shift!, unshift!, enqueue!, and dequeue!, though I'm not really sure which.

@StefanKarpinski
Copy link
Member Author

Yikes. The only real reason to have a collection-like API for such a thing, even if it is actually a queue, is if it might make sense to pass RemoteRefQueue into some generic code that works on a Queue or plain old Array. Does that actually make any sense? I'm not sure, but I feel like that's the fundamental question that needs to be asked.

@JeffBezanson
Copy link
Member

As soon as the put and take functions were added, RemoteRef became genuinely equivalent to a "channel" or "synchronized queue". As such its name is no longer very accurate, except in that it can still hold only one value. I think at this point I'd be fine with renaming it to Channel or something else. If you have a function that consumes things from a queue, passing it one of these would be totally legitimate as far as I can tell.

@StefanKarpinski
Copy link
Member Author

Ok, in that case, maybe we should go ahead with the rename and make sure that you can exchange a normal Queue object with a Channel object. Using push! and shift! for channels does seem a bit odd, but sure.

@JeffBezanson
Copy link
Member

I don't insist on rocking the boat so much here; I was just really excited to remove two exports. I see the point that channels don't quite fit either queues or I/O devices. Do you have any ideas for names?

@StefanKarpinski
Copy link
Member Author

I think put! and take! are ok. Go calls these operations "put" and "take" [1] even though the they use special <- syntax for them. Rich Hickey also uses the terms "put" and "take" [2], although core.async calls these operations >! and <!. So I think calling them put! and take! is pretty reasonable. One option is to call the operations that and then make normal collections support the put! and take! operations where appropriate. There are two directions of interchangeability to consider:

  1. Using a Channel in generic code for a normal Queue object.
  2. Using a normal Queue object in generic code for a Channel.

For the former, you want Channel to support Queue operations, whereas for the latter, you want Queue to support Channel operations. It might also be reasonable to use the put! and take! operations for queues and stacks one can imagine swapping one for the other to change behavior from LIFO to FIFO or vice versa – or using some kind of priority queue for even different behavior.

[1] http://golangtutorials.blogspot.com/2011/06/channels-in-go.html
[2] http://clojure.com/blog/2013/06/28/clojure-core-async-channels.html

@JeffBezanson
Copy link
Member

I'm fine with put! and take!; those are some pretty solid precedents. But if we are actually going to add put! and take! for other data structures, and/or queue operations for channels, then I think we should just consider channels a special kind of queue and use the queue names everywhere. They are just queues that happen to block when empty or full.

@jakebolewski
Copy link
Member

This will mesh really nicely with using accelerators such as GPU's. I like the fact that at a high level you will be able to put! or take! an Array to/from a GPU as easily as a remote processor. One thing to think about from an API perspective is distinguishing between blocking/nonblocking calls. It will be nice to have a consistent API for this.

@amitmurthy
Copy link
Contributor

Sort of related to the discussion above.

If RemoteRefis going to replaced by something like a Channel, which is basically a Q used for communication between processes, then I feel that the implementation/behavior of produce / consume should also be changed to use Channels too. We can thus have the same pattern for inter-task messaging - whether between tasks on the same process or between tasks on two different processes.

I suggest,

  • Every Task has two implicit Channels. An inbox and and outbox. These should actually be instantiated only when they are accessed for the first time.
  • recv() waits on the self input channel for a Task
  • recv(t) waits on the output channel for a Task t
  • consume(t, v) will be deprecated. It becomes equivalent to m=recv(t); send(t, v); m
  • send(v) appends to the output channel for self. Returns immediately - does not wait for it
    to be consumed. This helps the producer and consumer run on separate processes without any synchronization.
  • send(t, v) appends to the output channel for t. Returns immediately.

Independent Channels and their corresponding take! and put! will not be required in the normal course of development - they are low level APIs. To be used only when you want more complex messaging patterns like a single task waiting on multiple channels or similar.

remotecall* family of functions will be used for executing code on different processes, while messaging between tasks is via send/recv calls, with Channel, put! and take! being lower level functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This change will break code
Projects
None yet
Development

No branches or pull requests

4 participants