Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit cb428553 authored by Dianne Hackborn's avatar Dianne Hackborn
Browse files

Fix issue #10948509: Crash in procstats when there is no data 

Not dealing with the case where there is a null list.

Also fixed some bugs I found while looking at this:

- When resetting the stats, we would use a newly computed time stamp
  for the total durations rather than the one we used to reset the
  proc/service entries.  This would result in them being able to be
  slightly > 100%.
- There was a bug in how we split a single process state into its
  per-package representation, where we would but the cloned process
  state into the new package's entry (instead of properly for its
  own package entry), to be immediately overwritten by the new
  process state we make for that package.  This could result in
  bad data for processes that have multiple packages.
- There was a bug in resetting service stats, where we wouldn't
  update the overall run timestamp, allowing that time to sometimes
  be > 100%.
- There was a bug in computing pss data for processes with multiple
  packages, where the pss data was not distributed across all of the
  activity per-package process states.
- There was a bug in computing the zram information that would cause
  it to compute the wrong value, and then never be displayed.

Finally a little code refactoring so that ProcessState and ServiceState
can now share a common implementation for the table of duration values.

Change-Id: I5e0f4e9107829b81f395dad9419c33257b4f8902
parent 398d7f61

core/java/com/android/internal/app/ProcessStats.java

+203 −140

File changed.

Preview size limit exceeded, changes collapsed.

core/jni/android_os_Debug.cpp

+9 −10
Original line number Diff line number Diff line
@@ -156,31 +156,32 @@ struct graphics_memory_pss 
 * Uses libmemtrack to retrieve graphics memory that the process is using.

 * Any graphics memory reported in /proc/pid/smaps is not included here.

 */

static int read_memtrack_memory(struct memtrack_proc* p, int pid, struct graphics_memory_pss* graphics_mem)

static int read_memtrack_memory(struct memtrack_proc* p, int pid,

        struct graphics_memory_pss* graphics_mem)

{

    int err = memtrack_proc_get(p, pid);

    if (err != 0) {

        ALOGE("failed to get memory consumption info: %d", err);

        ALOGW("failed to get memory consumption info: %d", err);

        return err;

    }



    ssize_t pss = memtrack_proc_graphics_pss(p);

    if (pss < 0) {

        ALOGE("failed to get graphics pss: %d", pss);

        ALOGW("failed to get graphics pss: %d", pss);

        return pss;

    }

    graphics_mem->graphics = pss / 1024;



    pss = memtrack_proc_gl_pss(p);

    if (pss < 0) {

        ALOGE("failed to get gl pss: %d", pss);

        ALOGW("failed to get gl pss: %d", pss);

        return pss;

    }

    graphics_mem->gl = pss / 1024;



    pss = memtrack_proc_other_pss(p);

    if (pss < 0) {

        ALOGE("failed to get other pss: %d", pss);

        ALOGW("failed to get other pss: %d", pss);

        return pss;

    }

