Skip to content

Commit

Permalink
apacheGH-524: Performance: avoid buffer copying
Browse files Browse the repository at this point in the history
In ChannelAsyncOutputStream, do not create a new SSH packet if the
caller already had passed in an SSH packet buffer, and we're writing
it fully. This avoids allocating a new buffer, which gives GC less
to do, and it also avoid needlessly copying the data around. Instead
the prepared SSH packet buffer is encoded and written directly.

Bug: apache#524
  • Loading branch information
tomaswolf committed Jul 15, 2024
1 parent f2dad09 commit cc3cefc
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.apache.sshd.common.io.WritePendingException;
import org.apache.sshd.common.session.Session;
import org.apache.sshd.common.session.SessionContext;
import org.apache.sshd.common.session.helpers.PacketBuffer;
import org.apache.sshd.common.util.buffer.Buffer;
import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
import org.apache.sshd.common.util.closeable.AbstractCloseable;
Expand Down Expand Up @@ -397,6 +398,20 @@ protected void onWritten(IoWriteFutureImpl future, int total, int length, IoWrit
protected Buffer createSendBuffer(Buffer buffer, Channel channel, int length) {
SessionContext.validateSessionPayloadSize(length, "Invalid send buffer length: %d");

if (buffer instanceof PacketBuffer && buffer.available() == length && cmd == SshConstants.SSH_MSG_CHANNEL_DATA) {
// Caller had prepared a packet buffer with SSH header and everything already,
// and we're writing it completely.
//
// Fill in cmd and length, and off we go.
int wpos = buffer.wpos();
buffer.rpos(SshConstants.SSH_PACKET_HEADER_LEN);
buffer.wpos(SshConstants.SSH_PACKET_HEADER_LEN);
buffer.putByte(cmd);
buffer.putUInt(channel.getRecipient());
buffer.putUInt(length);
buffer.wpos(wpos);
return buffer;
}
Session s = channel.getSession();
Buffer buf = s.createBuffer(cmd, length + 12);
buf.putUInt(channel.getRecipient());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1361,7 +1361,7 @@ protected boolean doInvokeUnimplementedMessageHandler(int cmd, Buffer buffer) th
@Override
public Buffer createBuffer(byte cmd, int len) {
if (len <= 0) {
return prepareBuffer(cmd, new ByteArrayBuffer());
return prepareBuffer(cmd, new PacketBuffer());
}

// Since the caller claims to know how many bytes they will need
Expand All @@ -1376,7 +1376,7 @@ public Buffer createBuffer(byte cmd, int len) {
len += outMacSize;
}

return prepareBuffer(cmd, new ByteArrayBuffer(new byte[len + Byte.SIZE], false));
return prepareBuffer(cmd, new PacketBuffer(new byte[len + Byte.SIZE], false));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.sshd.common.session.helpers;

import org.apache.sshd.common.util.buffer.ByteArrayBuffer;

/**
* A {@link ByteArrayBuffer} that will be prepared with the SSH header (5 bytes).
*/
public class PacketBuffer extends ByteArrayBuffer {

public PacketBuffer() {
super();
}

public PacketBuffer(byte[] buf, boolean readOnly) {
super(buf, readOnly);
}
}

0 comments on commit cc3cefc

Please sign in to comment.