Merge pull request #20533 from vespa-engine/balder/compute-if-absentv7.517.18

Balder/compute if absent

5 files changed, 110 insertions, 47 deletions

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 7a3f260cd34..93f8ec0440b 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
@@ -153,11 +153,8 @@ public class VespaMetrics {
         }
 
         private static Set<ConsumerId> extractConsumers(Set<ConsumerId> configuredConsumers) {
-            Set<ConsumerId> consumers = Collections.emptySet();
-            if (configuredConsumers != null) {
-                consumers = configuredConsumers;
-            }
-            return consumers;
+            if (configuredConsumers != null) return configuredConsumers;
+            return Set.of();
         }
     }
 
@@ -189,14 +186,7 @@ public class VespaMetrics {
             mergedDimensions.putAll(metric.getDimensions());
             mergedDimensions.putAll(serviceDimensions);
             AggregationKey aggregationKey = new AggregationKey(mergedDimensions, metric.getConsumers());
-
-            if (aggregated.containsKey(aggregationKey)) {
-                aggregated.get(aggregationKey).add(metric);
-            } else {
-                List<Metric> ml = new ArrayList<>();
-                ml.add(metric);
-                aggregated.put(aggregationKey, ml);
-            }
+            aggregated.computeIfAbsent(aggregationKey, key -> new ArrayList<>()).add(metric);
         }
     }
 
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/Metric.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/Metric.java
index a9e6e8e6e3c..9265e8ef1e5 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/Metric.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/Metric.java
@@ -5,8 +5,6 @@ import ai.vespa.metricsproxy.metric.model.ConsumerId;
 import ai.vespa.metricsproxy.metric.model.DimensionId;
 import ai.vespa.metricsproxy.metric.model.MetricId;
 
