pebbe / zmq4

A Go interface to ZeroMQ version 4
BSD 2-Clause "Simplified" License
1.18k stars 163 forks source link

Not possible to change the statue of an existing Poller when set up #75

Closed abligh closed 8 years ago

abligh commented 8 years ago

Poller.Add adds a new socket to an existing Poller, but there is no way of changing what is being polled, i.e. turning on and off IN/OUT polling. I would suggest that Poller.Add should be changed such that if passed an existing socket, it modifies the event status polled for, rather than re-adds the socket (which I believe is illegal anyway as a pollset cannot contain the same socket twice).

Would you accept a patch to this effect?

pebbe commented 8 years ago

This change would mean that everytime a socket is added, Poller.Add would have to go through the list of sockets to look for a match. Would that be a problem? I don't know how many sockets there typically are in a poller.

You could add a method Poller.Replace

But is there much demand for this, changing the event status?

I think a function Poller.Delete() might be more useful.

abligh commented 8 years ago

I could easily add Poller.Update(), which takes an offset into the slice and new Events state. That would be more useful than Delete() as I just want to change the Events state. Note you can use an Events state of 0 to not poll for anything (i.e. to temporarily stop polling one socket).

The use case is a non-blocking proxy where you want to avoid polling for output on a socket when you have nothing to output (for instance), but always want to be polling for input. Otherwise you need to make a new Poller each time.

On 13 Feb 2016, at 13:29, Peter Kleiweg notifications@github.com wrote:

This change would mean that everytime a socket is added, Poller.Add would have to go through the list of sockets to look for a match. Would that be a problem? I don't know how many sockets there typically are in a poller.

You could add a method Poller.Replace

But is there much demand for this, changing the event status?

I think a function Poller.Delete() might be more useful.

� Reply to this email directly or view it on GitHub.

Alex Bligh

pebbe commented 8 years ago

Alex Bligh notifications@github.com schreef op 13 februari 2016 15:04:59 CET:

I could easily add Poller.Update(), which takes an offset into the slice and new Events state. That would be more useful than Delete() as I just want to change the Events state. Note you can use an Events state of 0 to not poll for anything (i.e. to temporarily stop polling one socket).

That sounds like a good idea.

But do you always know at what position in the poller a socket is located? You might need two functions, one for updating by index, the other for updating by socket pointer.

Or you could make Poller.Add return the index of the added socket. Currently, it doesn't return anything, so it wouldn't break existing code. Then just Poller.Update by index would probably be enough.

Peter Kleiweg http://pkleiweg.home.xs4all.nl

abligh commented 8 years ago

If you simply add all the sockets to the poller at the start of the loop, you know where they are located as it's the order in which you added them. You need to know this in order to usefully interpret the response in any case. You can then update the appropriate index. The indices never change if nothing is ever added or removed. You can achieve the same effect as 'delete' by setting 'events' to zero. IE within the loop, the only thing that changes in the array is 'Events'.

That said, making add return the index would make for a much nicer interface.

I could make it it update the socket pointers, and provide a facility to remove them as well, but that's more than I need for my purposes. Anyway, if you are interested in a patch I will try to knock one up.

On 13 Feb 2016, at 14:27, Peter Kleiweg notifications@github.com wrote:

Alex Bligh notifications@github.com schreef op 13 februari 2016 15:04:59 CET:

I could easily add Poller.Update(), which takes an offset into the slice and new Events state. That would be more useful than Delete() as I just want to change the Events state. Note you can use an Events state of 0 to not poll for anything (i.e. to temporarily stop polling one socket).

That sounds like a good idea.

But do you always know at what position in the poller a socket is located? You might need two functions, one for updating by index, the other for updating by socket pointer.

Or you could make Poller.Add return the index of the added socket. Currently, it doesn't return anything, so it wouldn't break existing code. Then just Poller.Update by index would probably be enough.

Peter Kleiweg http://pkleiweg.home.xs4all.nl � Reply to this email directly or view it on GitHub.

Alex Bligh

pebbe commented 8 years ago

Perhaps sockets are added to the poller in a more complex way, and you don't know what socket is at what postition. You don't need to know the positions for Poller.Poll, only for Poller.PollAll.

A Poller.Update() that uses indices, and Poller.Add returning the position, I would leave it at that for the moment. I don't want to add things that are not really needed.

I don't know if the whole Poller thing was a good idea. I added it for the examples in the tutorial, but there may be other ways to achieve the same thing, with each socket its own goroutine, using a select instead of polling.

abligh commented 8 years ago

OK thanks. I'll add that and send you a pull request.

Incidentally, what I'm writing is something that wraps a zmq socket into channels, so you can use normal channel semantics. You're welcome to that too if you are interested, though right now I'm doing it in an additional package.

On 13 Feb 2016, at 18:07, Peter Kleiweg notifications@github.com wrote:

Perhaps sockets are added to the poller in a more complex way, and you don't know what socket is at what postition. You don't need to know the positions for Poller.Poll, only for Poller.PollAll.

A Poller.Update() that uses indices, and Poller.Add returning the position, I would leave it at that for the moment. I don't want to add things that are not really needed.

I don't know if the whole Poller thing was a good idea. I added it for the examples in the tutorial, but there may be other ways to achieve the same thing, with each socket its own goroutine, using a select instead of polling.

� Reply to this email directly or view it on GitHub.

Alex Bligh

abligh commented 8 years ago

See PR #76