[#645] Parse descriptor annotations eagerly and discard documentation

.gitignore

@@ -23,3 +23,5 @@ out
 *~
 # OS X generated file
 .DS_Store
+# Claude Code
+CLAUDE.md

core/pom.xml

@@ -91,6 +91,12 @@
             <optional>true</optional>
         </dependency>
 
+        <dependency>
+            <groupId>org.openjdk.jol</groupId>
+            <artifactId>jol-core</artifactId>
+            <scope>test</scope>
+        </dependency>
+
         <dependency>
             <groupId>org.jboss.marshalling</groupId>
             <artifactId>jboss-marshalling</artifactId>

core/src/main/java/org/infinispan/protostream/annotations/impl/ProtoMessageTypeMetadata.java

@@ -10,7 +10,8 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
-import java.util.LinkedList;
+
+
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -80,7 +81,7 @@ public class ProtoMessageTypeMetadata extends ProtoTypeMetadata {
 
    private XMethod unknownFieldSetSetter;
 
-   private final Map<XClass, ProtoTypeMetadata> innerTypes = new HashMap<>();
+   private final List<ProtoTypeMetadata> innerTypes = new ArrayList<>();
 
    protected ProtoMessageTypeMetadata(BaseProtoSchemaGenerator protoSchemaGenerator, XClass annotatedClass, XClass javaClass) {
       super(getProtoName(annotatedClass, javaClass), javaClass);
@@ -159,7 +160,7 @@ public SortedMap<Integer, ProtoFieldMetadata> getFields() {
    }
 
    protected void addInnerType(ProtoTypeMetadata typeMetadata) {
-      innerTypes.put(typeMetadata.getJavaClass(), typeMetadata);
+      innerTypes.add(typeMetadata);
    }
 
    @Override
@@ -192,13 +193,13 @@ public void generateProto(IndentWriter iw, ProtoSyntax syntax) {
 
       reserved.generate(iw);
 
-      for (ProtoTypeMetadata t : innerTypes.values()) {
+      for (ProtoTypeMetadata t : innerTypes) {
          t.generateProto(iw, syntax);
       }
 
-      LinkedList<ProtoFieldMetadata> unprocessedFields = new LinkedList<>(fieldsByNumber.values());
+      List<ProtoFieldMetadata> unprocessedFields = new ArrayList<>(fieldsByNumber.values());
       while (!unprocessedFields.isEmpty()) {
-         ProtoFieldMetadata f = unprocessedFields.remove();
+         ProtoFieldMetadata f = unprocessedFields.remove(0);
          if (f.getOneof() == null) {
             f.generateProto(iw, syntax);
          } else {

core/src/main/java/org/infinispan/protostream/descriptors/AnnotatedDescriptor.java

@@ -37,8 +37,11 @@ public interface AnnotatedDescriptor {
    /**
     * Return the documentation text associated with this descriptor.
     *
-    * @return the documentation text or {@code null} if not present
+    * @return always {@code null}, documentation is no longer retained after annotation parsing
+    * @deprecated Documentation is no longer retained. Use {@link #getAnnotations()} or
+    * {@link #getProcessedAnnotation(String)} to access parsed annotation data.
     */
+   @Deprecated(forRemoval = true)
    String getDocumentation();
 
    /**

core/src/main/java/org/infinispan/protostream/descriptors/Descriptor.java

@@ -118,6 +118,11 @@ void setFileDescriptor(FileDescriptor fileDescriptor) {
       for (FieldDescriptor fieldDescriptor : fields) {
          fieldDescriptor.setFileDescriptor(fileDescriptor);
       }
+      for (OneOfDescriptor oneOf : oneofs) {
+         for (FieldDescriptor fieldDescriptor : oneOf.getFields()) {
+            fieldDescriptor.setFileDescriptor(fileDescriptor);
+         }
+      }
       for (Descriptor nested : nestedMessageTypes) {
          nested.setFileDescriptor(fileDescriptor);
       }

core/src/main/java/org/infinispan/protostream/descriptors/EnumValueDescriptor.java

@@ -18,15 +18,13 @@ public final class EnumValueDescriptor {
    private String fullName;
    private String scopedName; // the name of this enum value constant in its scope
    private final int number;
-   private final String documentation;
    private final List<Option> options;
    private EnumDescriptor enumDescriptor;
    private FileDescriptor fileDescriptor;
 
    private EnumValueDescriptor(Builder builder) {
       this.name = builder.name;
       this.number = builder.number;
-      this.documentation = builder.documentation;
       this.options = List.copyOf(builder.options);
    }
 
@@ -38,8 +36,12 @@ public int getNumber() {
       return number;
    }
 
+   /**
+    * @deprecated Documentation is no longer retained.
+    */
+   @Deprecated(forRemoval = true)
    public String getDocumentation() {
-      return documentation;
+      return null;
    }
 
    public List<Option> getOptions() {

core/src/main/java/org/infinispan/protostream/descriptors/FieldDescriptor.java

@@ -118,6 +118,7 @@ void setEnumType(EnumDescriptor enumDescriptor) {
 
    void setFileDescriptor(FileDescriptor fileDescriptor) {
       this.fileDescriptor = fileDescriptor;
+      processAnnotations();
    }
 
    @Override

core/src/main/java/org/infinispan/protostream/descriptors/OneOfDescriptor.java

@@ -10,22 +10,24 @@
 public final class OneOfDescriptor {
 
    private final String name;
-   private final String documentation;
    private final List<FieldDescriptor> fields;
    private Descriptor containingMessage;
 
-   private OneOfDescriptor(String name, String documentation, List<FieldDescriptor> fields) {
+   private OneOfDescriptor(String name, List<FieldDescriptor> fields) {
       this.name = name;
-      this.documentation = documentation;
       this.fields = List.copyOf(fields);
    }
 
    public String getName() {
       return name;
    }
 
+   /**
+    * @deprecated Documentation is no longer retained.
+    */
+   @Deprecated(forRemoval = true)
    public String getDocumentation() {
-      return documentation;
+      return null;
    }
 
    public List<FieldDescriptor> getFields() {
@@ -73,7 +75,7 @@ public Builder addField(FieldDescriptor.Builder field) {
       }
 
       public OneOfDescriptor build() {
-         return new OneOfDescriptor(name, documentation, fields);
+         return new OneOfDescriptor(name, fields);
       }
    }
 }

core/src/main/java/org/infinispan/protostream/impl/AnnotatedDescriptorImpl.java

@@ -2,7 +2,7 @@
 
 import java.util.Collections;
 import java.util.LinkedHashMap;
-import java.util.LinkedList;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
@@ -29,9 +29,9 @@ public abstract class AnnotatedDescriptorImpl implements AnnotatedDescriptor {
    protected String fullName;
 
    /*
-    * The (optional) documentation comment.
+    * The (optional) documentation comment. Cleared after annotation parsing to reduce memory footprint.
     */
-   protected final String documentation;
+   private String documentation;
 
    /*
     * The annotations found in the documentation.
@@ -64,9 +64,10 @@ public final String getFullName() {
       return fullName;
    }
 
+   @Deprecated(forRemoval = true)
    @Override
    public final String getDocumentation() {
-      return documentation;
+      return null;
    }
 
    /*
@@ -75,12 +76,22 @@ public final String getDocumentation() {
     *
     * @throws AnnotationParserException if annotation parsing fails
     */
-   private void processAnnotations() throws AnnotationParserException {
-      // we are lazily processing the annotations, if there is a documentation text attached to this element
+   protected void processAnnotations() throws AnnotationParserException {
       if (annotations == null) {
          if (documentation != null) {
             AnnotationParser parser = new AnnotationParser(documentation, true);
-            List<AnnotationElement.Annotation> parsedAnnotations = parser.parse();
+            List<AnnotationElement.Annotation> parsedAnnotations;
+            try {
+               parsedAnnotations = parser.parse();
+            } catch (AnnotationParserException e) {
+               // Documentation contains annotation-like syntax that cannot be parsed.
+               // This can happen for annotations intended for compile-time processing only.
+               log.debugf("Failed to parse annotations on %s: %s", fullName, e.getMessage());
+               annotations = Collections.emptyMap();
+               processedAnnotations = Collections.emptyMap();
+               documentation = null;
+               return;
+            }
             Map<String, AnnotationElement.Annotation> _annotations = new LinkedHashMap<>();
             Map<String, AnnotationElement.Annotation> _containers = new LinkedHashMap<>();
             for (AnnotationElement.Annotation annotation : parsedAnnotations) {
@@ -101,7 +112,7 @@ private void processAnnotations() throws AnnotationParserException {
                      if (annotationConfig.repeatable() != null) {
                         AnnotationElement.Annotation container = _containers.get(annotation.getName());
                         if (container == null) {
-                           List<AnnotationElement.Value> values = new LinkedList<>();
+                           List<AnnotationElement.Value> values = new ArrayList<>();
                            values.add(_annotations.remove(annotation.getName()));
                            values.add(annotation);
                            AnnotationElement.Attribute value = new AnnotationElement.Attribute(annotation.position, AnnotationElement.Annotation.VALUE_DEFAULT_ATTRIBUTE, new AnnotationElement.Array(annotation.position, values));
@@ -146,6 +157,7 @@ private void processAnnotations() throws AnnotationParserException {
             annotations = Collections.emptyMap();
             processedAnnotations = Collections.emptyMap();
          }
+         documentation = null;
       }
    }
 

core/src/main/java/org/infinispan/protostream/impl/ProtoStreamWriterImpl.java

@@ -5,7 +5,7 @@
 import java.time.Instant;
 import java.util.Collection;
 import java.util.Date;
-import java.util.LinkedList;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
@@ -450,7 +450,7 @@ public void writeBytes(String fieldName, InputStream input) throws IOException {
       }
 
       int len = 0;
-      List<byte[]> chunks = new LinkedList<>();
+      List<byte[]> chunks = new ArrayList<>();
       int bufLen;
       byte[] buffer = new byte[CHUNK_SIZE];
       while ((bufLen = input.read(buffer)) != -1) {

core/src/main/java/org/infinispan/protostream/impl/parser/AnnotationParser.java

@@ -1,7 +1,7 @@
 package org.infinispan.protostream.impl.parser;
 
 import java.util.LinkedHashMap;
-import java.util.LinkedList;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
@@ -38,7 +38,7 @@ public AnnotationParser(String input, boolean expectDocNoise) {
     * @throws AnnotationParserException if syntax errors are encountered
     */
    public List<AnnotationElement.Annotation> parse() throws AnnotationParserException {
-      List<AnnotationElement.Annotation> annotations = new LinkedList<>();
+      List<AnnotationElement.Annotation> annotations = new ArrayList<>();
       while (lexer.token != AnnotationTokens.EOF) {
          AnnotationElement.Annotation annotation = parseAnnotation();
          annotations.add(annotation);
@@ -245,7 +245,7 @@ private AnnotationElement.Array parseArray() {
       int start = lexer.getBufferPos();
       long pos = lexer.pos;
       expect(AnnotationTokens.LBRACE);
-      List<AnnotationElement.Value> values = new LinkedList<>();
+      List<AnnotationElement.Value> values = new ArrayList<>();
       while (lexer.token != AnnotationTokens.RBRACE && lexer.token != AnnotationTokens.EOF) {
          values.add(parseValue(start));
          start = lexer.getBufferPos();

core/src/test/java/org/infinispan/protostream/impl/parser/DescriptorsTest.java

@@ -14,7 +14,6 @@
 import java.util.Map;
 
 import org.hamcrest.core.StringStartsWith;
-import org.infinispan.protostream.AnnotationParserException;
 import org.infinispan.protostream.DescriptorParserException;
 import org.infinispan.protostream.FileDescriptorSource;
 import org.infinispan.protostream.config.Configuration;
@@ -766,8 +765,8 @@ public void testDocComment() {
       assertNotNull(typeX);
       assertEquals(1, typeX.getFields().size());
       FieldDescriptor field1 = typeX.getFields().get(0);
-      assertEquals("some doc text \nsome more doc text", typeX.getDocumentation().trim());
-      assertEquals("field doc text", field1.getDocumentation().trim());
+      assertNull(typeX.getDocumentation());
+      assertNull(field1.getDocumentation());
    }
 
    @Test
@@ -811,11 +810,11 @@ public void testDocAnnotations() {
       assertNotNull(typeX);
       assertEquals(1, typeX.getFields().size());
       FieldDescriptor field1 = typeX.getFields().get(0);
-      assertEquals("@Foo(fooValue) \nsome more doc text", typeX.getDocumentation().trim());
+      assertNull(typeX.getDocumentation());
       Map<String, AnnotationElement.Annotation> typeAnnotations = typeX.getAnnotations();
       assertEquals("fooValue", typeAnnotations.get("Foo").getDefaultAttributeValue().getValue());
       assertEquals("fooValue", typeX.getProcessedAnnotation("Foo"));
-      assertEquals("@Bar(barValue)", field1.getDocumentation().trim());
+      assertNull(field1.getDocumentation());
       Map<String, AnnotationElement.Annotation> fieldAnnotations = field1.getAnnotations();
       assertEquals("barValue", fieldAnnotations.get("Bar").getDefaultAttributeValue().getValue());
       assertEquals("barValue", field1.getProcessedAnnotation("Bar"));
@@ -840,7 +839,7 @@ public void testAnnotationParser() throws Exception {
       Descriptor userMessageType = messageTypes.get(0);
       assertEquals("sample_bank_account.User", userMessageType.getFullName());
       assertEquals(Boolean.TRUE, userMessageType.getProcessedAnnotation("Indexed"));
-      assertEquals("CommentOne\n@CommentTwo\n@Field(store = Store.YES)\n@SortableField\n", userMessageType.getFields().get(0).getDocumentation());
+      assertNull(userMessageType.getFields().get(0).getDocumentation());
 
       Descriptor accountMessageType = messageTypes.get(1);
       assertEquals("sample_bank_account.Account", accountMessageType.getFullName());
@@ -849,7 +848,7 @@ public void testAnnotationParser() throws Exception {
 
    @Test
    public void testDuplicateAnnotation() {
-      exception.expect(AnnotationParserException.class);
+      exception.expect(DescriptorParserException.class);
       exception.expectMessage("Error: 1,8: duplicate annotation definition \"Field\"");
 
       Configuration config = Configuration.builder().annotationsConfig()
@@ -867,10 +866,7 @@ public void testDuplicateAnnotation() {
             }""";
 
       FileDescriptorSource fileDescriptorSource = FileDescriptorSource.fromString("test.proto", testProto);
-      Map<String, FileDescriptor> descriptors = parseAndResolve(fileDescriptorSource, config);
-
-      //todo [anistor] this is waaay too lazy
-      descriptors.get("test.proto").getMessageTypes().get(0).getFields().get(0).getAnnotations();
+      parseAndResolve(fileDescriptorSource, config);
    }
 
    @Test
@@ -933,9 +929,6 @@ public void testDuplicateUndefinedAnnotation() {
 
    @Test
    public void testBrokenUndefinedAnnotation() {
-      exception.expect(AnnotationParserException.class);
-      exception.expectMessage("Error: 2,23: ')' expected");
-
       Configuration config = Configuration.builder().annotationsConfig()
             .annotation("Field", AnnotationElement.AnnotationTarget.FIELD)
             .attribute(AnnotationElement.Annotation.VALUE_DEFAULT_ATTRIBUTE)
@@ -954,8 +947,9 @@ public void testBrokenUndefinedAnnotation() {
       FileDescriptorSource fileDescriptorSource = FileDescriptorSource.fromString("test.proto", testProto);
       Map<String, FileDescriptor> descriptors = parseAndResolve(fileDescriptorSource, config);
 
-      //todo [anistor] The processing of annotations is waaay too lazy
-      descriptors.get("test.proto").getMessageTypes().get(0).getFields().get(0).getAnnotations();
+      // Syntax errors in documentation annotations are silently ignored during eager parsing
+      Map<String, AnnotationElement.Annotation> fieldAnnotations = descriptors.get("test.proto").getMessageTypes().get(0).getFields().get(0).getAnnotations();
+      assertTrue(fieldAnnotations.isEmpty());
    }
 
    @Test

core/src/test/java/org/infinispan/protostream/impl/parser/FileDescriptorMemoryTest.java

@@ -0,0 +1,24 @@
+package org.infinispan.protostream.impl.parser;
+
+import java.util.Map;
+
+import org.infinispan.protostream.FileDescriptorSource;
+import org.infinispan.protostream.config.Configuration;
+import org.infinispan.protostream.descriptors.FileDescriptor;
+import org.junit.Test;
+import org.openjdk.jol.info.GraphLayout;
+
+public class FileDescriptorMemoryTest {
+
+   @Test
+   public void testFileDescriptorMemoryFootprint() throws Exception {
+      Configuration config = Configuration.builder().build();
+      FileDescriptorSource source = FileDescriptorSource.fromResources("sample_bank_account/bank.proto");
+      Map<String, FileDescriptor> descriptors = DescriptorsTest.parseAndResolve(source, config);
+      FileDescriptor fileDescriptor = descriptors.get("sample_bank_account/bank.proto");
+
+      GraphLayout layout = GraphLayout.parseInstance(fileDescriptor);
+      System.out.println(layout.toFootprint());
+      System.out.println("Total size: " + layout.totalSize() + " bytes");
+   }
+}