    graphics_mem->other = pss / 1024;
@@ -199,7 +200,7 @@ static int read_memtrack_memory(int pid, struct graphics_memory_pss* graphics_me 


    struct memtrack_proc* p = memtrack_proc_new();

    if (p == NULL) {

        ALOGE("failed to create memtrack_proc");

        ALOGW("failed to create memtrack_proc");

        return -1;

    }
@@ -418,8 +419,6 @@ static void android_os_Debug_getDirtyPagesPid(JNIEnv *env, jobject clazz, 
        stats[HEAP_GL].privateDirty = graphics_mem.gl;

        stats[HEAP_OTHER_MEMTRACK].pss = graphics_mem.other;

        stats[HEAP_OTHER_MEMTRACK].privateDirty = graphics_mem.other;

    } else {

        ALOGE("Failed to read gpu memory");

    }



    for (int i=_NUM_CORE_HEAP; i<_NUM_EXCLUSIVE_HEAP; i++) {
@@ -623,7 +622,7 @@ static void android_os_Debug_getMemInfo(JNIEnv *env, jobject clazz, jlongArray o 
        close(fd);

        if (len > 0) {

            buffer[len] = 0;

            mem[MEMINFO_ZRAM_TOTAL] = atoll(buffer);

            mem[MEMINFO_ZRAM_TOTAL] = atoll(buffer)/1024;

        }

    }
@@ -633,7 +632,7 @@ static void android_os_Debug_getMemInfo(JNIEnv *env, jobject clazz, jlongArray o 
    }

    jlong* outArray = env->GetLongArrayElements(out, 0);

    if (outArray != NULL) {

        for (int i=0; i<maxNum && tags[i]; i++) {

        for (int i=0; i<maxNum; i++) {

            outArray[i] = mem[i];

        }

    }

services/java/com/android/server/am/ActivityManagerService.java

+5 −5
Original line number Diff line number Diff line
@@ -1689,7 +1689,7 @@ public final class ActivityManagerService extends ActivityManagerNative 
                                    && proc.pid == pid) {

                                num++;

                                proc.lastPssTime = SystemClock.uptimeMillis();

                                proc.baseProcessTracker.addPss(pss, tmp[0], true);

                                proc.baseProcessTracker.addPss(pss, tmp[0], true, proc.pkgList);

                                if (DEBUG_PSS) Slog.d(TAG, "PSS of " + proc.toShortString()

                                        + ": " + pss + " lastPss=" + proc.lastPss

                                        + " state=" + ProcessList.makeProcStateString(procState));
@@ -4278,7 +4278,7 @@ public final class ActivityManagerService extends ActivityManagerNative 
                    if (proc.thread != null && proc.setAdj == oomAdj) {

                        // Record this for posterity if the process has been stable.

                        proc.baseProcessTracker.addPss(infos[i].getTotalPss(),

                                infos[i].getTotalUss(), false);

                                infos[i].getTotalUss(), false, proc.pkgList);

                    }

                }

            }
@@ -4305,7 +4305,7 @@ public final class ActivityManagerService extends ActivityManagerNative 
                synchronized (this) {

                    if (proc.thread != null && proc.setAdj == oomAdj) {

                        // Record this for posterity if the process has been stable.

                        proc.baseProcessTracker.addPss(pss[i], tmpUss[0], false);

                        proc.baseProcessTracker.addPss(pss[i], tmpUss[0], false, proc.pkgList);

                    }

                }

            }
@@ -11748,7 +11748,7 @@ public final class ActivityManagerService extends ActivityManagerNative 
                synchronized (this) {

                    if (r.thread != null && oomAdj == r.getSetAdjWithServices()) {

                        // Record this for posterity if the process has been stable.

                        r.baseProcessTracker.addPss(myTotalPss, myTotalUss, true);

                        r.baseProcessTracker.addPss(myTotalPss, myTotalUss, true, r.pkgList);

                    }

                }










