merlimat closed pull request #1467: Avoid boxing of checksum into a Long
URL: https://github.com/apache/incubator-pulsar/pull/1467
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Consumer.java
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Consumer.java
index 4c5dbbca9..12782c43e 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Consumer.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Consumer.java
@@ -19,7 +19,6 @@
package org.apache.pulsar.broker.service;
import static com.google.common.base.Preconditions.checkArgument;
-import static org.apache.pulsar.common.api.Commands.readChecksum;
import java.util.Collections;
import java.util.Iterator;
@@ -37,7 +36,7 @@
import
org.apache.bookkeeper.util.collections.ConcurrentLongLongPairHashMap.LongPair;
import org.apache.pulsar.broker.PulsarServerException;
import org.apache.pulsar.broker.authentication.AuthenticationDataSource;
-import org.apache.pulsar.client.api.SubscriptionInitialPosition;
+
import org.apache.pulsar.common.api.Commands;
import org.apache.pulsar.common.api.proto.PulsarApi;
import org.apache.pulsar.common.api.proto.PulsarApi.CommandAck;
@@ -225,7 +224,7 @@ public SendMessageInfo sendMessages(final List<Entry>
entries) {
metadataAndPayload.retain();
// skip checksum by incrementing reader-index if
consumer-client doesn't support checksum verification
if (cnx.getRemoteEndpointProtocolVersion() <
ProtocolVersion.v11.getNumber()) {
- readChecksum(metadataAndPayload);
+ Commands.skipChecksumIfPresent(metadataAndPayload);
}
if (log.isDebugEnabled()) {
diff --git
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Producer.java
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Producer.java
index 35bc3151b..2088035cc 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Producer.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Producer.java
@@ -176,7 +176,7 @@ private boolean verifyChecksum(ByteBuf headersAndPayload) {
int readerIndex = headersAndPayload.readerIndex();
try {
- int checksum = readChecksum(headersAndPayload).intValue();
+ int checksum = readChecksum(headersAndPayload);
long computedChecksum = computeChecksum(headersAndPayload);
if (checksum == computedChecksum) {
return true;
diff --git
a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
index 08e4fb78e..09c807fef 100644
---
a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
+++
b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/ReplicatorTest.java
@@ -861,7 +861,7 @@ public void verifyChecksumAfterReplication() throws
Exception {
ByteBuf b = msg.getHeadersAndPayload();
assertTrue(Commands.hasChecksum(b));
- int parsedChecksum = Commands.readChecksum(b).intValue();
+ int parsedChecksum = Commands.readChecksum(b);
int computedChecksum = Crc32cIntChecksum.computeChecksum(b);
assertEquals(parsedChecksum, computedChecksum);
diff --git
a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/ChecksumTest.java
b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/ChecksumTest.java
index 3e6b3bd43..94c3762e8 100644
---
a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/ChecksumTest.java
+++
b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/ChecksumTest.java
@@ -74,7 +74,7 @@ public void verifyChecksumStoredInManagedLedger() throws
Exception {
ByteBuf b = entries.get(0).getDataBuffer();
assertTrue(Commands.hasChecksum(b));
- int parsedChecksum = Commands.readChecksum(b).intValue();
+ int parsedChecksum = Commands.readChecksum(b);
int computedChecksum = Crc32cIntChecksum.computeChecksum(b);
assertEquals(parsedChecksum, computedChecksum);
@@ -95,7 +95,7 @@ public void verifyChecksumSentToConsumer() throws Exception {
ByteBuf b = msg.getHeadersAndPayload();
assertTrue(Commands.hasChecksum(b));
- int parsedChecksum = Commands.readChecksum(b).intValue();
+ int parsedChecksum = Commands.readChecksum(b);
int computedChecksum = Crc32cIntChecksum.computeChecksum(b);
assertEquals(parsedChecksum, computedChecksum);
diff --git
a/pulsar-broker/src/test/java/org/apache/pulsar/compaction/CompactorTest.java
b/pulsar-broker/src/test/java/org/apache/pulsar/compaction/CompactorTest.java
index 1867ff490..59c46ecb2 100644
---
a/pulsar-broker/src/test/java/org/apache/pulsar/compaction/CompactorTest.java
+++
b/pulsar-broker/src/test/java/org/apache/pulsar/compaction/CompactorTest.java
@@ -207,7 +207,7 @@ public void testCompactEmptyTopic() throws Exception {
public ByteBuf extractPayload(RawMessage m) throws Exception {
ByteBuf payloadAndMetadata = m.getHeadersAndPayload();
- Commands.readChecksum(payloadAndMetadata);
+ Commands.skipChecksumIfPresent(payloadAndMetadata);
int metadataSize = payloadAndMetadata.readInt(); // metadata size
byte[] metadata = new byte[metadataSize];
payloadAndMetadata.readBytes(metadata);
diff --git
a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java
b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java
index c2154315c..bc62d3a54 100644
---
a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java
+++
b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java
@@ -1026,7 +1026,7 @@ private ByteBuf uncompressPayloadIfNeeded(MessageIdData
messageId, MessageMetada
private boolean verifyChecksum(ByteBuf headersAndPayload, MessageIdData
messageId) {
if (hasChecksum(headersAndPayload)) {
- int checksum = readChecksum(headersAndPayload).intValue();
+ int checksum = readChecksum(headersAndPayload);
int computedChecksum = computeChecksum(headersAndPayload);
if (checksum != computedChecksum) {
log.error(
diff --git
a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
index e1a505aa0..27757d119 100644
---
a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
+++
b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
@@ -733,7 +733,7 @@ protected boolean verifyLocalBufferIsNotCorrupted(OpSendMsg
op) {
headerFrame.skipBytes(cmdSize);
// verify if checksum present
if (hasChecksum(headerFrame)) {
- int checksum = readChecksum(headerFrame).intValue();
+ int checksum = readChecksum(headerFrame);
// msg.readerIndex is already at header-payload index,
Recompute checksum for headers-payload
int metadataChecksum = computeChecksum(headerFrame);
long computedChecksum = resumeChecksum(metadataChecksum,
msg.getSecond());
diff --git
a/pulsar-common/src/main/java/org/apache/pulsar/common/api/Commands.java
b/pulsar-common/src/main/java/org/apache/pulsar/common/api/Commands.java
index dcea2b2c8..f5470cce4 100644
--- a/pulsar-common/src/main/java/org/apache/pulsar/common/api/Commands.java
+++ b/pulsar-common/src/main/java/org/apache/pulsar/common/api/Commands.java
@@ -236,12 +236,19 @@ public static boolean hasChecksum(ByteBuf buffer) {
return buffer.getShort(buffer.readerIndex()) == magicCrc32c;
}
- public static Long readChecksum(ByteBuf buffer) {
- if(hasChecksum(buffer)) {
- buffer.skipBytes(2); //skip magic bytes
- return buffer.readUnsignedInt();
- } else{
- return null;
+ /**
+ * Read the checksum and advance the reader index in the buffer.
+ *
+ * Note: This method assume the checksum presence was already verified
before.
+ */
+ public static int readChecksum(ByteBuf buffer) {
+ buffer.skipBytes(2); //skip magic bytes
+ return buffer.readInt();
+ }
+
+ public static void skipChecksumIfPresent(ByteBuf buffer) {
+ if (hasChecksum(buffer)) {
+ readChecksum(buffer);
}
}
@@ -249,7 +256,7 @@ public static MessageMetadata parseMessageMetadata(ByteBuf
buffer) {
try {
// initially reader-index may point to start_of_checksum :
increment reader-index to start_of_metadata to parse
// metadata
- readChecksum(buffer);
+ skipChecksumIfPresent(buffer);
int metadataSize = (int) buffer.readUnsignedInt();
int writerIndex = buffer.writerIndex();
diff --git
a/pulsar-common/src/test/java/org/apache/pulsar/common/compression/CommandsTest.java
b/pulsar-common/src/test/java/org/apache/pulsar/common/compression/CommandsTest.java
index 182741606..186e7b222 100644
---
a/pulsar-common/src/test/java/org/apache/pulsar/common/compression/CommandsTest.java
+++
b/pulsar-common/src/test/java/org/apache/pulsar/common/compression/CommandsTest.java
@@ -58,7 +58,7 @@ public void testChecksumSendCommand() throws Exception {
/*** 1. verify checksum and metadataParsing ***/
boolean hasChecksum = Commands.hasChecksum(receivedBuf);
- int checksum = Commands.readChecksum(receivedBuf).intValue();
+ int checksum = Commands.readChecksum(receivedBuf);
// verify checksum is present
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
