michaelandrepearce commented on a change in pull request #2427: ARTEMIS-2170
Optimized CoreMessage's checkProperties and cleanupInternalProperties methods
URL: https://github.com/apache/activemq-artemis/pull/2427#discussion_r249497584
##########
File path:
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java
##########
@@ -579,36 +564,45 @@ public CoreMessage setUserID(UUID userID) {
/**
* I am keeping this synchronized as the decode of the Properties is lazy
*/
- protected TypedProperties checkProperties() {
+ public final TypedProperties getProperties() {
try {
+ TypedProperties properties = this.properties;
if (properties == null) {
- synchronized (this) {
- if (properties == null) {
- TypedProperties properties = new TypedProperties();
- if (buffer != null && propertiesLocation >= 0) {
- final ByteBuf byteBuf =
buffer.duplicate().readerIndex(propertiesLocation);
- properties.decode(byteBuf, coreMessageObjectPools == null
? null : coreMessageObjectPools.getPropertiesDecoderPools());
- }
- this.properties = properties;
- }
- }
+ properties = getOrInitializeTypedProperties();
}
-
- return this.properties;
+ return properties;
} catch (Throwable e) {
- // This is not an expected error, hence no specific logger created
- logger.warn("Could not decode properties for CoreMessage[messageID="
+ messageID + ",durable=" + durable + ",userID=" + userID + ",priority=" +
priority +
- ", timestamp=" + timestamp + ",expiration=" + expiration +
",address=" + address + ", propertiesLocation=" + propertiesLocation, e);
- if (buffer != null) {
- ByteBuf duplicatebuffer = buffer.duplicate();
- duplicatebuffer.readerIndex(0);
- logger.warn("Failed message has messageID=" + messageID + " and
the following buffer:\n" + ByteBufUtil.prettyHexDump(duplicatebuffer));
- } else {
- logger.warn("Failed message has messageID=" + messageID + " and
the buffer was null");
- }
- throw new RuntimeException(e.getMessage(), e);
+ throw onCheckPropertiesError(e);
+ }
+ }
+ private synchronized TypedProperties getOrInitializeTypedProperties() {
+ TypedProperties properties = this.properties;
+ if (properties == null) {
+ properties = new TypedProperties();
+ if (buffer != null && propertiesLocation >= 0) {
+ final ByteBuf byteBuf =
buffer.duplicate().readerIndex(propertiesLocation);
+ properties.decode(byteBuf, coreMessageObjectPools == null ? null :
coreMessageObjectPools.getPropertiesDecoderPools());
+ }
+ this.properties = properties;
+ }
+ return properties;
+ }
+
+ private RuntimeException onCheckPropertiesError(Throwable e) {
+ // This is not an expected error, hence no specific logger created
+ logger.warn("Could not decode properties for CoreMessage[messageID=" +
messageID + ",durable=" + durable + ",userID=" + userID + ",priority=" +
priority +
+ ", timestamp=" + timestamp + ",expiration=" + expiration
+ ",address=" + address + ", propertiesLocation=" + propertiesLocation, e);
+ final ByteBuf buffer = this.buffer;
+ if (buffer != null) {
+ //risky: a racy modification to buffer indexes could break this
duplicate operation
Review comment:
the try catch, where on exception we call, onCheckedProperties. Agree this
could be moved to getOrInitializeTypedProperties, i would still leave it though
where we use a local copy reference.
----------------------------------------------------------------
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
