Skip to content

Commit 7a387db

Browse files
CesarCoelhoclaude
andcommitted
Create the Elements of an Area on demand instead of holding one of each
Every Area held a live instance of each of its types, only so that createElement() could be called on it to make a copy. That meant the class of every type was loaded as soon as the Area was, whether or not a deployment ever exchanged that type. The generator now writes an ElementFactory for each Area, which switches on the service number and then on the type number, so a class is loaded only once a message carries that type. The element arrays of the Areas and Services are left empty. The type numbers of an Area are handed out from 1 upwards, so the switch takes the widest band that starts at 1 and still compiles to a jump table. What lies past that band goes to a second method. Where those numbers lie far from zero, each side of zero is switched on its own: together they would have to span the whole way across zero and would fall back to a binary search. On a Raspberry Pi the Areas now load in 411 ms rather than 1199 ms. MALElementsRegistry no longer keeps a map of live Elements, so addElement(), removeCallableElement() and howMany() are gone with it. More than one factory can be registered for the same Area, which is how the MAL/SPP testbed reaches its two hand written enumerations: their type numbers are past what the XML schema allows, so no generated factory can know about them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 7840a17 commit 7a387db

13 files changed

Lines changed: 1155 additions & 67 deletions

File tree

api-generator/generator-java/pom.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,11 @@
8080
<groupId>com.sun.xml.bind</groupId>
8181
<artifactId>jaxb-impl</artifactId>
8282
</dependency>
83+
<dependency>
84+
<groupId>junit</groupId>
85+
<artifactId>junit</artifactId>
86+
<scope>test</scope>
87+
</dependency>
8388
</dependencies>
8489

8590
<build>

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
package esa.mo.tools.stubgen;
2222

2323
import esa.mo.tools.stubgen.java.JavaCompositeClass;
24+
import esa.mo.tools.stubgen.java.JavaElementFactory;
2425
import esa.mo.tools.stubgen.java.JavaServiceInfo;
2526
import esa.mo.tools.stubgen.java.JavaExceptions;
2627
import esa.mo.tools.stubgen.java.JavaConsumer;
@@ -354,6 +355,10 @@ public void run() {
354355
logger.info(" > Creating Area Helper class: " + area.getName());
355356
helper.createAreaHelperClass(areaFolder, area);
356357

358+
logger.info(" > Creating Area Element Factory class: " + area.getName());
359+
JavaElementFactory elementFactory = new JavaElementFactory(this, typeInformation);
360+
elementFactory.createAreaElementFactoryClass(areaFolder, area);
361+
357362
// Create Area Exceptions
358363
JavaExceptions exceptions = new JavaExceptions(this);
359364
logger.info(" > Creating Area Exceptions for area: " + area.getName());

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

Lines changed: 429 additions & 0 deletions
Large diffs are not rendered by default.

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,10 +172,10 @@ public void createAreaHelperClass(File areaFolder, AreaType area) throws IOExcep
172172
}
173173
}
174174

175+
// Left empty on purpose: the Area factory creates these on demand, so
176+
// that the class of a type is only loaded once a message carries it.
175177
StringBuilder buf = new StringBuilder();
176-
for (String objectCall : elementList) {
177-
buf.append("\n ").append(objectCall).append(",");
178-
}
178+
elementList.clear();
179179
CompositeField objectInstVar = generator.createCompositeElementsDetails(file, false, areaNameCAPS + "_AREA_ELEMENTS",
180180
TypeUtils.createTypeReference(null, null, "org.ccsds.moims.mo.mal.structures.Element", false),
181181
false, true, "Area Elements.");
@@ -196,6 +196,8 @@ public void createAreaHelperClass(File areaFolder, AreaType area) throws IOExcep
196196
false, true, buf_2.toString(), false);
197197

198198
String areaObjectInitialValue = createAreaHelperClassInitialValue(areaNameCAPS, area.getVersion());
199+
areaObjectInitialValue = areaObjectInitialValue.substring(0, areaObjectInitialValue.length() - 1)
200+
+ ", new " + areaName + "ElementFactory())";
199201
file.addClassVariable(true, true, StdStrings.PUBLIC, areaVar, true, areaObjectInitialValue);
200202

201203
// create error numbers
@@ -221,6 +223,7 @@ public void createAreaHelperClass(File areaFolder, AreaType area) throws IOExcep
221223
file.flush();
222224
}
223225

