Kryonet sendToTCP/UDP Optimization?

I was looking at some of the code in https://github.com/EsotericSoftware/kryonet/blob/master/src/com/esotericsoftware/kryonet/Server.java and noticed the functions (which are seemingly called quite frequently) iterate through the entire list of connections… in O(n) time instead of O(1) time by indexing with connectionID.


public void sendToTCP (int connectionID, Object object) {
		Connection[] connections = this.connections;
		for (int i = 0, n = connections.length; i < n; i++) {
			Connection connection = connections[i];
			if (connection.id == connectionID) {
				connection.sendTCP(object);
				break;
			}
		}
}

public void sendToUDP (int connectionID, Object object) {
		Connection[] connections = this.connections;
		for (int i = 0, n = connections.length; i < n; i++) {
			Connection connection = connections[i];
			if (connection.id == connectionID) {
				connection.sendUDP(object);
				break;
			}
		}
}

Why is that?

I suppose that the ConnectionID could be changed or resetted by some other function?
If not, i guess it would be a good idea.

Better to investigate a bit further or wait someone with more experience on kryonet to reply.

+1 For noticing that

The only faster way would be to use a HashMap or an array (array is not really practical).

List was probably used for convenience in some other place of the code.

java.util.HashMap would be faster with lots of connections, but then still pretty slow.

i guess, they didn’t bother to implement another int2object map and went the simple path.

How many loop iterations is enough to make one array lookup and one int comparison in each iteration slow enough to warrant using a map? Is that number greater than the number of connections your server might ever have?

Is there a reason why send***() takes in the connection ID instead of a Connection object?

Quick and dirty JMH bench says about 20 assuming the average is that the item is found in the middle of the array:

`size = 8
Benchmark Mode Samples Score Score error Units
o.s.LoopBench.loop thrpt 10 238.392 4.639 ops/us
o.s.LoopBench.map thrpt 10 171.006 3.299 ops/us

size = 16
Benchmark Mode Samples Score Score error Units
o.s.LoopBench.loop thrpt 10 180.809 2.018 ops/us
o.s.LoopBench.map thrpt 10 171.538 2.048 ops/us

size = 64
Benchmark Mode Samples Score Score error Units
o.s.LoopBench.loop thrpt 10 56.051 1.301 ops/us
o.s.LoopBench.map thrpt 10 171.549 3.525 ops/us

size = 1024
Benchmark Mode Samples Score Score error Units
o.s.LoopBench.loop thrpt 10 4.067 0.088 ops/us
o.s.LoopBench.map thrpt 10 170.738 2.863 ops/us`

EDIT: correction, it turns out the map impl ramps up to ~230 to break even at about 10
EDIT2: this is also jdk8; jdk7 etc. hashmaps are a bit slower, but not much.

I think with the delays inherent in networking, this kind of optimization will make no difference.

For a game server, it is relevant. The server may also be running other CPU intensive tasks, like game logic, physics, etc. It’s a simple optimization with a noticeable improvement for larger game servers.

Again, why does the send method take in an ID instead of a raw Connection object? Connection doesn’t seem to be some kind of internal object, as its returned by the listener and there’s a getter to get all connections. If the only way to get a connection ID is to get it from a Connection object, what’s the point? Just add an overloaded version of all those functions that take in a Connection object instead. The sender probably holds its Connection object anyway.

thats why we have key-attachments. somebody was just lazy :slight_smile:

Premature optimization is premature. True it’s a lib and it’s nice to be as efficient as possible, but in reality the optimization is extremely unlikely to ever make a difference.