-import java.util.Collections;
-import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
@@ -34,12 +32,12 @@ public class Metric {
         this.time = time;
         this.value = value;
         this.name = name;
-        this.dimensions = dimensions;
+        this.dimensions = Map.copyOf(dimensions);
         this.description = description;
     }
 
     public Metric(MetricId name, Number value, long timestamp) {
-        this(name, value, timestamp, Collections.emptyMap(), "");
+        this(name, value, timestamp, Map.of(), "");
     }
 
     public Metric(MetricId name, Number value) {
@@ -47,7 +45,7 @@ public class Metric {
     }
 
     public void setDimensions(Map<DimensionId, String> dimensions) {
-        this.dimensions = dimensions;
+        this.dimensions = Map.copyOf(dimensions);
     }
 
     /**
@@ -104,7 +102,7 @@ public class Metric {
 
     @Override
     public Metric clone() {
-        return new Metric(name, value, time, new LinkedHashMap<>(dimensions), getDescription());
+        return new Metric(name, value, time, dimensions, getDescription());
     }
 
     @Override
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java
index 8157ecb72fd..f9443c46bad 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java
@@ -11,7 +11,6 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
@@ -123,7 +122,7 @@ public class MetricsParser {
             description = metric.get("description").textValue();
         }
 
-        Map<DimensionId, String> dim = Collections.emptyMap();
+        Map<DimensionId, String> dim = Map.of();
         if (metric.has("dimensions")) {
             JsonNode dimensions = metric.get("dimensions");
             StringBuilder sb = new StringBuilder();
@@ -139,7 +138,7 @@ public class MetricsParser {
                     String v = dimensions.get(k).textValue();
                     dim.put(toDimensionId(k), v);
                 }
-                uniqueDimensions.put(sb.toString(), Collections.unmodifiableMap(dim));
+                uniqueDimensions.put(sb.toString(), Map.copyOf(dim));
             }
             dim = uniqueDimensions.get(sb.toString());
         }
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/SystemPoller.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/SystemPoller.java
index 27f86b0d503..22eb2844d61 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/SystemPoller.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/SystemPoller.java
@@ -5,6 +5,7 @@ import ai.vespa.metricsproxy.metric.Metric;
 import ai.vespa.metricsproxy.metric.Metrics;
 import ai.vespa.metricsproxy.metric.model.MetricId;
 
+import java.io.Reader;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.HashMap;
@@ -106,7 +107,6 @@ public class SystemPoller {
      * @return array[0] = memoryResident, array[1] = memoryVirtual (kB units)
      */
     static long[] getMemoryUsage(VespaService service) {
-        long[] size = new long[2];
         BufferedReader br;
         int pid = service.getPid();
 
@@ -114,24 +114,33 @@ public class SystemPoller {
             br = new BufferedReader(new FileReader("/proc/" + pid + "/smaps"));
         } catch (FileNotFoundException ex) {
             service.setAlive(false);
-            return size;
+            return new long[2];
         }
-        String line;
         try {
-            while ((line = br.readLine()) != null) {
-                String[] elems = line.split("\\s+");
-                /* Memory size is given in kB - convert to bytes by multiply with 1024*/
-                if (line.startsWith("Rss:")) {
-                    size[memoryTypeResident] += Long.parseLong(elems[1]) * 1024;
-                } else if (line.startsWith("Size:")) {
-                    size[memoryTypeVirtual] += Long.parseLong(elems[1]) * 1024;
-                }
-            }
-
-            br.close();
+            return getMemoryUsage(br);
         } catch (IOException ex) {
             log.log(Level.FINE, "Unable to read line from smaps file", ex);
-            return size;
+            return new long[2];
+        } finally {
+            try {
+                br.close();
+            } catch (IOException ex) {
+                log.log(Level.FINE, "Closing of smaps file failed", ex);
+            }
+        }
+    }
+    static long[] getMemoryUsage(BufferedReader br) throws IOException{
+        String line;
+        long[] size = new long[2];
+        while ((line = br.readLine()) != null) {
+            /* Memory size is given in kB - convert to bytes by multiply with 1024*/
+            if (line.startsWith("Rss:")) {
+                String remain = line.substring(4).trim();
+                size[memoryTypeResident] += Long.parseLong(remain.substring(0, remain.indexOf(' '))) * 1024;
+            } else if (line.startsWith("Size:")) {
+                String remain = line.substring(5).trim();
+                size[memoryTypeVirtual] += Long.parseLong(remain.substring(0, remain.indexOf(' '))) * 1024;
+            }
         }
 
         return size;
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/SystemPollerTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/SystemPollerTest.java
index f5ae0048275..76909e050d5 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/SystemPollerTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/SystemPollerTest.java
@@ -4,18 +4,19 @@ package ai.vespa.metricsproxy.service;
 import ai.vespa.metricsproxy.metric.Metric;
 import ai.vespa.metricsproxy.metric.Metrics;
 import ai.vespa.metricsproxy.metric.model.MetricId;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import java.io.BufferedReader;
+import java.io.IOException;
 import java.io.StringReader;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertFalse;
 
 /**
  * @author Unknown
@@ -71,18 +72,84 @@ public class SystemPollerTest {
         List<VespaService> services = new ArrayList<>();
         services.add(s);
 
-        assertThat(s.isAlive(), is(false));
+        assertFalse(s.isAlive());
 
         long n = SystemPoller.getPidJiffies(s);
-        assertThat(n, is(0L));
+        assertEquals(0L, n);
         long[] memusage = SystemPoller.getMemoryUsage(s);
-        assertThat(memusage[0], is(0L));
-        assertThat(memusage[1], is(0L));
+        assertEquals(0L, memusage[0]);
+        assertEquals(0L, memusage[1]);
     }
 
+    private static final String smaps =
+            "00400000-004de000 r-xp 00000000 fe:01 670312                             /usr/bin/bash\n" +
+            "Size:                888 kB\n" +
+            "KernelPageSize:        4 kB\n" +
+            "MMUPageSize:           4 kB\n" +
+            "Rss:                 824 kB\n" +
+            "Pss:                 150 kB\n" +
+            "Shared_Clean:        824 kB\n" +
+            "Shared_Dirty:          0 kB\n" +
+            "Private_Clean:         0 kB\n" +
+            "Private_Dirty:         0 kB\n" +
+            "Referenced:          824 kB\n" +
+            "Anonymous:             0 kB\n" +
+            "LazyFree:              0 kB\n" +
+            "AnonHugePages:         0 kB\n" +
+            "ShmemPmdMapped:        0 kB\n" +
+            "FilePmdMapped:         0 kB\n" +
+            "Shared_Hugetlb:        0 kB\n" +
+            "Private_Hugetlb:       0 kB\n" +
+            "Swap:                  0 kB\n" +
+            "SwapPss:               0 kB\n" +
+            "Locked:                0 kB\n" +
+            "THPeligible:    0\n" +
+            "VmFlags: rd ex mr mw me dw \n" +
+            "006dd000-006de000 r--p 000dd000 fe:01 670312                             /usr/bin/bash\n" +
+            "Size:                  4 kB\n" +
+            "KernelPageSize:        4 kB\n" +
+            "MMUPageSize:           4 kB\n" +
+            "Rss:                   4 kB\n" +
+            "Pss:                   4 kB\n" +
+            "Shared_Clean:          0 kB\n" +
+            "Shared_Dirty:          0 kB\n" +
+            "Private_Clean:         4 kB\n" +
+            "Private_Dirty:         0 kB\n" +
+            "Referenced:            4 kB\n" +
+            "Anonymous:             4 kB\n" +
+            "LazyFree:              0 kB\n" +
+            "AnonHugePages:         0 kB\n" +
+            "ShmemPmdMapped:        0 kB\n" +
+            "FilePmdMapped:         0 kB\n" +
+            "Shared_Hugetlb:        0 kB\n" +
+            "Private_Hugetlb:       0 kB\n" +
+            "Swap:                  0 kB\n" +
+            "SwapPss:               0 kB\n" +
+            "Locked:                0 kB\n" +
+            "THPeligible:    0\n" +
+            "VmFlags: rd mr mw me dw ac \n";
+
     @Test
-    public
-    void testPerProcessJiffies() {
+    public void testSmapsParsing() throws IOException {
+        BufferedReader br = new BufferedReader(new StringReader(smaps));
+        long[] memusage = SystemPoller.getMemoryUsage(br);
+        assertEquals(913408L, memusage[0]);
+        assertEquals(847872L, memusage[1]);
+    }
+
+    @Ignore
+    @Test
+    public void benchmarkSmapsParsing() throws IOException {
+        for (int i=0; i < 100000; i++) {
+            BufferedReader br = new BufferedReader(new StringReader(smaps));
+            long[] memusage = SystemPoller.getMemoryUsage(br);
+            assertEquals(913408L, memusage[0]);
+            assertEquals(847872L, memusage[1]);
+        }
+    }
+
+    @Test
+    public void testPerProcessJiffies() {
         assertEquals(PER_PROC_JIFFIES[0], SystemPoller.getPidJiffies(new BufferedReader(new StringReader(perProcStats[0]))));
         assertEquals(PER_PROC_JIFFIES[1], SystemPoller.getPidJiffies(new BufferedReader(new StringReader(perProcStats[1]))));
     }
@@ -109,8 +176,8 @@ public class SystemPollerTest {
     public void testCPUJiffies() {
         String line = "cpu1  102180864 789 56766899 12800020140 1654757 0 0";
         CpuJiffies n = new CpuJiffies(line);
-        assertThat(n.getCpuId(), is(1));
-        assertThat(n.getTotalJiffies(), is(12960623449L));
+        assertEquals(1, n.getCpuId());
+        assertEquals(12960623449L, n.getTotalJiffies());
     }
 
     @Test