Remove throwaway builders, boxed positions and eager maps from the parse path

common/src/main/java/dev/cel/common/CelSource.java

@@ -33,6 +33,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import org.jspecify.annotations.Nullable;
 
 /** Represents the source content of an expression and related metadata. */
 @Immutable
@@ -162,9 +163,13 @@ public static final class Builder {
 
     private final CelCodePointArray codePoints;
     private final List<Integer> lineOffsets;
-    private final Map<Long, Integer> positions;
-    private final Map<Long, CelExpr> macroCalls;
-    private final ImmutableSet.Builder<Extension> extensions;
+    // Both maps start out immutable and empty, and are only copied into a mutable map once they are
+    // actually modified. This keeps the common case (a source that is either never populated, or
+    // populated in bulk from an already immutable map) allocation free.
+    private Map<Long, Integer> positions;
+    private Map<Long, CelExpr> macroCalls;
+    // Null until the first extension is added; extensions are rare.
+    private ImmutableSet.@Nullable Builder<Extension> extensions;
 
     private final boolean lineOffsetsAlreadyComputed;
     private String description;
@@ -176,9 +181,8 @@ private Builder() {
     private Builder(CelCodePointArray codePoints, List<Integer> lineOffsets) {
       this.codePoints = checkNotNull(codePoints);
       this.lineOffsets = checkNotNull(lineOffsets);
-      this.positions = new HashMap<>();
-      this.macroCalls = new HashMap<>();
-      this.extensions = ImmutableSet.builder();
+      this.positions = ImmutableMap.of();
+      this.macroCalls = ImmutableMap.of();
       this.description = "";
       this.lineOffsetsAlreadyComputed = !lineOffsets.isEmpty();
     }
@@ -207,39 +211,66 @@ public Builder addAllLineOffsets(Iterable<Integer> lineOffsets) {
       return this;
     }
 
+    /** Returns {@code map} as a mutable map, copying it first if it is still immutable. */
+    private static <K, V> Map<K, V> ensureMutable(Map<K, V> map) {
+      return map instanceof HashMap ? map : new HashMap<>(map);
+    }
+
+    /**
+     * Returns a map containing every entry of {@code map} plus every entry of {@code additions}.
+     *
+     * <p>If {@code map} is still the empty immutable placeholder a builder starts with, and {@code
+     * additions} is already immutable, then {@code additions} is adopted as-is and no copy is made.
+     * That is the common case: a source populated in bulk exactly once, which lets {@link #build()}
+     * reuse the argument directly. Otherwise the entries are merged into a mutable copy.
+     */
+    private static <K, V> Map<K, V> augmentedMap(Map<K, V> map, Map<K, V> additions) {
+      if (map instanceof ImmutableMap && map.isEmpty() && additions instanceof ImmutableMap) {
+        return additions;
+      }
+      Map<K, V> merged = ensureMutable(map);
+      merged.putAll(additions);
+      return merged;
+    }
+
     @CanIgnoreReturnValue
     public Builder addPositionsMap(Map<Long, Integer> positionsMap) {
       checkNotNull(positionsMap);
-      this.positions.putAll(positionsMap);
+      positions = augmentedMap(positions, positionsMap);
       return this;
     }
 
     @CanIgnoreReturnValue
     public Builder addPositions(long exprId, int position) {
-      this.positions.put(exprId, position);
+      positions = ensureMutable(positions);
+      positions.put(exprId, position);
       return this;
     }
 
     @CanIgnoreReturnValue
     public Builder removePositions(long exprId) {
-      this.positions.remove(exprId);
+      if (positions.containsKey(exprId)) {
+        positions = ensureMutable(positions);
+        positions.remove(exprId);
+      }
       return this;
     }
 
     @CanIgnoreReturnValue
     public Builder addMacroCalls(long exprId, CelExpr expr) {
-      this.macroCalls.put(exprId, expr);
+      macroCalls = ensureMutable(macroCalls);
+      macroCalls.put(exprId, expr);
       return this;
     }
 
     @CanIgnoreReturnValue
     public Builder addAllMacroCalls(Map<Long, CelExpr> macroCalls) {
-      this.macroCalls.putAll(macroCalls);
+      this.macroCalls = augmentedMap(this.macroCalls, macroCalls);
       return this;
     }
 
     public ImmutableSet<Extension> getExtensions() {
-      return extensions.build();
+      return extensions == null ? ImmutableSet.of() : extensions.build();
     }
 
     /**
@@ -249,6 +280,9 @@ public ImmutableSet<Extension> getExtensions() {
     @CanIgnoreReturnValue
     public Builder addAllExtensions(Iterable<? extends Extension> extensions) {
       checkNotNull(extensions);
+      if (this.extensions == null) {
+        this.extensions = ImmutableSet.builder();
+      }
       this.extensions.addAll(extensions);
       return this;
     }
@@ -287,13 +321,17 @@ public Optional<CelSourceLocation> getOffsetLocation(int offset) {
       return CelSourceHelper.getOffsetLocation(codePoints, offset);
     }
 
+    /** Returns a live, mutable view of the positions recorded so far. */
     @CheckReturnValue
     public Map<Long, Integer> getPositionsMap() {
-      return this.positions;
+      positions = ensureMutable(positions);
+      return positions;
     }
 
+    /** Returns a live, mutable view of the macro calls recorded so far. */
     @CheckReturnValue
     public Map<Long, CelExpr> getMacroCalls() {
+      macroCalls = ensureMutable(macroCalls);
       return macroCalls;
     }
 
@@ -310,7 +348,7 @@ public CelSource build() {
           ImmutableList.copyOf(lineOffsets),
           ImmutableMap.copyOf(positions),
           ImmutableMap.copyOf(macroCalls),
-          extensions.build());
+          getExtensions());
     }
   }
 

common/src/main/java/dev/cel/common/CelValidationResult.java

@@ -22,6 +22,7 @@
 import com.google.errorprone.annotations.Immutable;
 import com.google.errorprone.annotations.InlineMe;
 import dev.cel.common.annotations.Internal;
+import java.util.Comparator;
 import org.jspecify.annotations.Nullable;
 
 /**
@@ -31,6 +32,9 @@
 @Immutable
 public final class CelValidationResult {
 
+  private static final Comparator<CelIssue> BY_SOURCE_LOCATION =
+      comparing(CelIssue::getSourceLocation);
+
   @SuppressWarnings("Immutable")
   private final @Nullable Throwable failure;
 
@@ -64,11 +68,20 @@ private CelValidationResult(
       @Nullable Throwable failure) {
     this.ast = ast;
     this.source = source;
-    this.issues = ImmutableList.sortedCopyOf(comparing(CelIssue::getSourceLocation), issues);
-    this.hasError = issues.stream().anyMatch(CelValidationResult::issueIsError) || failure != null;
+    this.issues = ImmutableList.sortedCopyOf(BY_SOURCE_LOCATION, issues);
+    this.hasError = failure != null || containsError(issues);
     this.failure = failure;
   }
 
+  private static boolean containsError(ImmutableList<CelIssue> issues) {
+    for (int i = 0; i < issues.size(); i++) {
+      if (issueIsError(issues.get(i))) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   /**
    * Returns the validated {@code CelAbstractSyntaxTree} if one exists.
    *