Re: sshd 0.3.0 performance regression

2009-12-14 Thread Shawn Pearce
Shawn Pearce  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.

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.

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.


Re: sshd 0.3.0 performance regression

2010-02-20 Thread Guillaume Nodet
On Tue, Dec 15, 2009 at 05:05, Shawn Pearce  wrote:
> Shawn Pearce  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