gemmellr commented on code in PR #6323:
URL: https://github.com/apache/artemis/pull/6323#discussion_r3209357090


##########
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/MapCodec.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.utils.collections;
+
+import java.lang.invoke.MethodHandles;
+import java.util.function.Consumer;
+
+import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.utils.DataConstants;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This class encapsulates the encoding and decoding of a {@code Map<String, 
Object>}.

Review Comment:
   Doesnt seem consistent with the code below.



##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/PacketImpl.java:
##########
@@ -58,6 +58,8 @@ public class PacketImpl implements Packet {
    // 2.37.0
    public static final int ARTEMIS_2_37_0_VERSION = 136;
 
+   public static final int ARTEMIS_2_54_0_VERSION = 137;

Review Comment:
   I do not think this complex disk/wire level change should be targeting 
2.54.0 at this late stage (it was already meant to be out).



##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessageMapCodec.java:
##########
@@ -0,0 +1,204 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.protocol.amqp.broker;
+
+import java.lang.invoke.MethodHandles;
+
+import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
+import org.apache.activemq.artemis.api.core.Message;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.utils.collections.MapCodec;
+import org.apache.activemq.artemis.utils.collections.TypedProperties;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Encodes AMQP messages for storage using a hashmap-like encoding. It is used 
by {@link AMQPLargeMessagePersisterV2} and {@link AMQPMessagePersisterV4}. */
+public class AMQPMessageMapCodec extends MapCodec {
+
+   private static final Logger logger = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+   private static final ThreadLocal<AMQPMessageMapCodec> threadLocal = 
ThreadLocal.withInitial(AMQPMessageMapCodec::new);
+
+   public static AMQPMessageMapCodec getInstance() {
+      AMQPMessageMapCodec reader = threadLocal.get();
+      return reader;
+   }

Review Comment:
   Possibly over-thinking it, but the this got me wondering if rather than a 
subclass, this would work nicer as a method that took a processor / 
callback-handler style argument. The persister would then just need 1 of those 
while it worked during startup and then it would go away when done, unlike 
this. You could also then separately test the codec class (current parent) and 
the processor/callback fully that way rather than e.g creating a test subclass 
to test the parent bits, and not really _directly_ testing this class at all 
[it seems so far].
   
   The large and regular persister could still use the same processor and then 
just toggle a flag to indicate which behaviour to give.



##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/CoreRemotingConnection.java:
##########
@@ -81,6 +81,11 @@ default boolean isBeforeProducerMetricsChanged() {
       return version < PacketImpl.ARTEMIS_2_28_0_VERSION;
    }
 
+   default boolean isBeforeAMQPMapCodecChanged() {

Review Comment:
   'isBeforeAMQPMapCodecChanged' means barely anything to me now even with the 
context of the PR, its going to be incomprehensible the next time almost anyone 
sees it in future. Especially confusing since nothing about AMQP Map encodings 
has changed.
   
   'isBeforeAMQPMessagePersisterV4Change' or something else actually speaking 
to the persistence of AMQP messages, which is whats changing, would seem far 
more clear now and later.



##########
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/MapCodec.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.utils.collections;
+
+import java.lang.invoke.MethodHandles;
+import java.util.function.Consumer;
+
+import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.utils.DataConstants;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This class encapsulates the encoding and decoding of a {@code Map<String, 
Object>}.
+ * It supports encoding of objects of type Boolean, SimpleString, Integer, 
Long, Byte and Byte Array
+ * It provides support to predetermine the size of the encoding, and a 
callback reader for subclasses.
+ */
+public abstract class MapCodec {

Review Comment:
   This seems like an overly generic class name and package for whats 
ultiamtely got a fairly specialized use case. It might invite additions, 
whereas altering this at all in most ways would defeat the point of it being 
added for its intended use case.



##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessageMapCodec.java:
##########
@@ -0,0 +1,204 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.protocol.amqp.broker;
+
+import java.lang.invoke.MethodHandles;
+
+import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
+import org.apache.activemq.artemis.api.core.Message;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.utils.collections.MapCodec;
+import org.apache.activemq.artemis.utils.collections.TypedProperties;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Encodes AMQP messages for storage using a hashmap-like encoding. It is used 
by {@link AMQPLargeMessagePersisterV2} and {@link AMQPMessagePersisterV4}. */
+public class AMQPMessageMapCodec extends MapCodec {

Review Comment:
   Similarly think the name is too generic. This has nothing to do with AMQP 
maps but the name really suggests its closely related. This is specific to the 
MessagePersister stuff so it would be far clearer later if named accordingly.



##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessageMapCodec.java:
##########
@@ -0,0 +1,204 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.protocol.amqp.broker;
+
+import java.lang.invoke.MethodHandles;
+
+import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
+import org.apache.activemq.artemis.api.core.Message;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.utils.collections.MapCodec;
+import org.apache.activemq.artemis.utils.collections.TypedProperties;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Encodes AMQP messages for storage using a hashmap-like encoding. It is used 
by {@link AMQPLargeMessagePersisterV2} and {@link AMQPMessagePersisterV4}. */
+public class AMQPMessageMapCodec extends MapCodec {
+
+   private static final Logger logger = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+   private static final ThreadLocal<AMQPMessageMapCodec> threadLocal = 
ThreadLocal.withInitial(AMQPMessageMapCodec::new);
+
+   public static AMQPMessageMapCodec getInstance() {
+      AMQPMessageMapCodec reader = threadLocal.get();
+      return reader;
+   }
+
+   protected static final int NUMBER_OF_LONGS = 3;
+   protected static final int NUMBER_OF_INTEGERS = 1;
+   protected static final int NUMBER_OF_BYTES = 1;
+   protected static final int NUMBER_OF_BOOLEANS = 1;
+   protected static final int NUMBER_OF_STRINGS = 1;
+   protected static final int NUMBER_OF_BOOLEANS_LARGE_MESSAGE = 1; // large 
message has an additional boolean being used
+
+   protected static final short KEY_MESSAGE_ID = 1;
+   protected static final short KEY_MESSAGE_FORMAT = 2;
+   protected static final short KEY_ADDRESS = 3;
+   protected static final short KEY_EXPIRATION = 4;
+   protected static final short KEY_MEMORY_ESTIMATE = 5;
+   protected static final short KEY_PRIORITY = 6;
+   protected static final short KEY_IS_DURABLE = 7;
+   protected static final short KEY_EXTRA_PROPERTIES = 8;
+   protected static final short KEY_IS_REENCODED = 9; // used on large 
messages only
+
+   
///////////////////////////////////////////////////////////////////////////////////
+   // only used on reading, on writing we will write directly from the message
+   protected long messageID;
+   protected long messageFormat;
+   protected long messageExpiration;
+   protected SimpleString address;
+   protected int memoryEstimate;
+   protected byte priority;
+   protected boolean isDurable;
+   protected TypedProperties extraProperties;
+   protected boolean isReencoded; // used on large messages only
+   // only used on reading, on writing we will write directly from the message
+   
///////////////////////////////////////////////////////////////////////////////////
+
+   protected int getPayloadSize(Message record) {
+      AMQPMessage amqpMessage = (AMQPMessage) record;
+      int payloadSize = payloadSizeInteger(NUMBER_OF_INTEGERS) +
+         payloadSizeBoolean(NUMBER_OF_BOOLEANS) +
+         payloadSizeLong(NUMBER_OF_LONGS) +
+         payloadSizeByte(NUMBER_OF_BYTES) +
+         payloadSizeSimpleString(record.getAddressSimpleString());
+
+      if (record.isLargeMessage()) {
+         payloadSize += payloadSizeBoolean(1);
+      }
+
+      if (amqpMessage.getExtraProperties() != null) {
+         payloadSize += 
payloadSizeByteArray(amqpMessage.getExtraProperties().getEncodeSize());
+      }
+
+      return payloadSize;
+   }
+
+   public int getEncodeSize(Message record) {
+      return headerSize() + getPayloadSize(record);
+   }
+
+
+   public AMQPMessageMapCodec reset() {
+      messageID = 0;
+      messageFormat = 0;
+      messageExpiration = 0;
+      address = null;
+      memoryEstimate = 0;
+      priority = 0;
+      isDurable = false;
+      isReencoded = false;
+      extraProperties = null;
+      return this;
+   }
+
+   public int getNumberOfElements(Message record) {
+      AMQPMessage amqpMessage = (AMQPMessage) record;
+      int numberOfElements = NUMBER_OF_LONGS + NUMBER_OF_INTEGERS + 
NUMBER_OF_BYTES + NUMBER_OF_BOOLEANS + NUMBER_OF_STRINGS;
+      if (record.isLargeMessage()) {
+         numberOfElements += NUMBER_OF_BOOLEANS_LARGE_MESSAGE;
+      }
+      if (amqpMessage.getExtraProperties() != null) {
+         numberOfElements++;
+      }
+      return numberOfElements;
+   }
+
+   public void encode(ActiveMQBuffer buffer, Message record) {
+      AMQPMessage msgEncode = (AMQPMessage) record;
+      int initialPosition = buffer.writerIndex();
+      int payloadSize = getPayloadSize(record);
+      int mapSize = writeHeader(buffer, payloadSize, 
getNumberOfElements(record));
+
+      encodeElements(buffer, msgEncode);
+
+      assert buffer.writerIndex() == initialPosition + mapSize;
+   }
+
+   protected void encodeElements(ActiveMQBuffer buffer, AMQPMessage msgEncode) 
{
+      writeLong(buffer, KEY_MESSAGE_ID, msgEncode.getMessageID());
+      writeLong(buffer, KEY_MESSAGE_FORMAT, msgEncode.getMessageFormat());
+      writeSimpleString(buffer, KEY_ADDRESS, 
msgEncode.getAddressSimpleString());
+      writeLong(buffer, KEY_EXPIRATION, msgEncode.getExpiration());
+      writeInteger(buffer, KEY_MEMORY_ESTIMATE, msgEncode.getMemoryEstimate());
+      writeByte(buffer, KEY_PRIORITY, msgEncode.getPriority());
+      writeBoolean(buffer, KEY_IS_DURABLE, msgEncode.isDurable());
+      TypedProperties extraProperties = msgEncode.getExtraProperties();
+      if (extraProperties != null) {
+         writeByteArray(buffer, KEY_EXTRA_PROPERTIES, 
extraProperties.getEncodeSize(), extraProperties::encode);
+      }
+      if (msgEncode.isLargeMessage()) {
+         writeBoolean(buffer, KEY_IS_REENCODED, 
((AMQPLargeMessage)msgEncode).isReencoded());
+      }
+   }
+
+   @Override
+   public void onMapReadInteger(short key, int value) {
+      switch (key) {
+         case KEY_MEMORY_ESTIMATE -> memoryEstimate = value;
+         default -> logger.debug("Unknown Integer key={} value={} - ignoring", 
key, value);
+      }
+   }
+
+   @Override
+   public void onMapReadByte(short key, byte value) {
+      switch (key) {
+         case KEY_PRIORITY -> priority = value;
+         default -> logger.debug("Unknown Byte key={} value={} - ignoring", 
key, value);
+      }
+   }
+
+   @Override
+   public void onMapReadBoolean(short key, boolean value) {
+      switch (key) {
+         case KEY_IS_DURABLE -> isDurable = value;
+         case KEY_IS_REENCODED -> isReencoded = value;
+         default -> logger.debug("Unknown Boolean key={} value={} - ignoring", 
key, value);
+      }
+   }
+
+   @Override
+   public void onMapReadLong(short key, long value) {
+      switch (key) {
+         case KEY_MESSAGE_ID -> messageID = value;
+         case KEY_MESSAGE_FORMAT -> messageFormat = value;
+         case KEY_EXPIRATION -> messageExpiration = value;
+         default -> logger.debug("Unknown Long key={} value={} - ignoring", 
key, value);
+      }
+   }
+
+   @Override
+   public void onMapReadSimpleString(short key, SimpleString value) {
+      switch (key) {
+         case KEY_ADDRESS -> address = value;
+         default -> logger.debug("Unknown String key={} value={} - ignoring", 
key, value);
+      }
+   }
+
+   @Override
+   protected void onMapReadByteArray(short key, ActiveMQBuffer slice) {
+      switch (key) {
+         case KEY_EXTRA_PROPERTIES -> {
+            extraProperties = new 
TypedProperties(Message.INTERNAL_PROPERTY_NAMES_PREDICATE);
+            extraProperties.decode(slice.byteBuf());
+         }

Review Comment:
   Missing default (and debug log) is inconsistent with the others.



##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessageMapCodec.java:
##########
@@ -0,0 +1,204 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.protocol.amqp.broker;
+
+import java.lang.invoke.MethodHandles;
+
+import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
+import org.apache.activemq.artemis.api.core.Message;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.utils.collections.MapCodec;
+import org.apache.activemq.artemis.utils.collections.TypedProperties;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Encodes AMQP messages for storage using a hashmap-like encoding. It is used 
by {@link AMQPLargeMessagePersisterV2} and {@link AMQPMessagePersisterV4}. */
+public class AMQPMessageMapCodec extends MapCodec {
+
+   private static final Logger logger = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+   private static final ThreadLocal<AMQPMessageMapCodec> threadLocal = 
ThreadLocal.withInitial(AMQPMessageMapCodec::new);
+
+   public static AMQPMessageMapCodec getInstance() {
+      AMQPMessageMapCodec reader = threadLocal.get();
+      return reader;
+   }
+
+   protected static final int NUMBER_OF_LONGS = 3;
+   protected static final int NUMBER_OF_INTEGERS = 1;
+   protected static final int NUMBER_OF_BYTES = 1;
+   protected static final int NUMBER_OF_BOOLEANS = 1;
+   protected static final int NUMBER_OF_STRINGS = 1;
+   protected static final int NUMBER_OF_BOOLEANS_LARGE_MESSAGE = 1; // large 
message has an additional boolean being used

Review Comment:
   A single constant that just enumerated the size of things being added, 
commenting what each bit is as added, in the order they are encoded/decoded, 
seems like it would be more readable and maintainable later than this separate 
grouping of counts of types and the separate payload size calculation, and 
separate encoding +decoding. The 'payload size foo' methods could go, as its 
just added once as a constant instead of being added up repeatedly on every run 
through.
   
   Its already the case that this system here isnt even used in the tracking 
the size and counts relating to the map(possibly plural at some point) that is 
encoded, or the large message boolean that is separate, so its already 
necessary to inspect multiple places to figure our what is going on (which 
doesnt tell you the size of any given thing, just the group of them) instead of 
just reading a comment on a constant.



##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessageMapCodec.java:
##########
@@ -0,0 +1,204 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.protocol.amqp.broker;
+
+import java.lang.invoke.MethodHandles;
+
+import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
+import org.apache.activemq.artemis.api.core.Message;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.utils.collections.MapCodec;
+import org.apache.activemq.artemis.utils.collections.TypedProperties;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Encodes AMQP messages for storage using a hashmap-like encoding. It is used 
by {@link AMQPLargeMessagePersisterV2} and {@link AMQPMessagePersisterV4}. */
+public class AMQPMessageMapCodec extends MapCodec {
+
+   private static final Logger logger = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+   private static final ThreadLocal<AMQPMessageMapCodec> threadLocal = 
ThreadLocal.withInitial(AMQPMessageMapCodec::new);
+
+   public static AMQPMessageMapCodec getInstance() {
+      AMQPMessageMapCodec reader = threadLocal.get();
+      return reader;
+   }
+
+   protected static final int NUMBER_OF_LONGS = 3;
+   protected static final int NUMBER_OF_INTEGERS = 1;
+   protected static final int NUMBER_OF_BYTES = 1;
+   protected static final int NUMBER_OF_BOOLEANS = 1;
+   protected static final int NUMBER_OF_STRINGS = 1;
+   protected static final int NUMBER_OF_BOOLEANS_LARGE_MESSAGE = 1; // large 
message has an additional boolean being used
+
+   protected static final short KEY_MESSAGE_ID = 1;
+   protected static final short KEY_MESSAGE_FORMAT = 2;
+   protected static final short KEY_ADDRESS = 3;
+   protected static final short KEY_EXPIRATION = 4;
+   protected static final short KEY_MEMORY_ESTIMATE = 5;
+   protected static final short KEY_PRIORITY = 6;
+   protected static final short KEY_IS_DURABLE = 7;
+   protected static final short KEY_EXTRA_PROPERTIES = 8;
+   protected static final short KEY_IS_REENCODED = 9; // used on large 
messages only
+
+   
///////////////////////////////////////////////////////////////////////////////////
+   // only used on reading, on writing we will write directly from the message
+   protected long messageID;
+   protected long messageFormat;
+   protected long messageExpiration;
+   protected SimpleString address;
+   protected int memoryEstimate;
+   protected byte priority;
+   protected boolean isDurable;
+   protected TypedProperties extraProperties;
+   protected boolean isReencoded; // used on large messages only
+   // only used on reading, on writing we will write directly from the message
+   
///////////////////////////////////////////////////////////////////////////////////
+
+   protected int getPayloadSize(Message record) {
+      AMQPMessage amqpMessage = (AMQPMessage) record;
+      int payloadSize = payloadSizeInteger(NUMBER_OF_INTEGERS) +
+         payloadSizeBoolean(NUMBER_OF_BOOLEANS) +
+         payloadSizeLong(NUMBER_OF_LONGS) +
+         payloadSizeByte(NUMBER_OF_BYTES) +
+         payloadSizeSimpleString(record.getAddressSimpleString());
+
+      if (record.isLargeMessage()) {
+         payloadSize += payloadSizeBoolean(1);
+      }
+
+      if (amqpMessage.getExtraProperties() != null) {
+         payloadSize += 
payloadSizeByteArray(amqpMessage.getExtraProperties().getEncodeSize());
+      }
+
+      return payloadSize;
+   }
+
+   public int getEncodeSize(Message record) {
+      return headerSize() + getPayloadSize(record);
+   }
+
+
+   public AMQPMessageMapCodec reset() {
+      messageID = 0;
+      messageFormat = 0;
+      messageExpiration = 0;
+      address = null;
+      memoryEstimate = 0;
+      priority = 0;
+      isDurable = false;
+      isReencoded = false;
+      extraProperties = null;

Review Comment:
   These being in the same order as the fields above, and which they are 
encoded/decoded elsewhere, would be more readable.



-- 
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]

Reply via email to