Avoid creating many sets and lists containing the same list of consumers.

Use a dictionary to keep track of the unique ones.

5 files changed, 36 insertions, 55 deletions

diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java
index b08c3f4d668..02ccd2c988c 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java
@@ -36,7 +36,7 @@ public class MetricsPacket {
     public final ServiceId service;
     private final Map<MetricId, Number> metrics;
     private final Map<DimensionId, String> dimensions;
-    private final List<ConsumerId> consumers;
+    private final Set<ConsumerId> consumers;
 
     private MetricsPacket(int statusCode, String statusMessage, long timestamp, ServiceId service,
                           Map<MetricId, Number> metrics, Map<DimensionId, String> dimensions, Set<ConsumerId> consumers ) {
@@ -46,7 +46,7 @@ public class MetricsPacket {
         this.service = service;
         this.metrics = metrics;
         this.dimensions = dimensions;
-        this.consumers = new ArrayList<>(consumers);
+        this.consumers = Set.copyOf(consumers);
     }
 
     public Map<MetricId, Number> metrics() {
@@ -57,8 +57,8 @@ public class MetricsPacket {
         return unmodifiableMap(dimensions);
     }
 
-    public List<ConsumerId> consumers() {
-        return unmodifiableList(consumers);
+    public Set<ConsumerId> consumers() {
+        return consumers;
     }
 
     @Override
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModel.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModel.java
index e86ab2ab41a..d2be7be25d1 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModel.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModel.java
@@ -60,29 +60,6 @@ public class YamasJsonModel {
     }
 
     /**
-     * Convenience method to add targets to the routing object
-     *
-     * @param names Namespaces E.g "Vespa"
-     */
-    public void addRouting(Set<ConsumerId> names) {
-        // Setup routing structure if not already existing
-        if (routing == null) {
-            routing = new HashMap<>();
-        }
-
-        if (! routing.containsKey("yamas")) {
-            routing.put("yamas", new YamasJsonModel.YamasJsonNamespace());
-        }
-        YamasJsonModel.YamasJsonNamespace namespace = routing.get("yamas");
-
-        if (namespace.namespaces == null) {
-            namespace.namespaces = new ArrayList<>();
-        }
-
-        namespace.namespaces.addAll(names.stream().map(consumer -> consumer.id).collect(Collectors.toList()));
-    }
-
-    /**
      * Convenience method to add dimensions
      */
     public void addDimensions(Map<DimensionId, String> additionalDimensions, boolean replace) {
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonUtil.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonUtil.java
index 19bcdbfdb74..602879aa0b6 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonUtil.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonUtil.java
@@ -7,13 +7,14 @@ import ai.vespa.metricsproxy.metric.model.ServiceId;
 import com.fasterxml.jackson.core.JsonFactory;
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.google.common.collect.ImmutableMap;
+import com.yahoo.concurrent.CopyOnWriteHashMap;
 
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
@@ -30,6 +31,7 @@ public class YamasJsonUtil {
 
     static final String YAMAS_ROUTING = "yamas";
 
+    private static final Map<Set<ConsumerId>, Map<String, YamasJsonModel.YamasJsonNamespace>> globalNameSpaces = new CopyOnWriteHashMap<>();
     public static MetricsPacket.Builder toMetricsPacketBuilder(YamasJsonModel jsonModel) {
         if (jsonModel.application == null)
             throw new IllegalArgumentException("Service id cannot be null");
@@ -79,12 +81,12 @@ public class YamasJsonUtil {
         }
     }
 
-    private static YamasJsonModel getStatusYamasModel(String statusMessage, int statusCode, Collection<ConsumerId> consumers) {
+    private static YamasJsonModel getStatusYamasModel(String statusMessage, int statusCode, Set<ConsumerId> consumers) {
         YamasJsonModel model = new YamasJsonModel();
         model.status_code = statusCode;
         model.status_msg = statusMessage;
         model.application = "yms_check_vespa";
-        model.routing = ImmutableMap.of(YAMAS_ROUTING, toYamasJsonNamespaces(consumers));
+        model.routing = computeIfAbsent(consumers);
         return model;
     }
 
@@ -99,38 +101,38 @@ public class YamasJsonUtil {
         model.application = packet.service.id;
         model.timestamp = (packet.timestamp == 0L) ? null : packet.timestamp;
 
-        if (packet.metrics().isEmpty()) model.metrics = null;
-        else {
-            model.metrics = packet.metrics().entrySet().stream().collect(
+        model.metrics = (packet.metrics().isEmpty())
+                ? null
+                : packet.metrics().entrySet().stream().collect(
                     toLinkedMap(id2metric -> id2metric.getKey().id,
                                 id2metric -> id2metric.getValue().doubleValue()));
-        }
 
-        if (packet.dimensions().isEmpty()) model.dimensions = null;
-        else {
-            model.dimensions = packet.dimensions().entrySet()
+        model.dimensions = (packet.dimensions().isEmpty())
+                ? null
+                : packet.dimensions().entrySet()
                     .stream()
                     .filter(entry -> entry.getKey() != null && entry.getValue() != null)
-                    .collect(toLinkedMap(
-                            id2dim -> id2dim.getKey().id,
-                            Map.Entry::getValue)
-                    );
-        }
+                    .collect(toLinkedMap(id2dim -> id2dim.getKey().id, Map.Entry::getValue));
 
-        YamasJsonModel.YamasJsonNamespace namespaces = toYamasJsonNamespaces(packet.consumers());
-        if (namespaces.namespaces.isEmpty()) model.routing = null;
-        else model.routing = ImmutableMap.of(YAMAS_ROUTING, namespaces);
+        model.routing = computeIfAbsent(packet.consumers());
 
         return model;
     }
 
-    private static YamasJsonModel.YamasJsonNamespace toYamasJsonNamespaces(Collection<ConsumerId> consumers) {
-        YamasJsonModel.YamasJsonNamespace namespaces =  new YamasJsonModel.YamasJsonNamespace();
-        namespaces.namespaces = consumers.stream()
+    private static Map<String, YamasJsonModel.YamasJsonNamespace> computeIfAbsent(Set<ConsumerId> consumers) {
+        return globalNameSpaces.computeIfAbsent(consumers, YamasJsonUtil::createYamasJson);
+    }
+
+    private static Map<String, YamasJsonModel.YamasJsonNamespace> createYamasJson(Set<ConsumerId> consumers) {
+        List<String> namespaces = consumers.stream()
                 .filter(consumerId -> consumerId != defaultMetricsConsumerId)
                 .map(consumer -> consumer.id)
                 .collect(Collectors.toList());
-        return namespaces;
+        if (namespaces.isEmpty()) return null;
+
+        YamasJsonModel.YamasJsonNamespace yamasJsonNamespace = new YamasJsonModel.YamasJsonNamespace();
+        yamasJsonNamespace.namespaces = namespaces;
+        return Map.of(YAMAS_ROUTING, yamasJsonNamespace);
     }
 
 }
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/ExternalMetricsTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/ExternalMetricsTest.java
index 002e6334141..581367878ca 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/ExternalMetricsTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/ExternalMetricsTest.java
@@ -10,11 +10,12 @@ import com.google.common.collect.ImmutableList;
 import org.junit.Test;
 
 import java.util.List;
+import java.util.Set;
 
-import static ai.vespa.metricsproxy.metric.ExternalMetrics.VESPA_NODE_SERVICE_ID;
 import static ai.vespa.metricsproxy.metric.model.ConsumerId.toConsumerId;
 import static ai.vespa.metricsproxy.metric.model.ServiceId.toServiceId;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 /**
  * @author gjoranv
@@ -64,10 +65,10 @@ public class ExternalMetricsTest {
         List<MetricsPacket.Builder> packets = externalMetrics.getMetrics();
         assertEquals(1, packets.size());
 
-        List<ConsumerId> consumerIds = packets.get(0).build().consumers();
+        Set<ConsumerId> consumerIds = packets.get(0).build().consumers();
         assertEquals(2, consumerIds.size());
-        assertEquals(CUSTOM_CONSUMER_1, consumerIds.get(0));
-        assertEquals(CUSTOM_CONSUMER_2, consumerIds.get(1));
+        assertTrue(consumerIds.contains(CUSTOM_CONSUMER_1));
+        assertTrue(consumerIds.contains(CUSTOM_CONSUMER_2));
     }
 
 }
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModelTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModelTest.java
index 9b889085a87..540445fba5b 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModelTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModelTest.java
@@ -16,6 +16,7 @@ import static ai.vespa.metricsproxy.metric.model.MetricId.toMetricId;
 import static ai.vespa.metricsproxy.metric.model.ServiceId.toServiceId;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 /**
  * Tests for YamasJsonModel and YamasArrayJsonModel
@@ -43,7 +44,7 @@ public class YamasJsonModelTest {
 
         // Do some sanity checking
         assertEquals("vespa.searchnode", jsonModel.application);
-        assertEquals("Vespa", jsonModel.routing.get("yamas").namespaces.get(0));
+        assertTrue(jsonModel.routing.get("yamas").namespaces.contains("Vespa"));
         assertEquals(5.555555555E9, jsonModel.metrics.get("memory_rss"), 0.1d); //Not using custom double renderer
 
         // Serialize and verify
@@ -60,7 +61,7 @@ public class YamasJsonModelTest {
 
         // Do some sanity checking
         assertEquals(toServiceId("vespa.searchnode"), metricsPacket.service);
-        assertEquals(toConsumerId("Vespa"), metricsPacket.consumers().get(0));
+        assertTrue(metricsPacket.consumers().contains(toConsumerId("Vespa")));
         assertEquals(5.555555555E9, metricsPacket.metrics().get(toMetricId("memory_rss")).doubleValue(), 0.1d); //Not using custom double rendrer
 
         // Serialize and verify