On Tue, May 13, 2014 at 4:40 PM, Clebert Suconic <csuco...@redhat.com>wrote:
> > > On May 13, 2014, at 1:46 PM, Rafael Schloming <r...@alum.mit.edu> wrote: > > > I'm not sure this will work as an API. One problem I see with this is > that > > it is circumventing the engine's flow control logic. If you notice there > is > > logic inside send() to update counters on the session. Unless I've missed > > something your patch doesn't seem to have equivalent logic. This might > just > > be an oversight, but I don't see how you could easily add the same logic > > since you don't know how many bytes the payload is until much much later > on > > in the control flow of the engine. > > > > as I told you this was just a prototyped idea... it's not in fact > updating the window yet.. > > If this idea is a good idea, we could pursue the idea here. > Providing the option to pass in something more abstract than a byte[] is a good idea. I'm just observing that the Payload/setPayload interface as it stands is changing two fundamental aspects of the send interface. The first aspect being that the Sender implementation no longer has any up front idea of how much data it is being offered. The second aspect is the stream oriented nature of the Sender interface. The interface is designed to operate in terms of chunks of message data rather than in terms of an entire message. This is intentional because an AMQP message might be arbitrarily large, and the interface needs to allow for the possibility of an intermediary that can stream through message data without buffering the whole message. I don't see how such streaming could easily be accomplished with the Payload/setPayload interface. > > > Can you supply some more detail as to why it got 5% faster? If it was > > merely avoiding the copy, then I can think of some ways we could avoid > that > > copy without changing the API quite so drastically, e.g. just overload > > send() to take some sort of releasable buffer reference. > > The encoding is done directly the the FrameWriter::__outputBuffer. I've > made a framework where I'm testing the send and it made it somewhat fast > than copying the encoding over 1 million messages. > > On this case it could be a bit more if you encoded a MesasgeImpl on a new > buffer every time.... > > > > > FWIW, I think that a good buffer abstraction that we could use everywhere > > would help a lot. I suspect having distinct abstractions for payload > > buffers vs encodable buffers vs decodable buffers is just going to result > > in lots of unnecessary conversions. > > probably.. I was just trying to improve the idea of the payloads. I don't > like the send API right now.. I think it would make more sense to set the > payload on the delivery than send bytes through sender. > This is really a question of the generality of the API. Operating in terms of chunks of a message is always going to be lower level than operating in terms of entire messages, however it is also more general. If we only permitted the latter then we would be omitting important use cases. I think we can treat these issues separately though. We should be able to eliminate copying in the stream oriented API, while still providing a convenience API to make sending discrete messages more convenient. --Rafael
