SzyWilliam commented on code in PR #964:
URL: https://github.com/apache/ratis/pull/964#discussion_r1391934644


##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLogOutputStream.java:
##########
@@ -98,19 +87,26 @@ public void write(LogEntryProto entry) throws IOException {
     final int serialized = entry.getSerializedSize();
     final int proto = CodedOutputStream.computeUInt32SizeNoTag(serialized) + 
serialized;
     final int total = proto + 4; // proto and 4-byte checksum
-    final byte[] buf = sharedBuffer != null? sharedBuffer.get(): new 
byte[total];
-    Preconditions.assertTrue(total <= buf.length, () -> "total = " + total + " 
> buf.length " + buf.length);
     preallocateIfNecessary(total);
 
-    CodedOutputStream cout = CodedOutputStream.newInstance(buf);
-    cout.writeUInt32NoTag(serialized);
-    entry.writeTo(cout);
+    out.writeToBuffer(total, buf -> {
+      final int pos = buf.position();
+      final int protoEndPos= pos + proto;
+
+      final CodedOutputStream encoder = CodedOutputStream.newInstance(buf);
+      encoder.writeUInt32NoTag(serialized);
+      entry.writeTo(encoder);
 
-    checksum.reset();
-    checksum.update(buf, 0, proto);
-    ByteBuffer.wrap(buf, proto, 4).putInt((int) checksum.getValue());
+      // compute checksum
+      final ByteBuffer duplicated = buf.duplicate();
+      duplicated.position(pos).limit(protoEndPos);
+      checksum.reset();
+      checksum.update(duplicated);

Review Comment:
   Since we are using `DirectByteBuffer` here, the checksum operation 
essentially **reads all data from direct memory back to heap memory**. Even 
worse, the `UsafeOperation` does a lot validations before each 
`ByteBuffer.get`, leading to significant performance issues.
   ```java
   // 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?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to