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

data ownership of zmq_msg_init_data #47

Closed
stevengj opened this issue Feb 4, 2014 · 14 comments
Closed

data ownership of zmq_msg_init_data #47

stevengj opened this issue Feb 4, 2014 · 14 comments

Comments

@stevengj
Copy link
Contributor

stevengj commented Feb 4, 2014

I was looking over the Message constructor code because of #46, and the call to zmq_msg_init_data frightens me, especially in Message(io::IOBuffer).

According to the zmq_msg_init_data docs, this does not make a copy and "ØMQ shall take ownership of the supplied buffer". That makes it sound like terrible things will happen if Julia garbage-collects the data (e.g. the IOBuffer) before ØMQ is done with it. Maybe we've just been lucky so far?

Options:

  • Make a copy of the data. This is annoying for sending large buffers.
  • Store a reference to the data in an ObjectIdDict or something, to prevent it from being garbage-collected. Use the zmq_free_fn callback parameter to zmq_msg_init_data to remove the data from the dict once ØMQ is done with it. Problem: the callback needs to be threadsafe, so it probably needs to just call uv_async_send or something to tell Julia to gc the object once it wakes up.
@stevengj
Copy link
Contributor Author

stevengj commented Feb 5, 2014

When I first wrote this, I was thinking that the bufferorigin field of Message would be enough to prevent garbage collection of the buffer origin, but this has the chronology wrong: when Message is gc'ed, it calls zmq_msg_close, which tells ØMQ that it can deallocate the message, but ØMQ does not actually do so until it is done with the message, which might happen some time later. We have to hold on to the bufferorigin until ØMQ is done with the message, not just until Julia is done with the Message.

@StefanKarpinski
Copy link
Contributor

Thanks for looking into this. That code scared the bejeezus out of me too when I was looking at it. It would be great if we can arrange for ZMQ.recv to safely return a plain byte array without making a copy. Does ZMQ not allow us to provide it with an allocator for byte buffers?

@stevengj
Copy link
Contributor Author

stevengj commented Feb 5, 2014

I'm not worried about ZMQ.recv. Having Message <: AbstractVector{Uint8} seems enough for me.

The problem with zmq_msg_init_data is with sending 0-copy data.

@stevengj
Copy link
Contributor Author

stevengj commented Feb 5, 2014

I think the reason we haven't been bitten by this is that for sending strings or arrays in ØMQ version 3 or later, we currently use zmq_send, which doesn't use the Message type at all and I think makes a copy of the data.

@stevengj
Copy link
Contributor Author

stevengj commented Feb 5, 2014

However, I have a fix for the zero-copy Message constructor that stores a reference to the buffer origin in a global dictionary, and uses uv_async_send to thread-safely delete the reference when ØMQ is done with the data. Testing now.

@StefanKarpinski
Copy link
Contributor

Phenomenal. I'd love to simplify this API and getting better performance on top of that is a huge win.

@stevengj
Copy link
Contributor Author

stevengj commented Feb 5, 2014

I'm wondering if the zero-copy version should be called send! as a reminder to the caller that it holds on to its arguments, with the copying version called send (which might actually be faster for very short messages, since it will avoid the whole uv_async callback mechanism).

@stevengj
Copy link
Contributor Author

stevengj commented Feb 5, 2014

Of course, the old way might still be safe as long as the user doesn't pass the NOBLOCK flag? I'm not sure about the zmq_send semantics here...

@StefanKarpinski
Copy link
Contributor

So the ZMQ.send call continues to hold onto the array after it returns? Is this because ZMQ might not get a chance to send the message all at once? In which case, if the caller were to modify the buffer before the rest of the message gets sent, which would cause a mangled message to get sent? I wonder if it wouldn't be better to just make ZMQ.send block until ZMQ is done with the byte array. That's usually fine and doesn't allow anything strange to happen. If that does happen to be a performance issue, the caller can use ZMQ.send! instead.

There may be some opportunity for a ZMQ.send(s) do io ... end interface to do more clever buffer management in the case of a large buffer since it controls the buffer. The idea would be that it can return while ZMQ is still working on sending the message, keeping track of the fact that the buffer is still in use, and if another ZMQ.send call occurs while that buffer is still in use, allocate a new buffer for the new call, keeping the allocated buffers in a pool.

@StefanKarpinski
Copy link
Contributor

Thinking a little more on it, I don't think that ZMQ.send! is a good idea to expose. The problem is that the person calling this function knows that they shouldn't modify the buffer they gave it until ZMQ is done with it, but they have no way of knowing when ZMQ will be done with it. We do have a convention elsewhere in Julia that arguments to constructors "take ownership" of their arguments (e.g. UTF8String(::Array{Uint8})), so maybe the way to deal with this is to write ZMQ.send(Message(buf)), understanding that the message object that's constructed owns the buffer after this. Then we could define

ZMQ.send(buf::Array{Uint8}) = ZMQ.send(Message(copy(buf)))

or we could make it a blocking call instead.

@stevengj
Copy link
Contributor Author

stevengj commented Feb 5, 2014

How about this:

  • Message(foo) owns foo, and is allowed to assume that foo doesn't change underneath it (once send is called) for as long as ØMQ needs the data. This is the method of choice if you have an Array that you want to send without making a copy.
  • ZMQ.send(socket, foo) makes a copy of foo, unless foo is immutable (e.g. foo::String).
  • ZMQ.send(socket) do io ... end does not need to make a copy, since it owns io once the do block completes.

stevengj added a commit that referenced this issue Feb 5, 2014
@stevengj
Copy link
Contributor Author

stevengj commented Feb 5, 2014

I just pushed a possible solution to the zerocopy branch.

@stevengj
Copy link
Contributor Author

Ping.

@StefanKarpinski
Copy link
Contributor

Thanks for doing this. Sorry I missed your pings earlier – this looks (belatedly) very good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants