[WIP][Driver/C] Driver should use weaker incremements

[pveentjer]

In the C driver, many counter increments are done using:

aeron_counter_increment

peter@adaptive-8267:~/adaptive/aeron/aeron-driver/src/main/c$ grep 'aeron_counter_increment(' . -R
./aeron_driver_receiver.h:    aeron_counter_increment(receiver->errors_counter);
./aeron_driver_name_resolver.c:    aeron_counter_increment(resolver->error_counter);
./aeron_driver_name_resolver.c:        aeron_counter_increment(resolver->invalid_packets_counter);
./aeron_driver_name_resolver.c:            aeron_counter_increment(resolver->invalid_packets_counter);
./aeron_driver_name_resolver.c:                aeron_counter_increment(resolver->invalid_packets_counter);
./aeron_driver_name_resolver.c:                aeron_counter_increment(resolver->invalid_packets_counter);
./aeron_driver_name_resolver.c:            aeron_counter_increment(resolver->short_sends_counter);
./aeron_retransmit_handler.c:        aeron_counter_increment(handler->invalid_packets_counter);
./aeron_network_publication.c:                aeron_counter_increment(publication->short_sends_counter);
./aeron_network_publication.c:                aeron_counter_increment(publication->short_sends_counter);
./aeron_network_publication.c:            aeron_counter_increment(publication->short_sends_counter);
./aeron_network_publication.c:                    aeron_counter_increment(publication->short_sends_counter);
./aeron_network_publication.c:                aeron_counter_increment(publication->short_sends_counter);
./aeron_driver_conductor.c:    aeron_counter_increment(conductor->errors_counter);
./aeron_driver_sender.h:    aeron_counter_increment(sender->errors_counter);
./media/aeron_udp_channel_transport.h:    aeron_counter_increment(transport->errors_counter);
./media/aeron_send_channel_endpoint.c:        aeron_counter_increment(sender->invalid_frames_counter);
./media/aeron_send_channel_endpoint.c:                aeron_counter_increment(sender->invalid_frames_counter);
./media/aeron_send_channel_endpoint.c:                aeron_counter_increment(sender->invalid_frames_counter);
./media/aeron_send_channel_endpoint.c:                aeron_counter_increment(sender->invalid_frames_counter);
./media/aeron_send_channel_endpoint.c:                aeron_counter_increment(sender->invalid_frames_counter);
./media/aeron_send_channel_endpoint.c:                aeron_counter_increment(sender->invalid_frames_counter);
./media/aeron_receive_channel_endpoint.c:            aeron_counter_increment(endpoint->short_sends_counter);
./media/aeron_receive_channel_endpoint.c:            aeron_counter_increment(endpoint->short_sends_counter);
./media/aeron_receive_channel_endpoint.c:            aeron_counter_increment(endpoint->short_sends_counter);
./media/aeron_receive_channel_endpoint.c:            aeron_counter_increment(endpoint->short_sends_counter);
./media/aeron_receive_channel_endpoint.c:            aeron_counter_increment(channel_endpoint->short_sends_counter);
./media/aeron_receive_channel_endpoint.c:        aeron_counter_increment(channel_endpoint->errors_frames_sent_counter);
./media/aeron_receive_channel_endpoint.c:        aeron_counter_increment(receiver->invalid_frames_counter);
./media/aeron_receive_channel_endpoint.c:                aeron_counter_increment(receiver->invalid_frames_counter);
./media/aeron_receive_channel_endpoint.c:                aeron_counter_increment(receiver->invalid_frames_counter);
./media/aeron_receive_channel_endpoint.c:                aeron_counter_increment(receiver->invalid_frames_counter);

On Linux with GCC this translates to an xadd.

#define AERON_GET_AND_ADD_INT64(original, dst, value)                         \
do                                                                            \
{                                                                             \
    __asm__ __volatile__(                                                     \
        "lock; xaddq %0, %1"                                                  \
        : "=r"(original), "+m"(dst)                                           \
        : "0"((int64_t)value)                                                 \
        : "memory", "cc"                                                      \
        );                                                                    \
}                                                                             \
while (false)

I'm assuming all of the above counter usages, are written by a single thread, so there is no need for an increment that properly handles concurrent writers. lock xaddq is worse than a Java style volatile write (sequential consistent write) because it not only waits for the store buffer to be drained, inside the core there is some serialization of instructions to make sure that only 1 atomic instruction travels through the CPU instruction pipeline.

I added the aeron_counter_increment_opaque:

inline int64_t aeron_counter_increment_opaque(volatile int64_t *addr)
{
    int64_t current_value = *addr;
    *addr = current_value + 1;
    return current_value;
}

Although I'm not crazy about this Java name in the C code, it does exactly what is needed and it matches the Java semantics. The addr is volatile so the write will not be optimized out and GCC will ensure that value is properly aligned, so you don't end up with torn reads/writes (so the read and the write is atomic).

A slightly more expensive alternative would be to use the aeron_counter_increment_release:

inline int64_t aeron_counter_increment_release(volatile int64_t *addr)
{
    int64_t current_value = *addr;
    AERON_SET_RELEASE(*addr, current_value + 1);
    return current_value;
}

which you get for free on the X86 since every store is a release store. But you still need to pay the price for this ordering behavior on e.g. ARM even if it isn't needed and this slows down code for no good reason. It think it is best to go for the weakest ordering that still is correct so that we express intent and do not obstruct performance.

Currently there already exists aeron_counter_increment_plain.

inline int64_t aeron_counter_increment_plain(volatile int64_t *addr)
{
    int64_t current_value = *addr;
    *addr = current_value + 1;
    return current_value;
}

But if you want to match Java semantics, the volatile type qualifier should be dropped. I think the method should be dropped because counters are written to shared memory and there could be 1 or more readers, so we should make sure that writes are not optimized out.

Counters that increment by more than 1 make use of the aeron_counter_get_and_add_release which translate to a much cheaper release store

inline int64_t aeron_counter_get_and_add_release(volatile int64_t *addr, int64_t value)
{
    int64_t current = *addr;
    AERON_SET_RELEASE(*addr, current + value);
    return current;
}

Although they could also be relaxed, they are already cheaper than the aeron_counter_increment

The Java driver has similar problems. Lot of calls to increment() and excessive synchronization.