search

cartesi / machine-emulator

The off-chain implementation of the Cartesi Machine
GNU Lesser General Public License v3.0
58 stars 32 forks source link

Use keep alive connection for JSONRPC client #236

Closed edubart closed 1 month ago

edubart commented 2 months ago

This PR optimizes JSONRPC remote machine client to use keep alive connections when performing subsequent requests. This is an optimization that increases the amount of request throughput we can perform via remote machines, and also fixes possible issues about exhausting the amount of available client ports for an OS that does not support SO_LINGER socket workaround.

The SO_LINGER workaround is still available in the code, because it is still very useful when performing thousands of fork requests per second, where we cannot make full use of keep alive connections.

The improvement of performance is almost 2x, for instance, for the following loop:

  while true do
    machine:read_mcycle()
  end

These are the statistics in number of iterations per second:

JSONRPC (before this PR)  52500 req/s
JSONRPC ( after this PR)  95105 req/s
local                     19699651 req/s

Therefore this is about +81% of throughput improvement, still way below local machines, but fair enough.

Depends on

edubart commented 2 months ago

Why does the client seem to support multiple connections open?

I made a keep alive connection pool without making assumptions on how the parent class is using it, so I would not have to rework it (fewer changes) and also allowed me to optimize the use case of snapshot/rollback. While it does support keeping alive connections for multiple servers, it won't be used that much, because we will have at maximum 2 keep alive connections left open. I put a check on the amount of maximum keep alive connections to not let connections leak in future code changes, to be explicit about this, and to make sure things are working correctly, would also feel wrong for me to leave a connection pool without an upper limit. In practice this check will always fail because at this moment the jsonrpc client cannot keep alive more than 2 simultaneous connections.

Doesn't the client always talk to the same server? Does this have something to do with snapshot/rollback? If so, wouldn't it be simpler to keep a single connection open, the one to the top server?

Most times the class has one connection open, except when you do a snapshot(), in this case we will have 2 connections open until commit() or rollback() is called. Keeping two connections open we save one extra reconnection when we are done with the snapshot. So this is an optimization to speed up the snapshot/commit/rollback use case.

I lowered the MAX_KEEP_ALIVE_POOL to 2 and made a comment about this, to make more clear.

diegonehab commented 2 months ago

I was just wondering if this could have been a much simpler/smaller patch with more or less the same performance if you kept a single open connection.

edubart commented 2 months ago

I was just wondering if this could have been a much simpler/smaller patch with more or less the same performance if you kept a single open connection.

It could be a little smaller with the downside of having a slightly slower commit/rollback, because one extra reconnection. I think the patch is already simple/small enough.

edubart commented 1 month ago

This PR was superseded by https://github.com/cartesi/machine-emulator/pull/240