226+
224227
public String createAreaHelperClassInitialValue(String areaVar, short areaVersion) {
225228
return "(" + areaVar + "_AREA_NUMBER, " + areaVar + "_AREA_NAME, "
226229
+ areaVar + "_AREA_VERSION, " + areaVar + "_AREA_ELEMENTS, " + areaVar + "_AREA_SERVICES)";

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,10 +184,10 @@ public void createServiceInfoClass(File serviceFolder, AreaType area,
184184
}
185185
}
186186

187+
// Left empty on purpose: the Area factory creates these on demand, so
188+
// that the class of a type is only loaded once a message carries it.
187189
StringBuilder buf = new StringBuilder();
188-
for (String objectCall : elementInstantiations) {
189-
buf.append("\n ").append(objectCall).append(",");
190-
}
190+
elementInstantiations.clear();
191191
CompositeField objectInstVar = generator.createCompositeElementsDetails(file, false, serviceCAPS + "_SERVICE_ELEMENTS",
192192
TypeUtils.createTypeReference(null, null, "org.ccsds.moims.mo.mal.structures.Element", false),
193193
false, true, "Area elements.");
Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
1+
/* ----------------------------------------------------------------------------
2+
* Copyright (C) 2024 European Space Agency
3+
* European Space Operations Centre
4+
* Darmstadt
5+
* Germany
6+
* ----------------------------------------------------------------------------
7+
* System : CCSDS MO Service Stub Generator
8+
* ----------------------------------------------------------------------------
9+
* Licensed under the European Space Agency Public License, Version 2.0
10+
* You may not use this file except in compliance with the License.
11+
*
12+
* Except as expressly set forth in this License, the Software is provided to
13+
* You on an "as is" basis and without warranties of any kind, including without
14+
* limitation merchantability, fitness for a particular purpose, absence of
15+
* defects or errors, accuracy or non-infringement of intellectual property rights.
16+
*
17+
* See the License for the specific language governing permissions and
18+
* limitations under the License.
19+
* ----------------------------------------------------------------------------
20+
*/
21+
package esa.mo.tools.stubgen.java;
22+
23+
import java.util.ArrayList;
24+
import java.util.Collection;
25+
import java.util.List;
26+
import org.junit.Test;
27+
import static org.junit.Assert.assertEquals;
28+
import static org.junit.Assert.assertFalse;
29+
import static org.junit.Assert.assertTrue;
30+
31+
/**
32+
* Tests the rule that decides how far the switch of an area factory reaches
33+
* before the rest of the type numbers are moved out to a second method.
34+
*/
35+
public class TestJavaElementFactory {
36+
37+
/**
38+
* Builds the type numbers of an area the way the generator sees them: a
39+
* type and the list that holds it, which takes the same number negated.
40+
*
41+
* @param magnitudes The declared type numbers.
42+
* @return The type numbers with the numbers of their lists.
43+
*/
44+
private static Collection<Integer> withLists(int... magnitudes) {
45+
List<Integer> numbers = new ArrayList<>();
46+
47+
for (int magnitude : magnitudes) {
48+
numbers.add(magnitude);
49+
numbers.add(-magnitude);
50+
}
51+
52+
return numbers;
53+
}
54+
55+
private static int[] range(int first, int last) {
56+
int[] numbers = new int[last - first + 1];
57+
58+
for (int i = 0; i < numbers.length; i++) {
59+
numbers[i] = first + i;
60+
}
61+
62+
return numbers;
63+
}
64+
65+
/**
66+
* An area whose numbers run without a gap is answered by one switch, so the
67+
* band has to reach the last of them.
68+
*/
69+
@Test
70+
public void bandCoversAnAreaThatHasNoGaps() {
71+
assertEquals(12, JavaElementFactory.widestJumpTableBand(withLists(range(1, 12))));
72+
assertEquals(45, JavaElementFactory.widestJumpTableBand(withLists(range(1, 45))));
73+
}
74+
75+
/**
76+
* The numbers of the MAL run 1 to 19, then 101 to 105, then 1001 to 1010.
77+
* The first two groups are close enough to be held together, the last one
78+
* is not, so it is left for the second method.
79+
*/
80+
@Test
81+
public void bandStopsBeforeAGapThatIsTooWideToPayFor() {
82+
List<Integer> mal = new ArrayList<>();
83+
mal.addAll(withLists(range(1, 19)));
84+
mal.addAll(withLists(range(101, 105)));
85+
mal.addAll(withLists(range(1001, 1010)));
86+
87+
assertEquals(105, JavaElementFactory.widestJumpTableBand(mal));
88+
}
89+
90+
/**
91+
* A group of numbers that lies past a gap is still taken in when it is
92+
* dense enough to pay for the gap in front of it. Stopping at the first
93+
* candidate that fails would leave it out.
94+
*/
95+
@Test
96+
public void bandReachesPastAGapThatTheNumbersBehindItPayFor() {
97+
List<Integer> numbers = new ArrayList<>();
98+
numbers.addAll(withLists(1, 2, 3));
99+
numbers.addAll(withLists(range(20, 60)));
100+
101+
assertEquals(60, JavaElementFactory.widestJumpTableBand(numbers));
102+
}
103+
104+
/**
105+
* The band is counted out from 1, not placed on the longest run of numbers
106+
* that follow one another. An area that starts with a gap would otherwise
107+
* leave its first types out of the jump table.
108+
*/
109+
@Test
110+
public void bandIsAnchoredAtOneAndNotOnTheLongestRun() {
111+
// The run 10 to 15 is the longest, but 1 and 6 come first
112+
List<Integer> numbers = new ArrayList<>();
113+
numbers.addAll(withLists(1, 6));
114+
numbers.addAll(withLists(range(10, 15)));
115+
116+
int band = JavaElementFactory.widestJumpTableBand(numbers);
117+
assertEquals(15, band);
118+
assertTrue("Type number 1 has to be inside the band", band >= 1);
119+
}
120+
121+
/**
122+
* A single type is answered by a lookup rather than by a jump table, so
123+
* there is no band and the types are left in one switch.
124+
*/
125+
@Test
126+
public void aSingleTypeHasNoBand() {
127+
assertEquals(0, JavaElementFactory.widestJumpTableBand(withLists(1)));
128+
}
129+
130+
/**
131+
* A number that lies on its own far from the rest cannot widen the band,
132+
* whatever else the area declares.
133+
*/
134+
@Test
135+
public void aLoneNumberFarOutIsLeftOutOfTheBand() {
136+
List<Integer> numbers = new ArrayList<>();
137+
numbers.addAll(withLists(range(1, 20)));
138+
numbers.addAll(withLists(5000));
139+
140+
assertEquals(20, JavaElementFactory.widestJumpTableBand(numbers));
141+
}
142+
143+
/**
144+
* The band that is chosen has to be one the Java compiler really answers
145+
* with a jump table, and the one past it must not be.
146+
*/
147+
@Test
148+
public void chosenBandIsTheWidestThatStillCompilesToAJumpTable() {
149+
List<Integer> numbers = new ArrayList<>();
150+
numbers.addAll(withLists(range(1, 19)));
151+
numbers.addAll(withLists(range(101, 105)));
152+
numbers.addAll(withLists(range(1001, 1010)));
153+
154+
int band = JavaElementFactory.widestJumpTableBand(numbers);
155+
int inside = 0;
156+
157+
for (Integer number : numbers) {
158+
if (Math.abs(number) <= band) {
159+
inside++;
160+
}
161+
}
162+
163+
assertTrue("The chosen band is not a jump table",
164+
JavaElementFactory.compilesToJumpTable(inside, 2L * band + 1L));
165+
assertTrue("The whole area would still be a jump table",
166+
!JavaElementFactory.compilesToJumpTable(numbers.size(), 2L * 1010 + 1L));
167+
}
168+
169+
/**
170+
* Nothing to switch over is answered with no band, rather than with an
171+
* empty one that the generator would then try to split.
172+
*/
173+
@Test
174+
public void noTypesGiveNoBand() {
175+
assertEquals(0, JavaElementFactory.widestJumpTableBand(new ArrayList<>()));
176+
}
177+
178+
/**
179+
* Numbers that lie far from zero leave the whole way across zero empty
180+
* between the types and their lists, so each side is switched on its own.
181+
*/
182+
@Test
183+
public void numbersFarFromZeroAreSwitchedOneSideAtATime() {
184+
assertTrue(JavaElementFactory.shouldSplitOnSign(withLists(range(1001, 1010))));
185+
}
186+
187+
/**
188+
* Numbers that already answer with a jump table gain nothing from being
189+
* taken apart, and would only pay for the test that reaches each side.
190+
*/
191+
@Test
192+
public void numbersThatAreAlreadyAJumpTableAreLeftWhole() {
193+
assertFalse(JavaElementFactory.shouldSplitOnSign(withLists(range(1, 19))));
194+
assertFalse(JavaElementFactory.shouldSplitOnSign(withLists(range(1, 105))));
195+
}
196+
197+
/**
198+
* A single type far from zero is reached faster by one lookup over both of
199+
* its numbers than by a test and a jump table holding one entry each.
200+
*/
201+
@Test
202+
public void aSingleTypeFarFromZeroIsLeftWhole() {
203+
assertFalse(JavaElementFactory.shouldSplitOnSign(withLists(90)));
204+
}
205+
206+
/**
207+
* There is nothing to take apart when the numbers all fall on one side of
208+
* zero.
209+
*/
210+
@Test
211+
public void numbersOnOneSideOfZeroAreLeftWhole() {
212+
List<Integer> positives = new ArrayList<>();
213+
214+
for (int number = 1001; number <= 1010; number++) {
215+
positives.add(number);
216+
}
217+
218+
assertFalse(JavaElementFactory.shouldSplitOnSign(positives));
219+
assertFalse(JavaElementFactory.shouldSplitOnSign(new ArrayList<>()));
220+
}
221+
222+
/**
223+
* Both sides have to be worth a jump table on their own. One side that
224+
* stays a binary search is not worth the test that reaches it.
225+
*/
226+
@Test
227+
public void bothSidesHaveToBeWorthAJumpTable() {
228+
// The lists follow one another, the types themselves do not
229+
List<Integer> lopsided = new ArrayList<>();
230+
231+
for (int number = 1001; number <= 1010; number++) {
232+
lopsided.add(-number);
233+
}
234+
235+
lopsided.add(1001);
236+
lopsided.add(3000);
237+
lopsided.add(9000);
238+
239+
assertFalse(JavaElementFactory.shouldSplitOnSign(lopsided));
240+
}
241+
}
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/* ----------------------------------------------------------------------------
2+
* Copyright (C) 2024 European Space Agency
3+
* European Space Operations Centre
4+
* Darmstadt
5+
* Germany
6+
* ----------------------------------------------------------------------------
7+
* System : CCSDS MO MAL Java API
8+
* ----------------------------------------------------------------------------
9+
* Licensed under the European Space Agency Public License, Version 2.0
10+
* You may not use this file except in compliance with the License.
11+
*
12+
* Except as expressly set forth in this License, the Software is provided to
13+
* You on an "as is" basis and without warranties of any kind, including without
14+
* limitation merchantability, fitness for a particular purpose, absence of
15+
* defects or errors, accuracy or non-infringement of intellectual property rights.
16+
*
17+
* See the License for the specific language governing permissions and
18+
* limitations under the License.
19+
* ----------------------------------------------------------------------------
20+
*/
21+
package org.ccsds.moims.mo.mal;
22+
23+
import org.ccsds.moims.mo.mal.structures.Element;
24+
25+
/**
26+
* Creates the Elements of one Area.
27+
*
28+
* One of these is generated for each Area, and answers with a new Element for a
29+
* service number and a type number of that Area. The generated implementation
30+
* is a switch on the service number, and then a switch on the type number, so
31+
* that the class of a type is only ever loaded when a message actually carries
32+
* that type. Holding an instance of every type instead, only to be able to ask
33+
* it for a copy of itself, made every one of them load at start up.
34+
*/
35+
public interface AreaElementFactory {
36+
37+
/**
38+
* Returns a new Element of the given type, or null if this Area does not
39+
* define it.
40+
*
41+
* @param serviceNumber The service the type belongs to, or 0 for the types
42+
* defined by the Area itself.
43+
* @param typeNumber The type number, negative for the list of a type.
44+
* @return A new Element, or null if this Area does not define that type.
45+
*/
46+
Element createElement(int serviceNumber, int typeNumber);
47+
48+
/**
49+
* Returns the number of the Area this factory creates the Elements of.
50+
*
51+
* @return The Area number.
52+
*/
53+
int getAreaNumber();
54+
55+
/**
56+
* Returns the version of the Area this factory creates the Elements of.
57+
*
58+
* @return The Area version.
59+
*/
60+
int getAreaVersion();
61+
}

0 commit comments

Comments
 (0)