[
https://issues.apache.org/jira/browse/ARTEMIS-2170?focusedWorklogId=187820&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-187820
]
ASF GitHub Bot logged work on ARTEMIS-2170:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 21/Jan/19 15:44
Start Date: 21/Jan/19 15:44
Worklog Time Spent: 10m
Work Description: michaelandrepearce commented on 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:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 187820)
Time Spent: 1h 40m (was: 1.5h)
> Optimized CoreMessage's checkProperties and cleanupInternalProperties methods
> -----------------------------------------------------------------------------
>
> Key: ARTEMIS-2170
> URL: https://issues.apache.org/jira/browse/ARTEMIS-2170
> Project: ActiveMQ Artemis
> Issue Type: Improvement
> Components: Broker
> Reporter: Francesco Nigro
> Assignee: Francesco Nigro
> Priority: Minor
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> CoreMessage::checkProperties perform too many volatile read/write and has a
> too big body just to handle exceptional cases, while
> cleanupInternalProperties is called on the hot path of session send, but is
> performing too many synchronized operations and loopup on TypedProperties.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
