Merge "Fix: convert number of pages to bytes"

2 files changed, 44 insertions, 19 deletions

diff --git a/services/core/java/com/android/server/am/MemoryStatUtil.java b/services/core/java/com/android/server/am/MemoryStatUtil.java
index 228c71d91b8f..a8e1ccca8b9d 100644
--- a/services/core/java/com/android/server/am/MemoryStatUtil.java
+++ b/services/core/java/com/android/server/am/MemoryStatUtil.java
@@ -38,6 +38,7 @@ import java.util.regex.Pattern;
  */
 final class MemoryStatUtil {
     static final int BYTES_IN_KILOBYTE = 1024;
+    static final int PAGE_SIZE = 4096;
 
     private static final String TAG = TAG_WITH_CLASS_NAME ? "MemoryStatUtil" : TAG_AM;
 
@@ -68,7 +69,7 @@ final class MemoryStatUtil {
 
     private static final int PGFAULT_INDEX = 9;
     private static final int PGMAJFAULT_INDEX = 11;
-    private static final int RSS_IN_BYTES_INDEX = 23;
+    private static final int RSS_IN_PAGES_INDEX = 23;
 
     private MemoryStatUtil() {}
 
@@ -146,15 +147,15 @@ final class MemoryStatUtil {
         final MemoryStat memoryStat = new MemoryStat();
         Matcher m;
         m = PGFAULT.matcher(memoryStatContents);
-        memoryStat.pgfault = m.find() ? Long.valueOf(m.group(1)) : 0;
+        memoryStat.pgfault = m.find() ? Long.parseLong(m.group(1)) : 0;
         m = PGMAJFAULT.matcher(memoryStatContents);
-        memoryStat.pgmajfault = m.find() ? Long.valueOf(m.group(1)) : 0;
+        memoryStat.pgmajfault = m.find() ? Long.parseLong(m.group(1)) : 0;
         m = RSS_IN_BYTES.matcher(memoryStatContents);
-        memoryStat.rssInBytes = m.find() ? Long.valueOf(m.group(1)) : 0;
+        memoryStat.rssInBytes = m.find() ? Long.parseLong(m.group(1)) : 0;
         m = CACHE_IN_BYTES.matcher(memoryStatContents);
-        memoryStat.cacheInBytes = m.find() ? Long.valueOf(m.group(1)) : 0;
+        memoryStat.cacheInBytes = m.find() ? Long.parseLong(m.group(1)) : 0;
         m = SWAP_IN_BYTES.matcher(memoryStatContents);
-        memoryStat.swapInBytes = m.find() ? Long.valueOf(m.group(1)) : 0;
+        memoryStat.swapInBytes = m.find() ? Long.parseLong(m.group(1)) : 0;
         return memoryStat;
     }
 
@@ -163,7 +164,12 @@ final class MemoryStatUtil {
         if (memoryMaxUsageContents == null || memoryMaxUsageContents.isEmpty()) {
             return 0;
         }
-        return Long.valueOf(memoryMaxUsageContents);
+        try {
+            return Long.parseLong(memoryMaxUsageContents);
+        } catch (NumberFormatException e) {
+            Slog.e(TAG, "Failed to parse value", e);
+            return 0;
+        }
     }
 
     /**
@@ -181,11 +187,16 @@ final class MemoryStatUtil {
             return null;
         }
 
-        final MemoryStat memoryStat = new MemoryStat();
-        memoryStat.pgfault = Long.valueOf(splits[PGFAULT_INDEX]);
-        memoryStat.pgmajfault = Long.valueOf(splits[PGMAJFAULT_INDEX]);
-        memoryStat.rssInBytes = Long.valueOf(splits[RSS_IN_BYTES_INDEX]);
-        return memoryStat;
+        try {
+            final MemoryStat memoryStat = new MemoryStat();
+            memoryStat.pgfault = Long.parseLong(splits[PGFAULT_INDEX]);
+            memoryStat.pgmajfault = Long.parseLong(splits[PGMAJFAULT_INDEX]);
+            memoryStat.rssInBytes = Long.parseLong(splits[RSS_IN_PAGES_INDEX]) * PAGE_SIZE;
+            return memoryStat;
+        } catch (NumberFormatException e) {
+            Slog.e(TAG, "Failed to parse value", e);
+            return null;
+        }
     }
 
     /**
@@ -199,7 +210,7 @@ final class MemoryStatUtil {
         }
         Matcher m = RSS_HIGH_WATERMARK_IN_BYTES.matcher(procStatusContents);
         // Convert value read from /proc/pid/status from kilobytes to bytes.
-        return m.find() ? Long.valueOf(m.group(1)) * BYTES_IN_KILOBYTE : 0;
+        return m.find() ? Long.parseLong(m.group(1)) * BYTES_IN_KILOBYTE : 0;
     }
 
     /**
diff --git a/services/tests/servicestests/src/com/android/server/am/MemoryStatUtilTest.java b/services/tests/servicestests/src/com/android/server/am/MemoryStatUtilTest.java
index e8a824a12300..9a283febe906 100644
--- a/services/tests/servicestests/src/com/android/server/am/MemoryStatUtilTest.java
+++ b/services/tests/servicestests/src/com/android/server/am/MemoryStatUtilTest.java
@@ -18,6 +18,7 @@ package com.android.server.am;
 
 import static com.android.server.am.MemoryStatUtil.BYTES_IN_KILOBYTE;
 import static com.android.server.am.MemoryStatUtil.MemoryStat;
+import static com.android.server.am.MemoryStatUtil.PAGE_SIZE;
 import static com.android.server.am.MemoryStatUtil.parseMemoryMaxUsageFromMemCg;
 import static com.android.server.am.MemoryStatUtil.parseMemoryStatFromMemcg;
 import static com.android.server.am.MemoryStatUtil.parseMemoryStatFromProcfs;
@@ -32,6 +33,8 @@ import androidx.test.runner.AndroidJUnit4;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import java.util.Collections;
+
 @RunWith(AndroidJUnit4.class)
 @SmallTest
 public class MemoryStatUtilTest {
@@ -95,7 +98,7 @@ public class MemoryStatUtilTest {
             "0",
             "2206",
             "1257177088",
-            "3", // this is rss in bytes
+            "3", // this is RSS (number of pages)
             "4294967295",
             "2936971264",
             "2936991289",
@@ -173,7 +176,7 @@ public class MemoryStatUtilTest {
         + "nonvoluntary_ctxt_switches:\t104\n";
 
     @Test
-    public void testParseMemoryStatFromMemcg_parsesCorrectValues() throws Exception {
+    public void testParseMemoryStatFromMemcg_parsesCorrectValues() {
         MemoryStat stat = parseMemoryStatFromMemcg(MEMORY_STAT_CONTENTS);
         assertEquals(1, stat.pgfault);
         assertEquals(2, stat.pgmajfault);
@@ -183,7 +186,7 @@ public class MemoryStatUtilTest {
     }
 
     @Test
-    public void testParseMemoryStatFromMemcg_emptyMemoryStatContents() throws Exception {
+    public void testParseMemoryStatFromMemcg_emptyMemoryStatContents() {
         MemoryStat stat = parseMemoryStatFromMemcg("");
         assertNull(stat);
 
@@ -204,17 +207,22 @@ public class MemoryStatUtilTest {
     }
 
     @Test
-    public void testParseMemoryStatFromProcfs_parsesCorrectValues() throws Exception {
+    public void testParseMemoryMaxUsageFromMemCg_incorrectValue() {
+        assertEquals(0, parseMemoryMaxUsageFromMemCg("memory"));
+    }
+
+    @Test
+    public void testParseMemoryStatFromProcfs_parsesCorrectValues() {
         MemoryStat stat = parseMemoryStatFromProcfs(PROC_STAT_CONTENTS);
         assertEquals(1, stat.pgfault);
         assertEquals(2, stat.pgmajfault);
-        assertEquals(3, stat.rssInBytes);
+        assertEquals(3 * PAGE_SIZE, stat.rssInBytes);
         assertEquals(0, stat.cacheInBytes);
         assertEquals(0, stat.swapInBytes);
     }
 
     @Test
-    public void testParseMemoryStatFromProcfs_emptyContents() throws Exception {
+    public void testParseMemoryStatFromProcfs_emptyContents() {
         MemoryStat stat = parseMemoryStatFromProcfs("");
         assertNull(stat);
 
@@ -223,6 +231,12 @@ public class MemoryStatUtilTest {
     }
 
     @Test
+    public void testParseMemoryStatFromProcfs_invalidValue() {
+        String contents = String.join(" ", Collections.nCopies(24, "memory"));
+        assertNull(parseMemoryStatFromProcfs(contents));
+    }
+
+    @Test
     public void testParseVmHWMFromProcfs_parsesCorrectValue() {
         assertEquals(137668, parseVmHWMFromProcfs(PROC_STATUS_CONTENTS) / BYTES_IN_KILOBYTE);
     }