Answer the types of an Area from createElement() where it declares them all

Most Areas declare every one of their types at Area level, and their
services declare none. The switch over the service number then had one
branch that led anywhere, and a method for each service that only ever
returned nothing.

Where no service of an Area declares a type, createElement() now holds
the switch over the type numbers itself. The MAL reaches its jump table
straight away rather than through a call that HotSpot would not have
inlined, the switch over the service numbers is gone from twelve of the
seventeen generated factories, and about forty methods that returned
nothing are no longer written at all. Areas whose services do declare
types are written as before.

The factories are registered while the messages of the ones already
registered are being decoded, so the list they are held in is now one
that can be read through at the same time. Which factory answers when
two of them claim a type is written down: the first registered.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: CesarCoelho

api-generator/generator-java/src/main/java/esa/mo/tools/stubgen/java/JavaElementFactory.java

@@ -37,6 +37,7 @@
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -188,35 +189,87 @@ static int widestJumpTableBand(Collection<Integer> numbers) {
      */
     private void addTypeSwitch(ClassWriter file, String methodName,
             List<String[]> types, String comment) throws IOException {
+        TypeSplit split = splitTypes(types);
+
+        // Splitting the types over two methods only pays off when there is a
+        // jump table to keep and something that would otherwise stretch it.
+        if (!split.isSplit()) {
+            addSwitchMethod(file, methodName, comment, split.all(), null);
+            return;
+        }
+
+        addSwitchMethod(file, methodName, comment, split.inBand, outOfBandNameOf(methodName));
+        addOutOfBandMethod(file, methodName, split);
+    }
+
+    /**
+     * The types of one switch, split into the ones a jump table can hold and
+     * the ones whose numbers lie too far out for it.
+     */
+    private static final class TypeSplit {
+
+        private final Map<Integer, String> inBand = new TreeMap<>();
+
+        private final Map<Integer, String> outOfBand = new TreeMap<>();
+
+        /**
+         * @return True if the types have to be reached by two switches.
+         */
+        boolean isSplit() {
+            return !inBand.isEmpty() && !outOfBand.isEmpty();
+        }
+
+        /**
+         * @return Every type, whether or not the jump table can hold it.
+         */
+        Map<Integer, String> all() {
+            Map<Integer, String> every = new TreeMap<>(inBand);
+            every.putAll(outOfBand);
+            return every;
+        }
+    }
+
+    /**
+     * Splits the types into the ones the widest jump table can hold and the
+     * ones that lie past it.
+     *
+     * @param types The types, as pairs of type number and the expression that
+     * creates one.
+     * @return The split.
+     */
+    private static TypeSplit splitTypes(List<String[]> types) {
         Map<Integer, String> byNumber = new TreeMap<>();
 
         for (String[] type : types) {
             byNumber.put(Integer.valueOf(type[0]), type[1]);
         }
 
         int band = widestJumpTableBand(byNumber.keySet());
-        Map<Integer, String> inBand = new TreeMap<>();
-        Map<Integer, String> outOfBand = new TreeMap<>();
+        TypeSplit split = new TypeSplit();
 
         for (Map.Entry<Integer, String> entry : byNumber.entrySet()) {
             boolean fits = Math.abs(entry.getKey()) <= band;
-            (fits ? inBand : outOfBand).put(entry.getKey(), entry.getValue());
+            (fits ? split.inBand : split.outOfBand).put(entry.getKey(), entry.getValue());
         }
 
-        // Splitting the types over two methods only pays off when there is a
-        // jump table to keep and something that would otherwise stretch it.
-        if (inBand.isEmpty() || outOfBand.isEmpty()) {
-            addSwitchMethod(file, methodName, comment, byNumber, null);
-            return;
-        }
+        return split;
+    }
 
-        String outside = methodName + "OutOfBand";
-        addSwitchMethod(file, methodName, comment, inBand, outside);
-        addSwitchMethod(file, outside, "Creates an Element whose type number lies too"
-                + " far out to be held in the jump table of " + methodName + "()."
-                + " This says nothing about how often the type is asked for: the"
-                + " numbers of an Area are not handed out in the order of use.",
-                outOfBand, null);
+    private static String outOfBandNameOf(String methodName) {
+        return methodName + "OutOfBand";
+    }
+
+    /**
+     * Writes the method that answers for the types the jump table could not
+     * hold.
+     */
+    private void addOutOfBandMethod(ClassWriter file, String methodName,
+            TypeSplit split) throws IOException {
+        addSwitchMethod(file, outOfBandNameOf(methodName),
+                "Creates an Element whose type number lies too far out to be held"
+                + " in the jump table that is asked first. This says nothing about"
+                + " how often the type is asked for: the numbers of an Area are not"
+                + " handed out in the order of use.", split.outOfBand, null);
     }
 
     /**
@@ -290,11 +343,26 @@ private void addSwitchMethod(ClassWriter file, String methodName, String comment
         MethodWriter method = file.addMethodOpenStatement(false, true, StdStrings.PRIVATE,
                 rtype, methodName, Arrays.asList(arg), null, comment, null, null, false);
 
+        addSwitchBody(method, types, fallback);
+        method.addMethodCloseStatement();
+    }
+
+    /**
+     * Writes the body that answers a type number, into a method that is already
+     * open. The body is one switch, or a switch for each side of zero where the
+     * numbers lie too far out to be held together.
+     *
+     * @param method The method to write the body to.
+     * @param types The types the body answers for, by type number.
+     * @param fallback The method the body falls back on for a type number it
+     * does not answer for, or null to answer with nothing.
+     */
+    private void addSwitchBody(MethodWriter method, Map<Integer, String> types,
+            String fallback) throws IOException {
         String missing = (fallback == null) ? "null" : fallback + "(typeNumber)";
 
         if (types.isEmpty()) {
             method.addLine("return " + missing + ";");
-            method.addMethodCloseStatement();
             return;
         }
 
@@ -314,12 +382,10 @@ private void addSwitchMethod(ClassWriter file, String methodName, String comment
             method.addLine("}");
             method.addLine("");
             addSwitchLines(method, negatives, missing, "");
-            method.addMethodCloseStatement();
             return;
         }
 
         addSwitchLines(method, types, missing, "");
-        method.addMethodCloseStatement();
     }
 
     /**
@@ -383,18 +449,46 @@ public void createAreaElementFactoryClass(File areaFolder, AreaType area) throws
         CompositeField argType = generator.createCompositeElementsDetails(file, false, "typeNumber",
                 TypeUtils.createTypeReference(null, null, "int", false), false, false, null);
 
+        // The types of each service, gathered up front: an Area whose services
+        // declare none of their own is reached without a switch over them.
+        Map<ServiceType, List<String[]>> typesByService = new LinkedHashMap<>();
+        boolean anyServiceHasTypes = false;
+
+        for (ServiceType service : area.getService()) {
+            List<String[]> serviceTypes = (service.getDataTypes() == null) ? new LinkedList<>()
+                    : collectTypes(areaName, service.getName(),
+                            service.getDataTypes().getCompositeOrEnumeration());
+            typesByService.put(service, serviceTypes);
+            anyServiceHasTypes = anyServiceHasTypes || !serviceTypes.isEmpty();
+        }
+
         MethodWriter method = file.addMethodOpenStatementOverride(rtype, "createElement",
                 Arrays.asList(argService, argType), null, false);
-        method.addLine("switch (serviceNumber) {");
-        method.addLine("    case 0: return createAreaElement(typeNumber);");
+        TypeSplit areaSplit = splitTypes(areaTypes);
 
-        for (ServiceType service : area.getService()) {
-            method.addLine("    case " + service.getNumber() + ": return create"
-                    + service.getName() + "Element(typeNumber);");
+        if (anyServiceHasTypes) {
+            method.addLine("switch (serviceNumber) {");
+            method.addLine("    case 0: return createAreaElement(typeNumber);");
+
+            for (ServiceType service : area.getService()) {
+                method.addLine("    case " + service.getNumber() + ": return create"
+                        + service.getName() + "Element(typeNumber);");
+            }
+
+            method.addLine("    default: return null;");
+            method.addLine("}");
+        } else {
+            // Every type of this Area is declared by the Area itself, so the
+            // types are answered here rather than through a switch over
+            // services that would only ever lead back to the one branch.
+            method.addLine("if (serviceNumber != 0) {");
+            method.addLine("    return null; // This Area declares no types under a service");
+            method.addLine("}");
+            method.addLine("");
+            addSwitchBody(method, areaSplit.inBand,
+                    areaSplit.isSplit() ? outOfBandNameOf("createAreaElement") : null);
         }
 
-        method.addLine("    default: return null;");
-        method.addLine("}");
         method.addMethodCloseStatement();
 
         // The factory says which Area it belongs to, so that registering it
@@ -412,15 +506,19 @@ public void createAreaElementFactoryClass(File areaFolder, AreaType area) throws
         areaVersion.addLine("return " + area.getVersion() + ";");
         areaVersion.addMethodCloseStatement();
 
-        addTypeSwitch(file, "createAreaElement", areaTypes,
-                "Creates an Element declared by the area itself.");
+        if (anyServiceHasTypes) {
+            addTypeSwitch(file, "createAreaElement", areaTypes,
+                    "Creates an Element declared by the area itself.");
 
-        for (ServiceType service : area.getService()) {
-            List<String[]> serviceTypes = (service.getDataTypes() == null) ? new LinkedList<>()
-                    : collectTypes(areaName, service.getName(),
-                            service.getDataTypes().getCompositeOrEnumeration());
-            addTypeSwitch(file, "create" + service.getName() + "Element", serviceTypes,
-                    "Creates an Element declared by the " + service.getName() + " service.");
+            for (Map.Entry<ServiceType, List<String[]>> entry : typesByService.entrySet()) {
+                addTypeSwitch(file, "create" + entry.getKey().getName() + "Element",
+                        entry.getValue(), "Creates an Element declared by the "
+                        + entry.getKey().getName() + " service.");
+            }
+        } else if (areaSplit.isSplit()) {
+            // createElement() holds the jump table itself, so only the types
+            // that did not fit in it are left to write out
+            addOutOfBandMethod(file, "createAreaElement", areaSplit);
         }
 
         file.addClassCloseStatement();

apis/api-area001-v003-mal/src/main/java/org/ccsds/moims/mo/mal/MALElementsRegistry.java

@@ -21,6 +21,7 @@
 package org.ccsds.moims.mo.mal;
 
 import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import org.ccsds.moims.mo.mal.structures.Element;
@@ -42,15 +43,22 @@ public class MALElementsRegistry {
      * says which Area it belongs to, so nothing here can register a factory
      * against the wrong number or version.
      */
-    private final List<AreaElementFactory> AREA_FACTORIES = new java.util.ArrayList<>();
+    private final List<AreaElementFactory> AREA_FACTORIES = new CopyOnWriteArrayList<>();
 
     /**
      * Registers a factory, so that the Elements it creates can be reached
      * without every one of their classes having to be loaded first.
      *
      * More than one factory can be registered for the same Area. Types whose
      * numbers the XML schema cannot express are written by hand, and their
-     * factory is registered alongside the generated one of that Area.
+     * factory is registered alongside the generated one of that Area. Should
+     * two of them answer for the same type, the one registered first is the one
+     * that is asked, so a factory added later cannot take a type away from the
+     * one that already had it.
+     *
+     * Areas are registered while the messages of the ones already registered
+     * are being decoded, so the list this adds to is one that can be read
+     * through at the same time.
      *
      * @param factory The factory that creates the Elements.
      */
@@ -84,9 +92,10 @@ private Element createFromAreaFactory(final long typeId) {
         final int serviceNumber = TypeId.serviceNumberOf(typeId);
         final int typeNumber = TypeId.typeNumberOf(typeId);
 
-        for (int i = 0; i < AREA_FACTORIES.size(); i++) {
-            AreaElementFactory factory = AREA_FACTORIES.get(i);
-
+        // Walked in one go, over the factories that were there when this
+        // started: a factory registered halfway through must not be seen for
+        // some of this scan and not for the rest of it.
+        for (AreaElementFactory factory : AREA_FACTORIES) {
             if (factory.getAreaNumber() == areaNumber && factory.getAreaVersion() == areaVersion) {
                 Element element = factory.createElement(serviceNumber, typeNumber);