HY-73 issue #532: bugfix when resetting groups decoder, then old grou…

…p references were being nullified which caused unecessary allocation on new decodings
real-logic · Dec 10, 2024 · 712699c · 712699c
1 parent edb557a
commit 712699c
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 15 deletions.
diff --git a/...o-codecs/src/main/java/uk/co/real_logic/artio/dictionary/generation/DecoderGenerator.java b/...o-codecs/src/main/java/uk/co/real_logic/artio/dictionary/generation/DecoderGenerator.java
@@ -416,28 +416,21 @@ protected String resetGroup(final Entry entry)
                 "    {\n" +
                 "        for (final %2$s %6$s : %5$s.iterator())\n" +
                 "        {\n" +
+                "            %6$s.reset();\n" +
                 "            if (%6$s.next() == null)\n" +
                 "            {\n" +
-                "                %6$s.reset();\n" +
                 "                break;\n" +
                 "            }\n" +
-                "            else\n" +
-                "            {\n" +
-                "                %6$s.reset();\n" +
-                "            }\n" +
                 "        }\n" +
                 "        %3$s = MISSING_INT;\n" +
                 "        has%4$s = false;\n" +
-                "        %7$s = null;\n" +
                 "    }\n\n",
                 resetMethod,
                 decoderClassName(name),
                 formatPropertyName(numberField.name()),
                 numberField.name(),
                 iteratorFieldName(group),
-                formatPropertyName(decoderClassName(name)),
-                formatPropertyName(name)
-            );
+                formatPropertyName(decoderClassName(name)));
         }
     }
 
@@ -491,9 +484,6 @@ private String additionalReset(final boolean isGroup)
     {
         return
             "        buffer = null;\n" +
-              (isGroup ?
-            "        next = null;\n" : ""
-              ) +
             "        if (" + CODEC_VALIDATION_ENABLED + ")\n" +
             "        {\n" +
             "            invalidTagId = Decoder.NO_ERROR;\n" +
@@ -1743,6 +1733,7 @@ private String generateDecodePrefix(
             "        this.buffer = buffer;\n" +
             "        final int end = offset + length;\n" +
             "        int position = offset;\n" +
+            "        int positionIter = position;\n" +
             (hasCommonCompounds ? "        position += header.decode(buffer, position, length);\n" : "") +
             (isGroup ? "        seenFields.clear();\n" : "") +
             "        int tag;\n\n" +
@@ -1921,7 +1912,17 @@ private String decodeGroup(final Entry entry)
             "                {\n" +
             "                    if (%1$sCurrent != null)\n" +
             "                    {\n" +
-            "                        position += %1$sCurrent.decode(buffer, position, end - position);\n" +
+            "                        positionIter = %1$sCurrent.decode(buffer, position, end - position);\n" +
+            "                        if (positionIter == 0 && " + CODEC_VALIDATION_ENABLED + ")\n" +
+            "                        {\n" +
+            "                                invalidTagId = tag;\n" +
+            "                                rejectReason = " + INCORRECT_NUMINGROUP_COUNT_FOR_REPEATING_GROUP + ";\n" +
+            "                                break;\n" +
+            "                        }\n" +
+            "                        else\n" +
+            "                        {\n" +
+            "                                position += positionIter;\n" +
+            "                        }\n" +
             "                        %1$sCurrent = %1$sCurrent.next();\n" +
             "                    }\n" +
             "                }\n" +

diff --git a/.../test/java/uk/co/real_logic/artio/dictionary/generation/AbstractDecoderGeneratorTest.java b/.../test/java/uk/co/real_logic/artio/dictionary/generation/AbstractDecoderGeneratorTest.java
@@ -1179,10 +1179,14 @@ public void shouldResetAllNestedRepeatingGroupEntries() throws Exception
         assertEquals(2, getNoEgGroupGroupCounter(decoder));
 
         group = getEgGroup(decoder);
-        assertNull(getNestedGroup(group));
+
+        // Although the message does not have nestedEg tags, the decoders will remain
+        // This is to ensure allocation is done only when it's necessary to add new elements to a repeating group
+        // for more details see issue https://github.com/real-logic/artio/issues/532
+        assertNotNull(getNestedGroup(group));
 
         group = next(group);
-        assertNull(getNestedGroup(group));
+        assertNotNull(getNestedGroup(group));
     }
 
     @Test