Merge pull request #19030 from vespa-engine/balder/keep-an-unmodifiable-set-instead-of-creating-a-copy-on-use

Instead of creatin a new Set of consumers every time, just keep an un…

2 files changed, 14 insertions, 15 deletions

diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/MetricsConsumers.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/MetricsConsumers.java
index 8691b569a95..35d498b9f77 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/MetricsConsumers.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/MetricsConsumers.java
@@ -7,6 +7,7 @@ import ai.vespa.metricsproxy.metric.model.ConsumerId;
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -30,7 +31,7 @@ public class MetricsConsumers {
     private final Map<ConsumerId, List<ConfiguredMetric>> consumerMetrics;
 
     // All consumers for each metric (more useful than the opposite map).
-    private final Map<ConfiguredMetric, List<ConsumerId>> consumersByMetric;
+    private final Map<ConfiguredMetric, Set<ConsumerId>> consumersByMetric;
 
     public MetricsConsumers(ConsumersConfig config) {
         consumerMetrics = config.consumer().stream().collect(
@@ -47,7 +48,7 @@ public class MetricsConsumers {
         return consumerMetrics.get(consumer);
     }
 
-    public Map<ConfiguredMetric, List<ConsumerId>> getConsumersByMetric() {
+    public Map<ConfiguredMetric, Set<ConsumerId>> getConsumersByMetric() {
         return consumersByMetric;
     }
 
@@ -59,14 +60,16 @@ public class MetricsConsumers {
      * Helper function to create mapping from metric to consumers.
      * TODO: consider reversing the mapping in metrics-consumers.def instead: metric{}.consumer[]
      */
-    private static Map<ConfiguredMetric, List<ConsumerId>>
+    private static Map<ConfiguredMetric, Set<ConsumerId>>
     createConsumersByMetric(Map<ConsumerId, List<ConfiguredMetric>> metricsByConsumer) {
-        Map<ConfiguredMetric, List<ConsumerId>> consumersByMetric = new LinkedHashMap<>();
+        Map<ConfiguredMetric, Set<ConsumerId>> consumersByMetric = new LinkedHashMap<>();
         metricsByConsumer.forEach(
                 (consumer, metrics) -> metrics.forEach(
-                        metric -> consumersByMetric.computeIfAbsent(metric, unused -> new ArrayList<>())
+                        metric -> consumersByMetric.computeIfAbsent(metric, unused -> new HashSet<>())
                                 .add(consumer)));
-        return Collections.unmodifiableMap(consumersByMetric);
+        Map<ConfiguredMetric, Set<ConsumerId>> unmodifiableConsumersByMetric = new LinkedHashMap<>();
+        consumersByMetric.forEach((configuredMetric, consumerIds) -> unmodifiableConsumersByMetric.put(configuredMetric, Set.copyOf(consumerIds)));
+        return Collections.unmodifiableMap(unmodifiableConsumersByMetric);
     }
 
     public static <T, K, U> Collector<T, ?, Map<K, U>> toUnmodifiableLinkedMap(Function<? super T, ? extends K> keyMapper,
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java
index 0c83d8f73c4..3fc2bb316a1 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java
@@ -68,7 +68,7 @@ public class VespaMetrics {
     public List<MetricsPacket.Builder> getMetrics(List<VespaService> services) {
         List<MetricsPacket.Builder> metricsPackets = new ArrayList<>();
 
-        Map<ConfiguredMetric, List<ConsumerId>> consumersByMetric = metricsConsumers.getConsumersByMetric();
+        Map<ConfiguredMetric, Set<ConsumerId>> consumersByMetric = metricsConsumers.getConsumersByMetric();
 
         for (VespaService service : services) {
             // One metrics packet for system metrics
@@ -119,7 +119,7 @@ public class VespaMetrics {
      * In order to include a metric, it must exist in the given map of metric to consumers.
      * Each returned metric will contain a collection of consumers that it should be routed to.
      */
-    private Metrics getServiceMetrics(Metrics allServiceMetrics, Map<ConfiguredMetric, List<ConsumerId>> consumersByMetric) {
+    private Metrics getServiceMetrics(Metrics allServiceMetrics, Map<ConfiguredMetric, Set<ConsumerId>> consumersByMetric) {
         Metrics configuredServiceMetrics = new Metrics();
         configuredServiceMetrics.setTimeStamp(getMostRecentTimestamp(allServiceMetrics));
         for (Metric candidate : allServiceMetrics.getMetrics()) {
@@ -139,21 +139,17 @@ public class VespaMetrics {
         return dimensions;
     }
 
-    private Set<ConsumerId> extractConsumers(List<ConsumerId> configuredConsumers) {
+    private Set<ConsumerId> extractConsumers(Set<ConsumerId> configuredConsumers) {
         Set<ConsumerId> consumers = Collections.emptySet();
         if (configuredConsumers != null) {
-            if ( configuredConsumers.size() == 1) {
-                consumers = Collections.singleton(configuredConsumers.get(0));
-            } else if (configuredConsumers.size() > 1){
-                consumers = Set.copyOf(configuredConsumers);
-            }
+            consumers = configuredConsumers;
         }
         return consumers;
     }
 
     private Metric metricWithConfigProperties(Metric candidate,
                                               ConfiguredMetric configuredMetric,
-                                              Map<ConfiguredMetric, List<ConsumerId>> consumersByMetric) {
+                                              Map<ConfiguredMetric, Set<ConsumerId>> consumersByMetric) {
         Metric metric = candidate.clone();
         metric.setDimensions(extractDimensions(candidate.getDimensions(), configuredMetric.dimension()));
         metric.setConsumers(extractConsumers(consumersByMetric.get(configuredMetric)));