On Tue, Dec 15, 2009 at 05:05, Shawn Pearce <s...@google.com> wrote: > Shawn Pearce <s...@google.com> wrote: >> We're seeing a performance regression in SSHD 0.3.0, the throughput >> is about 1/2 of what it was in SSHD 0.2.0. >> >> Bisecting the problem in git led me to revision 882491, which was >> the bug fix for https://issues.apache.org/jira/browse/SSHD-49. > > I committed a few changes that actually improve this situation > considerably, we're now back to where we were with 0.2.0, and even > slightly faster as our write buffer allocation is now better sized > to meet packet writes.
Thx! > > Guillaume, I really didn't follow how the delta in 882491 fixed > SSHD-49's problem. You changed a bunch of stuff there all at once. > But it seems to me the root cause was using putBytes() and not > putRawBytes(), so the split packets were corrupted with an extra > 4 bytes of data due to the unnecessary length being inserted. IIRC, there were two problems with the test case for SSHD-49. The first one was the use of putBytes instead of putRawBytes, but there was also a problem with the buffering strategy, where in some cases (a very big message from server to client), the server was waiting forever for some space that would never come. The problem has been fixed in the following commit http://svn.apache.org/viewvc?view=revision&revision=835419 where I've added a new method to the Window object to make sure the server was not trying to send too much data. The real problem is that the loop / send / wait logic was done inside the write() method but was not taking into account all the parameters (the current buffer size for example). So the above commit actually move the logic into the flush method. The next commit (http://svn.apache.org/viewvc/mina/sshd/trunk/sshd-core/src/main/java/org/apache/sshd/common/channel/ChannelOutputStream.java?r1=882491&r2=882490&pathrev=882491) fixes another problem I found during testing. I thought, given the loop was actually moved to the flush() method that we did not really need it in the write() method. What if we remove the loop in the write() method and simply call flush() after having filled the buffer ? > The other changes related to buffering everything on the stream > until the application calls flush() seem to me to be completely > unrelated to solving SSHD-49, and actually caused problems for > Commands that write a lot of data and don't ever call flush(). > > I'm still a bit unhappy with throughput. I guess I need to dig into > SSHD-54 here. The throughput still isn't as fast as we'd like it > to be, especially over LAN connections. > -- Cheers, Guillaume Nodet ------------------------ Blog: http://gnodet.blogspot.com/ ------------------------ Open Source SOA http://fusesource.com
