views:

96

answers:

4

Hi!

I have build a simple application that opens a ServerSocket, and on connection, it connects itself to another server socket on a remote machine. To implement port forwarding, I use two threads, one that reads from the local inputstream and streams to the remote sockets outputstream, and vice versa.

The implementation feels a bit inperformant, and so I ask you if you know a better implementation strategy, or even have some code lying around to achive this in a performant way.

PS: I know I could use IPTables on Linux, but this has to work on Windows.

PPS: If you post implementations for this simple task, I will create a benchmark to test all given implementations. The solution should be fast for many small (~100bytes) packages and steady data streams.

My current implementation is this (executed on each of the two threads for each direction):

public static void route(InputStream inputStream, OutputStream outputStream) throws IOException {
    byte[] buffer = new byte[65536];
    while( true ) {
        // Read one byte to block
        int b = inputStream.read();
        if( b == - 1 ) {
            log.info("No data available anymore. Closing stream.");
            inputStream.close();
            outputStream.close();
            return;
        }
        buffer[0] = (byte)b;
        // Read remaining available bytes
        b = inputStream.read(buffer, 1, Math.min(inputStream.available(), 65535));
        if( b == - 1 ) {
            log.info("No data available anymore. Closing stream.");
            inputStream.close();
            outputStream.close();
            return;
        }
        outputStream.write(buffer, 0, b+1);
    }
}
A: 

If your code isn't performant, maybe your buffers aren't large enough.

Too small buffers mean that more request will be done and less performances.


On the same topic :

Colin Hebert
+1  A: 

Take a look at tcpmon. Its purpose is to monitor tcp data, but it also forwards to a different host/port.

And here is some code for port forwarding taken from a book (it's not in English, so I'm pasting the code rather than giving a link to the book e-version):

Bozho
Stupid me... I thought read(byte[]) would fill the byte array first
Daniel
A: 

Did 2 reads and one buffer check per loop iteration really sped things up and have you measured that? Looks like a premature optimization to me... From personal experience, simply reading into a small buffer and then writing it to the output works well enough. Like that: byte[] buf = new byte[1024]; int read = m_is.read(buf); while(read != -1) { m_os.write(buf, 0, read); m_fileOut.write(buf, 0, read); read = m_is.read(buf); } This is from an old proxy of mine that used InputStream.read() in the first version, then went to available() check + 1024 byte buffer in the second and settled on the code above in the third.

If you really really need performance (or just want to learn), use java.nio or one of the libraries that build on it. Do note that IO performance tends to behave wildly different on different platforms.

bod
+2  A: 

A couple of observations:

  • The one byte read at the start of the loop does nothing to improve performance. Probably the reverse in fact.

  • The call to inputStream.available() is unnecessary. You should just try to read to "buffer size" characters. A read on a Socket streamwill return as many characters as are currently available, but won't block until the buffer is full. (I cannot find anything in the javadocs that says this, but I'm sure it is the case. A lot of things would perform poorly ... or break ... if read blocked until the buffer was full.)

  • As @user479257 points out, you should get better throughput by using java.nio and reading and writing ByteBuffers. This will cut down on the amount of data copying that occurs in the JVM.

  • Your method will leak Socket Streams if a read, write or close operation throws an exception. You should use a try ... finally as follows to ensure that the streams are always closed no matter what happens.


public static void route(InputStream inputStream, OutputStream outputStream) 
throws IOException {
    byte[] buffer = new byte[65536];
    try {
        while( true ) {
            ...
            b = inputStream.read(...);
            if( b == - 1 ) {
                log.info("No data available anymore. Closing stream.");
                return;
            }
            outputStream.write(buffer, 0, b+1);
        }
    } finally {
        try { inputSteam.close();} catch (IOException ex) { /* ignore */ }
        try { outputSteam.close();} catch (IOException ex) { /* ignore */ }
    }
}
Stephen C
+1 I already found out about the unnessesary byte at the beginning. But you missed the b+1 at the end that should be just b. Is it faster to read NIO ByteBuffers? Aren't they just ordinary byte arrays if they don't come from disk but from a network interface?
Daniel
@Daniel - It is not **my** job to find all of your bugs. :-)
Stephen C