perf changes

[robertroeser]

Changes the RScocket client and server to use the a synchronized version of the Agrona Int2ObjectHashMap. The NonBlockingHashMapLong was leading to GC pauses.

Uses SendPublisher that writes to the Netty channel directly - on my laptop the performance increase is approx 200% throughput improvement, with better tail latency on the Ping/Pong test as well our internal code that relies on RSocket.

[yschimke]

LGTM

I keep expecting the use of libraries like reactor to mean that the engine of RSocket is this shiny simple "only one way to logically write this" chrome engine. I'm still getting surprised that we end up writing so much stream infra code. In this case, things like SendPublisher.

[mostroverkhov]

LGTM

[rdegnan]

LGTM other than a few methods that should be synchronized -- I don't think that will make much difference to performance.

[rdegnan]

I think handleSendProcessorError and handleSendProcessorCancel should now be synchronized as they iterate over sendingSubscriptions/channelProcessors (previously this wasn't necessary because of Cliff Click magic).

[robertroeser]

I wrapped the map with Collections.synchronizedMap so it should effectively be doing that.

[rdegnan]

Collections.synchronizedMap doesn't actually help when you iterate over any of its collection views -- per the documentation:

 * It is imperative that the user manually synchronize on the returned
 * map when iterating over any of its collection views:
 * <pre>
 *  Map m = Collections.synchronizedMap(new HashMap());
 *      ...
 *  Set s = m.keySet();  // Needn't be in synchronized block
 *      ...
 *  synchronized (m) {  // Synchronizing on m, not s!
 *      Iterator i = s.iterator(); // Must be in synchronized block
 *      while (i.hasNext())
 *          foo(i.next());
 *  }
 * </pre>
 * Failure to follow this advice may result in non-deterministic behavior.

If we use forEach everywhere instead of a for loop that would also be fine.

[robertroeser]

I looked at the code again. I think we are fine - the problem would be if something adds something to map while we're iterating over it. Looking at the non blocking map the same thing would happen. We should probably add something to the request* methods so that you can't add items to the map when handling a cancel or an error, and then this isn't a problem.

[rdegnan]

I think handleSendProcessorError and handleSendProcessorCancel and terminate should now be synchronized as they iterate over senders/receivers (previously this wasn't necessary because of Cliff Click magic).

[robertroeser]

@yschimke I'm hoping these changes make it back into reactor-*

[He-Pin]

Netty already has one.

[robertroeser]

Switched to it