You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Shawn Pearce <so...@google.com> on 2009/12/15 01:15:08 UTC
sshd 0.3.0 performance regression
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.
Any thoughts as to why this is causing a 50% drop on throughput
from server to client?
commit 49e19df60a9d45322491dda2fc608969e6811db5
Author: Guillaume Nodet <gn...@apache.org>
Date: Fri Nov 20 10:48:50 2009 +0000
SSHD-49: Fix a problem with the Output channel logic for the remote window
git-svn-id: https://svn.apache.org/repos/asf/mina/sshd/trunk@882491 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/channel/ChannelOutputStream.java b/sshd-core/src/main/java/org/apache/sshd/common/channel/ChannelOutputStream.java
index aab789a..63bcd1a 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/channel/ChannelOutputStream.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/channel/ChannelOutputStream.java
@@ -59,17 +59,8 @@ public class ChannelOutputStream extends OutputStream {
if (closed) {
throw new SshException("Already closed");
}
- while (l > 0) {
- int _l = Math.min(l, remoteWindow.getPacketSize() - bufferLength);
- if (_l <= 0) {
- flush();
- continue;
- }
- buffer.putRawBytes(buf, s, _l);
- bufferLength += _l;
- s += _l;
- l -= _l;
- }
+ buffer.putRawBytes(buf, s, l);
+ bufferLength += l;
}
@Override
@@ -81,14 +72,14 @@ public class ChannelOutputStream extends OutputStream {
while (bufferLength > 0) {
Buffer buf = buffer;
int total = bufferLength;
- int length = Math.min(remoteWindow.waitForSpace(), total);
+ int length = Math.min(Math.min(remoteWindow.waitForSpace(), total), remoteWindow.getPacketSize());
int pos = buf.wpos();
buf.wpos(cmd == SshConstants.Message.SSH_MSG_CHANNEL_EXTENDED_DATA ? 14 : 10);
buf.putInt(length);
- buf.wpos(pos);
+ buf.wpos(buf.wpos() + length);
newBuffer();
if (total > length) {
- buffer.putBytes(buf.array(), pos - (total - length), total - length);
+ buffer.putRawBytes(buf.array(), pos - (total - length), total - length);
bufferLength = total - length;
}
remoteWindow.waitAndConsume(length);
Re: sshd 0.3.0 performance regression
Posted by Guillaume Nodet <gn...@gmail.com>.
On Tue, Dec 15, 2009 at 05:05, Shawn Pearce <so...@google.com> wrote:
> Shawn Pearce <so...@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
Re: sshd 0.3.0 performance regression
Posted by Shawn Pearce <so...@google.com>.
Shawn Pearce <so...@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.
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.