tabish121 commented on code in PR #6323:
URL: https://github.com/apache/artemis/pull/6323#discussion_r3024474441
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java:
##########
@@ -119,6 +118,12 @@
*/
public abstract class AMQPMessage extends RefCountMessage implements
org.apache.activemq.artemis.api.core.Message {
+ // The basic (minimal) size an AMQP message uses.
+ // This is an estimate, and it's based on the following test:
+ // By running AMQPGlobalMaxTest::testSendUntilOME, you look at the initial
memory used by the broker without any messages.
+ // By the time you get the OME, you can do some bare calculations on how
much each message uses and get an AVG.
+ public static final int AMQP_OFFSET = 1300;
Review Comment:
The name here isn't very helpful in knowing what this is supposed to
represent, something MINIMUM_SIZE_ESTIMATE or the like could make reading the
other bits of the code easier, took me a bit to realize this isn't an offset
when I started reviewing the changes.
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java:
##########
@@ -546,36 +550,26 @@ protected ApplicationProperties
lazyDecodeApplicationProperties() {
// need to synchronize access to lazyDecodeApplicationProperties to avoid
clashes with getMemoryEstimate
protected synchronized ApplicationProperties
lazyDecodeApplicationProperties(ReadableBuffer data) {
if (applicationProperties == null && applicationPropertiesPosition !=
VALUE_NOT_PRESENT) {
- applicationProperties = scanForMessageSection(data,
applicationPropertiesPosition, ApplicationProperties.class);
- if (owner != null && memoryEstimate != -1) {
- // the memory has already been tracked and needs to be updated to
reflect the new decoding
- int addition =
unmarshalledApplicationPropertiesMemoryEstimateFromData(data);
-
- // it is difficult to track the updates for paged messages
- // for that reason we won't do it if paged
- // we also only do the update if the message was previously routed
- // so if a debug method or an interceptor changed the size before
routing we would get a different size
- if (!isPaged && routed) {
- ((PagingStore) owner).addSize(addition, false);
- final int updatedEstimate = memoryEstimate + addition;
- memoryEstimate = updatedEstimate;
- }
- }
+ readApplicationProperties(data, applicationPropertiesPosition);
}
return applicationProperties;
}
+ protected ApplicationProperties readApplicationProperties(ReadableBuffer
data, int position) {
+ applicationProperties = scanForMessageSection(data, position,
ApplicationProperties.class);
+ return applicationProperties;
+ }
+
protected int
unmarshalledApplicationPropertiesMemoryEstimateFromData(ReadableBuffer data) {
- if (applicationProperties != null) {
- // they have been unmarshalled, estimate memory usage based on their
encoded size
- if (remainingBodyPosition != VALUE_NOT_PRESENT) {
- return remainingBodyPosition - applicationPropertiesPosition;
- } else {
- return data.capacity() - applicationPropertiesPosition;
- }
+ ensureScanning();
+
+ // they have been unmarshalled, estimate memory usage based on their
encoded size
Review Comment:
This comment is now incorrect as they may not have been unmarshaled but have
been located and their size has been determined.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
