RATIS-589. Eliminate buffer copying in SegmentedRaftLogOutputStream.

[szetszwo]

See RATIS-589

[SzyWilliam]

@szetszwo Thanks for the patch! the idea of zero copy is exciting!
However, the current implementation seems to have an issue. While avoiding one internal heap copy operation, we are now facing an additional direct memory to heap copy operation.

[SzyWilliam]

Since we are using DirectByteBuffer here, the checksum operation essentially reads all data from direct memory back to heap memory. Even worse, the UsafeOperation will do a lot of validations before each ByteBuffer.get, leading to significant performance issues.

// from DirectByteBuffer.java
public byte get(int i) {
        try {
            return ((SCOPED_MEMORY_ACCESS.getByte(session(), null, ix(checkIndex(i)))));
        } finally {
            Reference.reachabilityFence(this);
        }
    }

Maybe we can found a way to compute checksum directly on LogEntryProto, since it's a heap object?

[szetszwo]

@SzyWilliam , thanks for reviewing this!

Since we are using DirectByteBuffer here, the checksum operation essentially reads all data from direct memory back to heap memory.

You are right that it has to read the direct buffer for CRC computation. Even for byte[], it has to read the array. There seems no ways to avoid reading it for CRC computation.

Even worse, the UsafeOperation will do a lot of validations before each ByteBuffer.get, leading to significant performance issues.

Good point! We may use getLong(..) to reduce the number of calls.

According to the result from https://stackoverflow.com/questions/59008751/direct-java-nio-bytebuffer-vs-java-array-performance-test , the performance for byte[] is better than direct ByteBuffer for memory access. However, we will gain back the performance when including the buffer copying.

[szetszwo]

... While avoiding one internal heap copy operation, we are now facing an additional direct memory to heap copy operation.

Yes, this change avoids

• one copy (i.e. one heap read and one heap write)
• one more heap read for crc

but adds

• one direct buffer read for crc

Maybe we can found a way to compute checksum directly on LogEntryProto, since it's a heap object?

This is a good suggestion! Let me see if there is a way to do it.

[szetszwo]

... Let me see if there is a way to do it.

One idea (similar to DigestOutputStream) is to create a new class CrcByteBuffer extending ByteBuffer. When ever something is written to it, it will update CRC. Then, it avoid reading from the buffer.

[szetszwo]

... create a new class CrcByteBuffer extending ByteBuffer.

Since there are package private method CrcByteBuffer have to be in java.nio. Everything seems to work until getting

• java.lang.SecurityException: Prohibited package name: java.nio

when running unit tests.

[SzyWilliam]

We may use getLong(..) to reduce the number of calls.

You are right! getLong() seems good enough to amortize one index validation to 8 bytes.

Since there are package private method CrcByteBuffer have to be in java.nio. Everything seems to work until getting
java.lang.SecurityException: Prohibited package name: java.nio

Thanks for taking time on this exploration! That's really unfortunate, we are almost there. Still, I think getLong is good enough!

[SzyWilliam]

+1 the change looks good to me. Thanks @szetszwo for the big patch! It's exciting to see the zero-copy features!

[szetszwo]

@SzyWilliam , thanks a lot for reviewing this and the suggestions!