@@ -11909,7 +11909,7 @@ public final class ActivityManagerService extends ActivityManagerNative













                if (memInfo.getZramTotalSizeKb() != 0) {















                    if (!isCompact) {















                        pw.print("     ZRAM: "); pw.print(memInfo.getZramTotalSizeKb());













                                pw.print(" kB used for ");













                                pw.print(" kB physical used for ");















                                pw.print(memInfo.getSwapTotalSizeKb()















                                        - memInfo.getSwapFreeSizeKb());















                                pw.print(" kB in swap (");

































services/java/com/android/server/am/ProcessStatsService.java



+23
−11




























Original line number
Diff line number
Diff line








@@ -61,7 +61,6 @@ public final class ProcessStatsService extends IProcessStats.Stub {













    static final String STATE_FILE_SUFFIX = ".bin"; // Suffix to use for state filenames.















    static final String STATE_FILE_CHECKIN_SUFFIX = ".ci"; // State files that have checked in.















    static long WRITE_PERIOD = 30*60*1000;      // Write file every 30 minutes or so.













    static long COMMIT_PERIOD = 3*60*60*1000;  // Commit current stats every 3 hours.































    final ActivityManagerService mAm;















    final File mBaseDir;










@@ -160,7 +159,7 @@ public final class ProcessStatsService extends IProcessStats.Stub {













    public boolean shouldWriteNowLocked(long now) {















        if (now > (mLastWriteTime+WRITE_PERIOD)) {















            if (SystemClock.elapsedRealtime()













                    > (mProcessStats.mTimePeriodStartRealtime+COMMIT_PERIOD)) {













                    > (mProcessStats.mTimePeriodStartRealtime+ProcessStats.COMMIT_PERIOD)) {















                mCommitPending = true;















            }















            return true;










@@ -358,7 +357,7 @@ public final class ProcessStatsService extends IProcessStats.Stub {













            boolean sepScreenStates, int[] screenStates, boolean sepMemStates, int[] memStates,















            boolean sepProcStates, int[] procStates, long now, String reqPackage) {















        ArrayList<ProcessStats.ProcessState> procs = mProcessStats.collectProcessesLocked(













                screenStates, memStates, procStates, now, reqPackage);













                screenStates, memStates, procStates, procStates, now, reqPackage);















        if (procs.size() > 0) {















            if (header != null) {















                pw.println(header);










@@ -457,7 +456,7 @@ public final class ProcessStatsService extends IProcessStats.Stub {













            if (curTime < minTime) {















                // Need to add in older stats to reach desired time.















                ArrayList<String> files = getCommittedFiles(0, false, true);













                if (files.size() > 0) {













                if (files != null && files.size() > 0) {















                    current.setDataPosition(0);















                    ProcessStats stats = ProcessStats.CREATOR.createFromParcel(current);















                    current.recycle();










@@ -520,7 +519,7 @@ public final class ProcessStatsService extends IProcessStats.Stub {













    static private void dumpHelp(PrintWriter pw) {















        pw.println("Process stats (procstats) dump options:");















        pw.println("    [--checkin|-c|--csv] [--csv-screen] [--csv-proc] [--csv-mem]");













        pw.println("    [--details] [--full-details] [--current] [--one-day]");













        pw.println("    [--details] [--full-details] [--current] [--hours]");















        pw.println("    [--commit] [--reset] [--clear] [--write] [-h] [<package.name>]");















        pw.println("  --checkin: perform a checkin: print and delete old committed states.");















        pw.println("  --c: print only state in checkin format.");









@@ -532,7 +531,7 @@ public final class ProcessStatsService extends IProcessStats.Stub {













        pw.println("  --details: dump all execution details, not just summary.");















        pw.println("  --full-details: dump only detail information, for all saved state.");















        pw.println("  --current: only dump current state.");













        pw.println("  --one-day: dump stats aggregated across about one day.");













        pw.println("  --hours: aggregate over about N last hours.");















        pw.println("  --commit: commit current stats to disk and reset to start new stats.");















        pw.println("  --reset: reset current stats, without committing.");















        pw.println("  --clear: clear all stats; does both --reset and deletes old stats.");










@@ -562,7 +561,7 @@ public final class ProcessStatsService extends IProcessStats.Stub {













        boolean dumpDetails = false;















        boolean dumpFullDetails = false;















        boolean dumpAll = false;













        boolean oneDay = false;













        int aggregateHours = 0;















        String reqPackage = null;















        boolean csvSepScreenStats = false;















        int[] csvScreenStats = new int[] { ProcessStats.ADJ_SCREEN_OFF, ProcessStats.ADJ_SCREEN_ON};










@@ -632,8 +631,20 @@ public final class ProcessStatsService extends IProcessStats.Stub {













                    dumpDetails = true;















                } else if ("--full-details".equals(arg)) {















                    dumpFullDetails = true;













                } else if ("--one-day".equals(arg)) {













                    oneDay = true;













                } else if ("--hours".equals(arg)) {













                    i++;













                    if (i >= args.length) {













                        pw.println("Error: argument required for --hours");













                        dumpHelp(pw);













                        return;













                    }













                    try {













                        aggregateHours = Integer.parseInt(args[i]);













                    } catch (NumberFormatException e) {













                        pw.println("Error: --hours argument not an int -- " + args[i]);













                        dumpHelp(pw);













                        return;













                    }















                } else if ("--current".equals(arg)) {















                    currentOnly = true;















                } else if ("--commit".equals(arg)) {










@@ -750,8 +761,9 @@ public final class ProcessStatsService extends IProcessStats.Stub {













                */















            }















            return;













        } else if (oneDay) {













            ParcelFileDescriptor pfd = getStatsOverTime(24*60*60*1000);













        } else if (aggregateHours != 0) {













            ParcelFileDescriptor pfd = getStatsOverTime(aggregateHours*60*60*1000













                    - (ProcessStats.COMMIT_PERIOD/2));















            if (pfd == null) {















                pw.println("Unable to build stats!");















                return;