Add test and benchmark for smaps parsing and avoid using regex.

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<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