I took a look at your changes, and I think they are very good. However, I want to discuss some changes I made to them to accommodate some of the constraint requirements that NMS has, as well as to safely introduce these changes.
First, I would like to keep the default TcpNoDelayEnabled setting as false, rather than changing it to true. Without knowing how it will impact other users, I think this is safer, and for those who want to tune their connection, they can do so. If you can clarify how changing the default will help everyone in the general case, then we can change the default setting. I just don't know what the full ramifications might be. Second, there are two sides to the communication -- the client socket and the broker socket. The WireFormatInfo sets the broker's socket settings, and your changes wait for the renegotiateWireFormat to come back from the broker in order to set the TcpNoDelayEnabled setting on the client's socket. This will only work for OpenWire format, and not for STOMP. Because of this, and other reasons, we need to have two separate setting flags. I suggest the flag for setting the client's socket setting to be "transport.TcpNoDelayEnabled". As you already know, the existing broker's setting is "wireformat.TcpNoDelayEnabled". Since this is wireformat specific, it is better to have a separate setting that can directly control the client's socket settings. Also, for Vadim, my previous post that showed "connection.NoDelay=true" was incorrect. It should have been "transport.NoDelay=true". However, that has been changed to "transport.TcpNoDelayEnabled=true" to be consistent with the existing "wireformat.TcpNoDelayEnabled" parameter. This may be a bit confusing, so let me throw out some sample connection URIs. I propose that nothing change to the existing defaults. If a user wants to override the settings, here is how it would look: activemq:tcp://localhost:61616?transport.TcpNoDelayEnabled=true This would set the client-side socket to turn on the NoDelay flag. activemq:tcp://localhost:61616?transport.TcpNoDelayEnabled=true&wireformat.TcpNoDelayEnabled=true This would set both the client side and broker side sockets to turn on the NoDelay flag. I haven't committed these changes yet. I'd like to get your feedback on this approach. Best, Jim semog wrote: > > Hi Stefan, > > Thanks for fixing the license. I'll take a look at the code changes. > This > is one of those areas where I think we have the potential for speeding up > the code, but also the potential for creating subtle problems. I'd like > to > make sure that these changes get tested very closely. > > The changes I put in change the start-up negotiation, at least that is the > intention. :) I'll send a message when the code is in there. Thanks > again > for contributing! > > - Jim > > On Thu, Aug 28, 2008 at 12:59 AM, Stefan_ <[EMAIL PROTECTED]> wrote: > >> >> Hi Jim, >> >> Sorry, but I simple overlooked the license agreement. I uploaded the >> patch >> again with the ASF license. >> >> I think adding the option to the URI is a good thing but Open Wire >> already >> negotiated the NoDelay option at connection start up. Unfortunately NMS >> hasn't yet supported this option so both the broker and the client use >> the >> Nagle algorithm which results in delays when sending messages. >> >> BTW I also removed the flush method call after writing to the socket as I >> recognized that this call also slows down the throughput. >> >> I would be very happy if the patch could be integrated into the source. >> >> Stefan >> >> >> >> semog wrote: >> > >> > Since I couldn't look at your code because of the license grant issue, >> > I looked in to what you had mentioned about the NoDelay option. I >> > took a stab at adding support for turning this (and several other >> > socket transport options) on and off from the connection URI. Once >> > you fix the license grant, I can look at your patch and integrate it >> > in with my changes. >> > >> > The solution I am playing with would look like this: >> > >> > activemq:tcp://localhost:61616?connection.NoDelay=true >> > >> > This would turn off the Nagle algorithm on the socket connection. >> > >> > Thanks! >> > -Jim >> > >> > >> >> -- >> View this message in context: >> http://www.nabble.com/Slow-performance-of-NMS-api-compared-to-Java-tp19158553p19195937.html >> Sent from the ActiveMQ - User mailing list archive at Nabble.com. >> >> > > -- View this message in context: http://www.nabble.com/Slow-performance-of-NMS-api-compared-to-Java-tp19158553p19205566.html Sent from the ActiveMQ - User mailing list archive at Nabble.com.
