From 3176ebcd96171ddf4bc9f5dd76f8fb9754dae738 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 15 Dec 2021 18:24:57 +0100 Subject: Use the simple computeIfAbsent method. --- .../java/ai/vespa/metricsproxy/core/VespaMetrics.java | 16 +++------------- 1 file changed, 3 insertions(+), 13 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 extractConsumers(Set configuredConsumers) { - Set 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 ml = new ArrayList<>(); - ml.add(metric); - aggregated.put(aggregationKey, ml); - } + aggregated.computeIfAbsent(aggregationKey, key -> new ArrayList<>()).add(metric); } } -- cgit v1.2.3 From 5aa228307c58ebafd32155d8ac7a0729d58bb874 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 15 Dec 2021 18:31:13 +0100 Subject: Unify on Map.of/copyOf --- .../src/main/java/ai/vespa/metricsproxy/metric/Metric.java | 10 ++++------ .../main/java/ai/vespa/metricsproxy/service/MetricsParser.java | 5 ++--- 2 files changed, 6 insertions(+), 9 deletions(-) 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 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 dim = Collections.emptyMap(); + Map 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()); } -- cgit v1.2.3 From 1427b78c8eb87229ec76b3be2d7f31acf5301d1c Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 15 Dec 2021 18:47:41 +0100 Subject: Only take regex cost when necessary --- .../src/main/java/ai/vespa/metricsproxy/service/SystemPoller.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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..7ab3bf2e5a6 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 @@ -119,11 +119,13 @@ public class SystemPoller { 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:")) { + String[] elems = line.split("\\s+"); size[memoryTypeResident] += Long.parseLong(elems[1]) * 1024; } else if (line.startsWith("Size:")) { + String[] elems = line.split("\\s+"); size[memoryTypeVirtual] += Long.parseLong(elems[1]) * 1024; } } -- cgit v1.2.3 From 47aa61f921242123db88c4f5334ecce119cfb209 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 15 Dec 2021 20:54:42 +0100 Subject: Add test and benchmark for smaps parsing and avoid using regex. --- .../vespa/metricsproxy/service/SystemPoller.java | 41 +++++----- .../metricsproxy/service/SystemPollerTest.java | 87 +++++++++++++++++++--- 2 files changed, 101 insertions(+), 27 deletions(-) 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 7ab3bf2e5a6..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,26 +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) { - - /* Memory size is given in kB - convert to bytes by multiply with 1024*/ - if (line.startsWith("Rss:")) { - String[] elems = line.split("\\s+"); - size[memoryTypeResident] += Long.parseLong(elems[1]) * 1024; - } else if (line.startsWith("Size:")) { - String[] elems = line.split("\\s+"); - 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 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 -- cgit v1.2.3