szetszwo commented on code in PR #964:
URL: https://github.com/apache/ratis/pull/964#discussion_r1392954044
##########
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:
@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.
--
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]
