OSDN Git Service

Save dumpstate duration stats so it can be tuned over time.
authorFelipe Leme <felipeal@google.com>
Fri, 4 Nov 2016 01:12:22 +0000 (18:12 -0700)
committerFelipe Leme <felipeal@google.com>
Thu, 10 Nov 2016 22:02:08 +0000 (14:02 -0800)
Fixes: 26373682
Test: dumpstate_test passes and manual verification

Change-Id: I72a308bfb314e157b12746c1be2c33833bdf9d8a

15 files changed:
cmds/dumpstate/DumpstateService.cpp
cmds/dumpstate/dumpstate.cpp
cmds/dumpstate/dumpstate.h
cmds/dumpstate/testdata/empty-file.txt [new file with mode: 0644]
cmds/dumpstate/testdata/stats-invalid-1st-NAN.txt [new file with mode: 0644]
cmds/dumpstate/testdata/stats-invalid-1st-negative.txt [new file with mode: 0644]
cmds/dumpstate/testdata/stats-invalid-1st-too-big.txt [new file with mode: 0644]
cmds/dumpstate/testdata/stats-invalid-2nd-NAN.txt [new file with mode: 0644]
cmds/dumpstate/testdata/stats-invalid-2nd-negative.txt [new file with mode: 0644]
cmds/dumpstate/testdata/stats-invalid-2nd-too-big.txt [new file with mode: 0644]
cmds/dumpstate/testdata/stats-invalid-both-NAN.txt [new file with mode: 0644]
cmds/dumpstate/testdata/stats-one-run-no-newline.txt [new file with mode: 0644]
cmds/dumpstate/testdata/stats-two-runs.txt [new file with mode: 0644]
cmds/dumpstate/tests/dumpstate_test.cpp
cmds/dumpstate/utils.cpp

index ce78ec6..a2a9018 100644 (file)
@@ -69,9 +69,10 @@ binder::Status DumpstateService::setListener(const std::string& name,
 }
 
 status_t DumpstateService::dump(int fd, const Vector<String16>&) {
-    dprintf(fd, "id: %lu\n", ds_.id_);
+    dprintf(fd, "id: %d\n", ds_.id_);
     dprintf(fd, "pid: %d\n", ds_.pid_);
-    dprintf(fd, "progress: %d / %d\n", ds_.progress_, ds_.weight_total_);
+    dprintf(fd, "progress:\n");
+    ds_.progress_->Dump(fd, "  ");
     dprintf(fd, "args: %s\n", ds_.args_.c_str());
     dprintf(fd, "extra_options: %s\n", ds_.extra_options_.c_str());
     dprintf(fd, "version: %s\n", ds_.version_.c_str());
@@ -79,7 +80,8 @@ status_t DumpstateService::dump(int fd, const Vector<String16>&) {
     dprintf(fd, "screenshot_path: %s\n", ds_.screenshot_path_.c_str());
     dprintf(fd, "log_path: %s\n", ds_.log_path_.c_str());
     dprintf(fd, "tmp_path: %s\n", ds_.tmp_path_.c_str());
-    dprintf(fd, "path: %s\n", ds_.extra_options_.c_str());
+    dprintf(fd, "path: %s\n", ds_.path_.c_str());
+    dprintf(fd, "extra_options: %s\n", ds_.extra_options_.c_str());
     dprintf(fd, "base_name: %s\n", ds_.base_name_.c_str());
     dprintf(fd, "name: %s\n", ds_.name_.c_str());
     dprintf(fd, "now: %ld\n", ds_.now_);
index 80a1b07..6201735 100644 (file)
@@ -683,7 +683,7 @@ void Dumpstate::PrintHeader() const {
     JustDumpFile("", "/proc/version");
     printf("Command line: %s\n", strtok(cmdline_buf, "\n"));
     printf("Bugreport format version: %s\n", version_.c_str());
-    printf("Dumpstate info: id=%lu pid=%d dry_run=%d args=%s extra_options=%s\n", id_, pid_,
+    printf("Dumpstate info: id=%d pid=%d dry_run=%d args=%s extra_options=%s\n", id_, pid_,
            dry_run_, args_.c_str(), extra_options_.c_str());
     printf("\n");
 }
@@ -1154,10 +1154,10 @@ static void dumpstate() {
     DumpModemLogs();
 
     printf("========================================================\n");
-    printf("== Final progress (pid %d): %d/%d (originally %d)\n", ds.pid_, ds.progress_,
-           ds.weight_total_, WEIGHT_TOTAL);
+    printf("== Final progress (pid %d): %d/%d (estimated %d)\n", ds.pid_, ds.progress_->Get(),
+           ds.progress_->GetMax(), ds.progress_->GetInitialMax());
     printf("========================================================\n");
-    printf("== dumpstate: done (id %lu)\n", ds.id_);
+    printf("== dumpstate: done (id %d)\n", ds.id_);
     printf("========================================================\n");
 }
 
@@ -1222,7 +1222,7 @@ bool Dumpstate::FinishZipFile() {
     char date[80];
     time_t the_real_now_please_stand_up = time(nullptr);
     strftime(date, sizeof(date), "%Y/%m/%d %H:%M:%S", localtime(&the_real_now_please_stand_up));
-    MYLOGD("dumpstate id %lu finished around %s (%ld s)\n", ds.id_, date,
+    MYLOGD("dumpstate id %d finished around %s (%ld s)\n", ds.id_, date,
            the_real_now_please_stand_up - ds.now_);
 
     if (!ds.AddZipEntry(entry_name, tmp_path_)) {
@@ -1418,8 +1418,17 @@ int main(int argc, char *argv[]) {
         exit(0);
     }
 
+    /* redirect output if needed */
+    bool is_redirecting = !use_socket && use_outfile;
+
+    // TODO: temporarily set progress until it's part of the Dumpstate constructor
+    std::string stats_path =
+        is_redirecting ? android::base::StringPrintf("%s/dumpstate-stats.txt", dirname(use_outfile))
+                       : "";
+    ds.progress_.reset(new Progress(stats_path));
+
     /* gets the sequential id */
-    int last_id = android::base::GetIntProperty(PROPERTY_LAST_ID, 0);
+    uint32_t last_id = android::base::GetIntProperty(PROPERTY_LAST_ID, 0);
     ds.id_ = ++last_id;
     android::base::SetProperty(PROPERTY_LAST_ID, std::to_string(last_id));
 
@@ -1445,7 +1454,7 @@ int main(int argc, char *argv[]) {
         MYLOGI("Running on dry-run mode (to disable it, call 'setprop dumpstate.dry_run false')\n");
     }
 
-    MYLOGI("dumpstate info: id=%lu, args='%s', extra_options= %s)\n", ds.id_, ds.args_.c_str(),
+    MYLOGI("dumpstate info: id=%d, args='%s', extra_options= %s)\n", ds.id_, ds.args_.c_str(),
            ds.extra_options_.c_str());
 
     MYLOGI("bugreport format version: %s\n", ds.version_.c_str());
@@ -1464,9 +1473,6 @@ int main(int argc, char *argv[]) {
         ds.update_progress_ = 1;
     }
 
-    /* redirect output if needed */
-    bool is_redirecting = !use_socket && use_outfile;
-
     if (is_redirecting) {
         ds.bugreport_dir_ = dirname(use_outfile);
         ds.base_name_ = basename(use_outfile);
@@ -1517,7 +1523,7 @@ int main(int argc, char *argv[]) {
                      "--es", "android.intent.extra.NAME", ds.name_,
                      "--ei", "android.intent.extra.ID", std::to_string(ds.id_),
                      "--ei", "android.intent.extra.PID", std::to_string(ds.pid_),
-                     "--ei", "android.intent.extra.MAX", std::to_string(WEIGHT_TOTAL),
+                     "--ei", "android.intent.extra.MAX", std::to_string(ds.progress_->GetMax()),
                 };
                 // clang-format on
                 send_broadcast("android.intent.action.BUGREPORT_STARTED", am_args);
@@ -1720,7 +1726,7 @@ int main(int argc, char *argv[]) {
                  "--receiver-permission", "android.permission.DUMP", "--receiver-foreground",
                  "--ei", "android.intent.extra.ID", std::to_string(ds.id_),
                  "--ei", "android.intent.extra.PID", std::to_string(ds.pid_),
-                 "--ei", "android.intent.extra.MAX", std::to_string(ds.weight_total_),
+                 "--ei", "android.intent.extra.MAX", std::to_string(ds.progress_->GetMax()),
                  "--es", "android.intent.extra.BUGREPORT", ds.path_,
                  "--es", "android.intent.extra.DUMPSTATE_LOG", ds.log_path_
             };
@@ -1743,8 +1749,10 @@ int main(int argc, char *argv[]) {
         }
     }
 
-    MYLOGD("Final progress: %d/%d (originally %d)\n", ds.progress_, ds.weight_total_, WEIGHT_TOTAL);
-    MYLOGI("done (id %lu)\n", ds.id_);
+    MYLOGD("Final progress: %d/%d (estimated %d)\n", ds.progress_->Get(), ds.progress_->GetMax(),
+           ds.progress_->GetInitialMax());
+    ds.progress_->Save();
+    MYLOGI("done (id %d)\n", ds.id_);
 
     if (is_redirecting) {
         fclose(stderr);
index 0680db2..9124f94 100644 (file)
@@ -184,20 +184,65 @@ class CommandOptions {
 };
 
 /*
- * Estimated total weight of bugreport generation.
+ * Keeps track of current progress and estimated max, saving stats on file to tune up future runs.
  *
- * Each section contributes to the total weight by an individual weight, so the overall progress
- * can be calculated by dividing the all completed weight by the total weight.
+ * Each `dumpstate` section contributes to the total weight by an individual weight, so the overall
+ * progress can be calculated by dividing the estimate max progress by the current progress.
  *
- * This value is defined empirically and it need to be adjusted as more sections are added.
+ * The estimated max progress is initially set to a value (`kDefaultMax) defined empirically, but
+ * it's adjusted after each dumpstate run by storing the average duration in a file.
  *
- * It does not need to match the exact sum of all sections, but ideally it should to be slight more
- * than such sum: a value too high will cause the bugreport to finish before the user expected (for
- * example, jumping from 70% to 100%), while a value too low will cause the progress to get stuck
- * at an almost-finished value (like 99%) for a while.
  */
-// TODO: move to dumpstate.cpp / utils.cpp once it's used in just one file
-static const int WEIGHT_TOTAL = 6500;
+class Progress {
+    friend class ProgressTest;
+    friend class DumpstateTest;
+
+  public:
+    /*
+     * Default estimation of the max duration of a bugreport generation.
+     *
+     * It does not need to match the exact sum of all sections, but ideally it should to be slight
+     * more than such sum: a value too high will cause the bugreport to finish before the user
+     * expected (for example, jumping from 70% to 100%), while a value too low will cause the
+     * progress to get stuck at an almost-finished value (like 99%) for a while.
+     *
+     * This constant is only used when the average duration from previous runs cannot be used.
+     */
+    static const int kDefaultMax;
+
+    Progress(const std::string& path = "");
+
+    // Gets the current progress.
+    int32_t Get() const;
+
+    // Gets the current estimated max progress.
+    int32_t GetMax() const;
+
+    // Gets the initial estimated max progress.
+    int32_t GetInitialMax() const;
+
+    // Increments progress (ignored if not positive).
+    // Returns `true` if the max progress increased as well.
+    bool Inc(int32_t delta);
+
+    // Persist the stats.
+    void Save();
+
+    void Dump(int fd, const std::string& prefix) const;
+
+  private:
+    Progress(int32_t initial_max, float growth_factor,
+             const std::string& path = "");                                // Used by test cases.
+    Progress(int32_t initial_max, int32_t progress, float growth_factor);  // Used by test cases.
+    void Load();
+    int32_t initial_max_;
+    int32_t progress_;
+    int32_t max_;
+    float growth_factor_;
+    int32_t n_runs_;
+    int32_t average_max_;
+    const std::string& path_;
+};
 
 /*
  * List of supported zip format versions.
@@ -310,12 +355,17 @@ class Dumpstate {
      */
     void TakeScreenshot(const std::string& path = "");
 
-    // TODO: members below should be private once refactor is finished
+    /////////////////////////////////////////////////////////////////////
+    // TODO: members below should be private once refactor is finished //
+    /////////////////////////////////////////////////////////////////////
+
+    // TODO: temporary method until Dumpstate object is properly set
+    void SetProgress(std::unique_ptr<Progress> progress);
 
     /*
      * Updates the overall progress of the bugreport generation by the given weight increment.
      */
-    void UpdateProgress(int delta);
+    void UpdateProgress(int32_t delta);
 
     /* Prints the dumpstate header on `stdout`. */
     void PrintHeader() const;
@@ -332,7 +382,7 @@ class Dumpstate {
     // TODO: initialize fields on constructor
 
     // dumpstate id - unique after each device reboot.
-    unsigned long id_;
+    uint32_t id_;
 
     // dumpstate pid
     pid_t pid_;
@@ -343,11 +393,7 @@ class Dumpstate {
     // Whether it should take an screenshot earlier in the process.
     bool do_early_screenshot_ = false;
 
-    // Currrent progress.
-    int progress_ = 0;
-
-    // Total estimated progress.
-    int weight_total_ = WEIGHT_TOTAL;
+    std::unique_ptr<Progress> progress_;
 
     // When set, defines a socket file-descriptor use to report progress to bugreportz.
     int control_socket_fd_ = -1;
diff --git a/cmds/dumpstate/testdata/empty-file.txt b/cmds/dumpstate/testdata/empty-file.txt
new file mode 100644 (file)
index 0000000..e69de29
diff --git a/cmds/dumpstate/testdata/stats-invalid-1st-NAN.txt b/cmds/dumpstate/testdata/stats-invalid-1st-NAN.txt
new file mode 100644 (file)
index 0000000..dad9fe8
--- /dev/null
@@ -0,0 +1 @@
+SIX_SIX_SIX 42
diff --git a/cmds/dumpstate/testdata/stats-invalid-1st-negative.txt b/cmds/dumpstate/testdata/stats-invalid-1st-negative.txt
new file mode 100644 (file)
index 0000000..4facef9
--- /dev/null
@@ -0,0 +1 @@
+-666 42
diff --git a/cmds/dumpstate/testdata/stats-invalid-1st-too-big.txt b/cmds/dumpstate/testdata/stats-invalid-1st-too-big.txt
new file mode 100644 (file)
index 0000000..42508f1
--- /dev/null
@@ -0,0 +1 @@
+4815162342 42
diff --git a/cmds/dumpstate/testdata/stats-invalid-2nd-NAN.txt b/cmds/dumpstate/testdata/stats-invalid-2nd-NAN.txt
new file mode 100644 (file)
index 0000000..a23ba2c
--- /dev/null
@@ -0,0 +1 @@
+666 FORTY_TWO
diff --git a/cmds/dumpstate/testdata/stats-invalid-2nd-negative.txt b/cmds/dumpstate/testdata/stats-invalid-2nd-negative.txt
new file mode 100644 (file)
index 0000000..dd529b4
--- /dev/null
@@ -0,0 +1 @@
+666 -42
diff --git a/cmds/dumpstate/testdata/stats-invalid-2nd-too-big.txt b/cmds/dumpstate/testdata/stats-invalid-2nd-too-big.txt
new file mode 100644 (file)
index 0000000..b148b46
--- /dev/null
@@ -0,0 +1 @@
+666 4815162342
diff --git a/cmds/dumpstate/testdata/stats-invalid-both-NAN.txt b/cmds/dumpstate/testdata/stats-invalid-both-NAN.txt
new file mode 100644 (file)
index 0000000..4a9466d
--- /dev/null
@@ -0,0 +1 @@
+N_RUNS AVERAGE
\ No newline at end of file
diff --git a/cmds/dumpstate/testdata/stats-one-run-no-newline.txt b/cmds/dumpstate/testdata/stats-one-run-no-newline.txt
new file mode 100644 (file)
index 0000000..0aef60c
--- /dev/null
@@ -0,0 +1 @@
+1 10
\ No newline at end of file
diff --git a/cmds/dumpstate/testdata/stats-two-runs.txt b/cmds/dumpstate/testdata/stats-two-runs.txt
new file mode 100644 (file)
index 0000000..9af1233
--- /dev/null
@@ -0,0 +1 @@
+2 15
index d692fda..2fcf321 100644 (file)
@@ -63,11 +63,48 @@ class DumpstateListenerMock : public IDumpstateListener {
     MOCK_METHOD0(onAsBinder, IBinder*());
 };
 
-class DumpstateTest : public Test {
+// Base class for all tests in this file
+class DumpstateBaseTest : public Test {
+  protected:
+    const std::string kTestPath = dirname(android::base::GetExecutablePath().c_str());
+    const std::string kFixturesPath = kTestPath + "/../dumpstate_test_fixture/";
+    const std::string kTestDataPath = kFixturesPath + "/testdata/";
+    const std::string kSimpleCommand = kFixturesPath + "dumpstate_test_fixture";
+    const std::string kEchoCommand = "/system/bin/echo";
+
+    /*
+     * Copies a text file fixture to a temporary file, returning it's path.
+     *
+     * Useful in cases where the test case changes the content of the tile.
+     */
+    std::string CopyTextFileFixture(const std::string& relative_name) {
+        std::string from = kTestDataPath + relative_name;
+        // Not using TemporaryFile because it's deleted at the end, and it's useful to keep it
+        // around for poking when the test fails.
+        std::string to = kTestDataPath + relative_name + ".tmp";
+        ALOGD("CopyTextFileFixture: from %s to %s\n", from.c_str(), to.c_str());
+        android::base::RemoveFileIfExists(to);
+        CopyTextFile(from, to);
+        return to.c_str();
+    }
+
+  private:
+    // Need a function that returns void to use assertions -
+    // https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#assertion-placement
+    void CopyTextFile(const std::string& from, const std::string& to) {
+        std::string content;
+        ASSERT_TRUE(android::base::ReadFileToString(from, &content)) << "could not read from "
+                                                                     << from;
+        ASSERT_TRUE(android::base::WriteStringToFile(content, to)) << "could not write to " << to;
+    }
+};
+
+class DumpstateTest : public DumpstateBaseTest {
   public:
     void SetUp() {
         SetDryRun(false);
         SetBuildType(android::base::GetProperty("ro.build.type", "(unknown)"));
+        ds.progress_.reset(new Progress());
         ds.update_progress_ = false;
     }
 
@@ -112,6 +149,11 @@ class DumpstateTest : public Test {
         ASSERT_EQ(2000, (int)uid);
     }
 
+    void SetProgress(long progress, long initial_max) {
+        ds.update_progress_ = true;
+        ds.progress_.reset(new Progress(initial_max, progress, 1.2));
+    }
+
     // TODO: remove when progress is set by Binder callbacks.
     void AssertSystemProperty(const std::string& key, const std::string& expected_value) {
         std::string actualValue = android::base::GetProperty(key, "not set");
@@ -119,56 +161,48 @@ class DumpstateTest : public Test {
     }
 
     // TODO: remove when progress is set by Binder callbacks.
-    std::string GetProgressMessageAndAssertSystemProperties(int progress, int weight_total,
-                                                            int old_weight_total = 0) {
-        EXPECT_EQ(progress, ds.progress_) << "invalid progress";
-        EXPECT_EQ(weight_total, ds.weight_total_) << "invalid weight_total";
+    std::string GetProgressMessageAndAssertSystemProperties(int progress, int max, int old_max = 0) {
+        EXPECT_EQ(progress, ds.progress_->Get()) << "invalid progress";
+        EXPECT_EQ(max, ds.progress_->GetMax()) << "invalid max";
 
         AssertSystemProperty(android::base::StringPrintf("dumpstate.%d.progress", getpid()),
                              std::to_string(progress));
 
-        bool max_increased = old_weight_total > 0;
+        bool max_increased = old_max > 0;
 
         std::string adjustment_message = "";
         if (max_increased) {
             AssertSystemProperty(android::base::StringPrintf("dumpstate.%d.max", getpid()),
-                                 std::to_string(weight_total));
-            adjustment_message = android::base::StringPrintf(
-                "Adjusting total weight from %d to %d\n", old_weight_total, weight_total);
+                                 std::to_string(max));
+            adjustment_message =
+                android::base::StringPrintf("Adjusting max progress from %d to %d\n", old_max, max);
         }
 
         return android::base::StringPrintf("%sSetting progress (dumpstate.%d.progress): %d/%d\n",
-                                           adjustment_message.c_str(), getpid(), progress,
-                                           weight_total);
+                                           adjustment_message.c_str(), getpid(), progress, max);
     }
 
-    std::string GetProgressMessage(const std::string& listener_name, int progress, int weight_total,
-                                   int old_weight_total = 0) {
-        EXPECT_EQ(progress, ds.progress_) << "invalid progress";
-        EXPECT_EQ(weight_total, ds.weight_total_) << "invalid weight_total";
+    std::string GetProgressMessage(const std::string& listener_name, int progress, int max,
+                                   int old_max = 0) {
+        EXPECT_EQ(progress, ds.progress_->Get()) << "invalid progress";
+        EXPECT_EQ(max, ds.progress_->GetMax()) << "invalid max";
 
-        bool max_increased = old_weight_total > 0;
+        bool max_increased = old_max > 0;
 
         std::string adjustment_message = "";
         if (max_increased) {
-            adjustment_message = android::base::StringPrintf(
-                "Adjusting total weight from %d to %d\n", old_weight_total, weight_total);
+            adjustment_message =
+                android::base::StringPrintf("Adjusting max progress from %d to %d\n", old_max, max);
         }
 
         return android::base::StringPrintf("%sSetting progress (%s): %d/%d\n",
                                            adjustment_message.c_str(), listener_name.c_str(),
-                                           progress, weight_total);
+                                           progress, max);
     }
 
     // `stdout` and `stderr` from the last command ran.
     std::string out, err;
 
-    std::string test_path = dirname(android::base::GetExecutablePath().c_str());
-    std::string fixtures_path = test_path + "/../dumpstate_test_fixture/";
-    std::string test_data_path = fixtures_path + "/testdata/";
-    std::string simple_command = fixtures_path + "dumpstate_test_fixture";
-    std::string echo_command = "/system/bin/echo";
-
     Dumpstate& ds = Dumpstate::GetInstance();
 };
 
@@ -177,52 +211,52 @@ TEST_F(DumpstateTest, RunCommandNoArgs) {
 }
 
 TEST_F(DumpstateTest, RunCommandNoTitle) {
-    EXPECT_EQ(0, RunCommand("", {simple_command}));
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand}));
     EXPECT_THAT(out, StrEq("stdout\n"));
     EXPECT_THAT(err, StrEq("stderr\n"));
 }
 
 TEST_F(DumpstateTest, RunCommandWithTitle) {
-    EXPECT_EQ(0, RunCommand("I AM GROOT", {simple_command}));
+    EXPECT_EQ(0, RunCommand("I AM GROOT", {kSimpleCommand}));
     EXPECT_THAT(err, StrEq("stderr\n"));
     // We don't know the exact duration, so we check the prefix and suffix
     EXPECT_THAT(out,
-                StartsWith("------ I AM GROOT (" + simple_command + ") ------\nstdout\n------"));
+                StartsWith("------ I AM GROOT (" + kSimpleCommand + ") ------\nstdout\n------"));
     EXPECT_THAT(out, EndsWith("s was the duration of 'I AM GROOT' ------\n"));
 }
 
 TEST_F(DumpstateTest, RunCommandWithLoggingMessage) {
     EXPECT_EQ(
-        0, RunCommand("", {simple_command},
+        0, RunCommand("", {kSimpleCommand},
                       CommandOptions::WithTimeout(10).Log("COMMAND, Y U NO LOG FIRST?").Build()));
     EXPECT_THAT(out, StrEq("stdout\n"));
     EXPECT_THAT(err, StrEq("COMMAND, Y U NO LOG FIRST?stderr\n"));
 }
 
 TEST_F(DumpstateTest, RunCommandRedirectStderr) {
-    EXPECT_EQ(0, RunCommand("", {simple_command},
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand},
                             CommandOptions::WithTimeout(10).RedirectStderr().Build()));
     EXPECT_THAT(out, IsEmpty());
     EXPECT_THAT(err, StrEq("stdout\nstderr\n"));
 }
 
 TEST_F(DumpstateTest, RunCommandWithOneArg) {
-    EXPECT_EQ(0, RunCommand("", {echo_command, "one"}));
+    EXPECT_EQ(0, RunCommand("", {kEchoCommand, "one"}));
     EXPECT_THAT(err, IsEmpty());
     EXPECT_THAT(out, StrEq("one\n"));
 }
 
 TEST_F(DumpstateTest, RunCommandWithMultipleArgs) {
-    EXPECT_EQ(0, RunCommand("", {echo_command, "one", "is", "the", "loniest", "number"}));
+    EXPECT_EQ(0, RunCommand("", {kEchoCommand, "one", "is", "the", "loniest", "number"}));
     EXPECT_THAT(err, IsEmpty());
     EXPECT_THAT(out, StrEq("one is the loniest number\n"));
 }
 
 TEST_F(DumpstateTest, RunCommandDryRun) {
     SetDryRun(true);
-    EXPECT_EQ(0, RunCommand("I AM GROOT", {simple_command}));
+    EXPECT_EQ(0, RunCommand("I AM GROOT", {kSimpleCommand}));
     // We don't know the exact duration, so we check the prefix and suffix
-    EXPECT_THAT(out, StartsWith("------ I AM GROOT (" + simple_command +
+    EXPECT_THAT(out, StartsWith("------ I AM GROOT (" + kSimpleCommand +
                                 ") ------\n\t(skipped on dry run)\n------"));
     EXPECT_THAT(out, EndsWith("s was the duration of 'I AM GROOT' ------\n"));
     EXPECT_THAT(err, IsEmpty());
@@ -230,14 +264,14 @@ TEST_F(DumpstateTest, RunCommandDryRun) {
 
 TEST_F(DumpstateTest, RunCommandDryRunNoTitle) {
     SetDryRun(true);
-    EXPECT_EQ(0, RunCommand("", {simple_command}));
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand}));
     EXPECT_THAT(out, IsEmpty());
     EXPECT_THAT(err, IsEmpty());
 }
 
 TEST_F(DumpstateTest, RunCommandDryRunAlways) {
     SetDryRun(true);
-    EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(10).Always().Build()));
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(10).Always().Build()));
     EXPECT_THAT(out, StrEq("stdout\n"));
     EXPECT_THAT(err, StrEq("stderr\n"));
 }
@@ -249,28 +283,28 @@ TEST_F(DumpstateTest, RunCommandNotFound) {
 }
 
 TEST_F(DumpstateTest, RunCommandFails) {
-    EXPECT_EQ(42, RunCommand("", {simple_command, "--exit", "42"}));
-    EXPECT_THAT(out, StrEq("stdout\n*** command '" + simple_command +
+    EXPECT_EQ(42, RunCommand("", {kSimpleCommand, "--exit", "42"}));
+    EXPECT_THAT(out, StrEq("stdout\n*** command '" + kSimpleCommand +
                            " --exit 42' failed: exit code 42\n"));
-    EXPECT_THAT(err, StrEq("stderr\n*** command '" + simple_command +
+    EXPECT_THAT(err, StrEq("stderr\n*** command '" + kSimpleCommand +
                            " --exit 42' failed: exit code 42\n"));
 }
 
 TEST_F(DumpstateTest, RunCommandCrashes) {
-    EXPECT_NE(0, RunCommand("", {simple_command, "--crash"}));
+    EXPECT_NE(0, RunCommand("", {kSimpleCommand, "--crash"}));
     // We don't know the exit code, so check just the prefix.
     EXPECT_THAT(
-        out, StartsWith("stdout\n*** command '" + simple_command + " --crash' failed: exit code"));
+        out, StartsWith("stdout\n*** command '" + kSimpleCommand + " --crash' failed: exit code"));
     EXPECT_THAT(
-        err, StartsWith("stderr\n*** command '" + simple_command + " --crash' failed: exit code"));
+        err, StartsWith("stderr\n*** command '" + kSimpleCommand + " --crash' failed: exit code"));
 }
 
 TEST_F(DumpstateTest, RunCommandTimesout) {
-    EXPECT_EQ(-1, RunCommand("", {simple_command, "--sleep", "2"},
+    EXPECT_EQ(-1, RunCommand("", {kSimpleCommand, "--sleep", "2"},
                              CommandOptions::WithTimeout(1).Build()));
-    EXPECT_THAT(out, StartsWith("stdout line1\n*** command '" + simple_command +
+    EXPECT_THAT(out, StartsWith("stdout line1\n*** command '" + kSimpleCommand +
                                 " --sleep 2' timed out after 1"));
-    EXPECT_THAT(err, StartsWith("sleeping for 2s\n*** command '" + simple_command +
+    EXPECT_THAT(err, StartsWith("sleeping for 2s\n*** command '" + kSimpleCommand +
                                 " --sleep 2' timed out after 1"));
 }
 
@@ -279,7 +313,7 @@ TEST_F(DumpstateTest, RunCommandIsKilled) {
     CaptureStderr();
 
     std::thread t([=]() {
-        EXPECT_EQ(SIGTERM, ds.RunCommand("", {simple_command, "--pid", "--sleep", "20"},
+        EXPECT_EQ(SIGTERM, ds.RunCommand("", {kSimpleCommand, "--pid", "--sleep", "20"},
                                          CommandOptions::WithTimeout(100).Always().Build()));
     });
 
@@ -306,37 +340,35 @@ TEST_F(DumpstateTest, RunCommandIsKilled) {
     out = GetCapturedStdout();
     err = GetCapturedStderr();
 
-    EXPECT_THAT(out, StrEq("*** command '" + simple_command +
+    EXPECT_THAT(out, StrEq("*** command '" + kSimpleCommand +
                            " --pid --sleep 20' failed: killed by signal 15\n"));
-    EXPECT_THAT(err, StrEq("*** command '" + simple_command +
+    EXPECT_THAT(err, StrEq("*** command '" + kSimpleCommand +
                            " --pid --sleep 20' failed: killed by signal 15\n"));
 }
 
 TEST_F(DumpstateTest, RunCommandProgressNoListener) {
-    ds.update_progress_ = true;
-    ds.progress_ = 0;
-    ds.weight_total_ = 30;
+    SetProgress(0, 30);
 
-    EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(20).Build()));
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(20).Build()));
     std::string progress_message = GetProgressMessageAndAssertSystemProperties(20, 30);
     EXPECT_THAT(out, StrEq("stdout\n"));
     EXPECT_THAT(err, StrEq("stderr\n" + progress_message));
 
-    EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(10).Build()));
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(10).Build()));
     progress_message = GetProgressMessageAndAssertSystemProperties(30, 30);
     EXPECT_THAT(out, StrEq("stdout\n"));
     EXPECT_THAT(err, StrEq("stderr\n" + progress_message));
 
     // Run a command that will increase maximum timeout.
-    EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(1).Build()));
-    progress_message = GetProgressMessageAndAssertSystemProperties(31, 36, 30);  // 20% increase
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(1).Build()));
+    progress_message = GetProgressMessageAndAssertSystemProperties(31, 37, 30);  // 20% increase
     EXPECT_THAT(out, StrEq("stdout\n"));
     EXPECT_THAT(err, StrEq("stderr\n" + progress_message));
 
     // Make sure command ran while in dry_run is counted.
     SetDryRun(true);
-    EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(4).Build()));
-    progress_message = GetProgressMessageAndAssertSystemProperties(35, 36);
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(4).Build()));
+    progress_message = GetProgressMessageAndAssertSystemProperties(35, 37);
     EXPECT_THAT(out, IsEmpty());
     EXPECT_THAT(err, StrEq(progress_message));
 }
@@ -345,36 +377,33 @@ TEST_F(DumpstateTest, RunCommandProgress) {
     sp<DumpstateListenerMock> listener(new DumpstateListenerMock());
     ds.listener_ = listener;
     ds.listener_name_ = "FoxMulder";
-
-    ds.update_progress_ = true;
-    ds.progress_ = 0;
-    ds.weight_total_ = 30;
+    SetProgress(0, 30);
 
     EXPECT_CALL(*listener, onProgressUpdated(20));
-    EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(20).Build()));
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(20).Build()));
     std::string progress_message = GetProgressMessage(ds.listener_name_, 20, 30);
     EXPECT_THAT(out, StrEq("stdout\n"));
     EXPECT_THAT(err, StrEq("stderr\n" + progress_message));
 
     EXPECT_CALL(*listener, onProgressUpdated(30));
-    EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(10).Build()));
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(10).Build()));
     progress_message = GetProgressMessage(ds.listener_name_, 30, 30);
     EXPECT_THAT(out, StrEq("stdout\n"));
     EXPECT_THAT(err, StrEq("stderr\n" + progress_message));
 
     // Run a command that will increase maximum timeout.
     EXPECT_CALL(*listener, onProgressUpdated(31));
-    EXPECT_CALL(*listener, onMaxProgressUpdated(36));
-    EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(1).Build()));
-    progress_message = GetProgressMessage(ds.listener_name_, 31, 36, 30);  // 20% increase
+    EXPECT_CALL(*listener, onMaxProgressUpdated(37));
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(1).Build()));
+    progress_message = GetProgressMessage(ds.listener_name_, 31, 37, 30);  // 20% increase
     EXPECT_THAT(out, StrEq("stdout\n"));
     EXPECT_THAT(err, StrEq("stderr\n" + progress_message));
 
     // Make sure command ran while in dry_run is counted.
     SetDryRun(true);
     EXPECT_CALL(*listener, onProgressUpdated(35));
-    EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(4).Build()));
-    progress_message = GetProgressMessage(ds.listener_name_, 35, 36);
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(4).Build()));
+    progress_message = GetProgressMessage(ds.listener_name_, 35, 37);
     EXPECT_THAT(out, IsEmpty());
     EXPECT_THAT(err, StrEq(progress_message));
 
@@ -385,14 +414,13 @@ TEST_F(DumpstateTest, RunCommandDropRoot) {
     // First check root case - only available when running with 'adb root'.
     uid_t uid = getuid();
     if (uid == 0) {
-        EXPECT_EQ(0, RunCommand("", {simple_command, "--uid"}));
+        EXPECT_EQ(0, RunCommand("", {kSimpleCommand, "--uid"}));
         EXPECT_THAT(out, StrEq("0\nstdout\n"));
         EXPECT_THAT(err, StrEq("stderr\n"));
         return;
     }
-    // Then drop root.
-
-    EXPECT_EQ(0, RunCommand("", {simple_command, "--uid"},
+    // Then run dropping root.
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand, "--uid"},
                             CommandOptions::WithTimeout(1).DropRoot().Build()));
     EXPECT_THAT(out, StrEq("2000\nstdout\n"));
     EXPECT_THAT(err, StrEq("drop_root_user(): already running as Shell\nstderr\n"));
@@ -406,29 +434,14 @@ TEST_F(DumpstateTest, RunCommandAsRootUserBuild) {
 
     DropRoot();
 
-    EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(1).AsRoot().Build()));
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(1).AsRoot().Build()));
 
     // We don't know the exact path of su, so we just check for the 'root ...' commands
     EXPECT_THAT(out, StartsWith("Skipping"));
-    EXPECT_THAT(out, EndsWith("root " + simple_command + "' on user build.\n"));
+    EXPECT_THAT(out, EndsWith("root " + kSimpleCommand + "' on user build.\n"));
     EXPECT_THAT(err, IsEmpty());
 }
 
-TEST_F(DumpstateTest, RunCommandAsRootNonUserBuild) {
-    if (IsUserBuild()) {
-        ALOGI("Skipping RunCommandAsRootNonUserBuild on user builds\n");
-        return;
-    }
-
-    DropRoot();
-
-    EXPECT_EQ(0, RunCommand("", {simple_command, "--uid"},
-                            CommandOptions::WithTimeout(1).AsRoot().Build()));
-
-    EXPECT_THAT(out, StrEq("0\nstdout\n"));
-    EXPECT_THAT(err, StrEq("stderr\n"));
-}
-
 TEST_F(DumpstateTest, DumpFileNotFoundNoTitle) {
     EXPECT_EQ(-1, DumpFile("", "/I/cant/believe/I/exist"));
     EXPECT_THAT(out,
@@ -446,52 +459,50 @@ TEST_F(DumpstateTest, DumpFileNotFoundWithTitle) {
 }
 
 TEST_F(DumpstateTest, DumpFileSingleLine) {
-    EXPECT_EQ(0, DumpFile("", test_data_path + "single-line.txt"));
+    EXPECT_EQ(0, DumpFile("", kTestDataPath + "single-line.txt"));
     EXPECT_THAT(err, IsEmpty());
     EXPECT_THAT(out, StrEq("I AM LINE1\n"));  // dumpstate adds missing newline
 }
 
 TEST_F(DumpstateTest, DumpFileSingleLineWithNewLine) {
-    EXPECT_EQ(0, DumpFile("", test_data_path + "single-line-with-newline.txt"));
+    EXPECT_EQ(0, DumpFile("", kTestDataPath + "single-line-with-newline.txt"));
     EXPECT_THAT(err, IsEmpty());
     EXPECT_THAT(out, StrEq("I AM LINE1\n"));
 }
 
 TEST_F(DumpstateTest, DumpFileMultipleLines) {
-    EXPECT_EQ(0, DumpFile("", test_data_path + "multiple-lines.txt"));
+    EXPECT_EQ(0, DumpFile("", kTestDataPath + "multiple-lines.txt"));
     EXPECT_THAT(err, IsEmpty());
     EXPECT_THAT(out, StrEq("I AM LINE1\nI AM LINE2\nI AM LINE3\n"));
 }
 
 TEST_F(DumpstateTest, DumpFileMultipleLinesWithNewLine) {
-    EXPECT_EQ(0, DumpFile("", test_data_path + "multiple-lines-with-newline.txt"));
+    EXPECT_EQ(0, DumpFile("", kTestDataPath + "multiple-lines-with-newline.txt"));
     EXPECT_THAT(err, IsEmpty());
     EXPECT_THAT(out, StrEq("I AM LINE1\nI AM LINE2\nI AM LINE3\n"));
 }
 
 TEST_F(DumpstateTest, DumpFileOnDryRunNoTitle) {
     SetDryRun(true);
-    EXPECT_EQ(0, DumpFile("", test_data_path + "single-line.txt"));
+    EXPECT_EQ(0, DumpFile("", kTestDataPath + "single-line.txt"));
     EXPECT_THAT(err, IsEmpty());
     EXPECT_THAT(out, IsEmpty());
 }
 
 TEST_F(DumpstateTest, DumpFileOnDryRun) {
     SetDryRun(true);
-    EXPECT_EQ(0, DumpFile("Might as well dump. Dump!", test_data_path + "single-line.txt"));
+    EXPECT_EQ(0, DumpFile("Might as well dump. Dump!", kTestDataPath + "single-line.txt"));
     EXPECT_THAT(err, IsEmpty());
-    EXPECT_THAT(out, StartsWith("------ Might as well dump. Dump! (" + test_data_path +
+    EXPECT_THAT(out, StartsWith("------ Might as well dump. Dump! (" + kTestDataPath +
                                 "single-line.txt) ------\n\t(skipped on dry run)\n------"));
     EXPECT_THAT(out, EndsWith("s was the duration of 'Might as well dump. Dump!' ------\n"));
     EXPECT_THAT(err, IsEmpty());
 }
 
 TEST_F(DumpstateTest, DumpFileUpdateProgressNoListener) {
-    ds.update_progress_ = true;
-    ds.progress_ = 0;
-    ds.weight_total_ = 30;
+    SetProgress(0, 30);
 
-    EXPECT_EQ(0, DumpFile("", test_data_path + "single-line.txt"));
+    EXPECT_EQ(0, DumpFile("", kTestDataPath + "single-line.txt"));
 
     std::string progress_message =
         GetProgressMessageAndAssertSystemProperties(5, 30);  // TODO: unhardcode WEIGHT_FILE (5)?
@@ -504,12 +515,10 @@ TEST_F(DumpstateTest, DumpFileUpdateProgress) {
     sp<DumpstateListenerMock> listener(new DumpstateListenerMock());
     ds.listener_ = listener;
     ds.listener_name_ = "FoxMulder";
-    ds.update_progress_ = true;
-    ds.progress_ = 0;
-    ds.weight_total_ = 30;
+    SetProgress(0, 30);
 
     EXPECT_CALL(*listener, onProgressUpdated(5));
-    EXPECT_EQ(0, DumpFile("", test_data_path + "single-line.txt"));
+    EXPECT_EQ(0, DumpFile("", kTestDataPath + "single-line.txt"));
 
     std::string progress_message =
         GetProgressMessage(ds.listener_name_, 5, 30);  // TODO: unhardcode WEIGHT_FILE (5)?
@@ -519,7 +528,7 @@ TEST_F(DumpstateTest, DumpFileUpdateProgress) {
     ds.listener_.clear();
 }
 
-class DumpstateServiceTest : public Test {
+class DumpstateServiceTest : public DumpstateBaseTest {
   public:
     DumpstateService dss;
 };
@@ -548,3 +557,229 @@ TEST_F(DumpstateServiceTest, SetListenerTwice) {
     EXPECT_TRUE(dss.setListener("whatever", listener, &result).isOk());
     EXPECT_FALSE(result);
 }
+
+class ProgressTest : public DumpstateBaseTest {
+  public:
+    Progress GetInstance(int32_t max, double growth_factor, const std::string& path = "") {
+        return Progress(max, growth_factor, path);
+    }
+
+    void AssertStats(const std::string& path, int32_t expected_runs, int32_t expected_average) {
+        std::string expected_content =
+            android::base::StringPrintf("%d %d\n", expected_runs, expected_average);
+        std::string actual_content;
+        ASSERT_TRUE(android::base::ReadFileToString(path, &actual_content))
+            << "could not read statsfrom" << path;
+        ASSERT_THAT(actual_content, StrEq(expected_content)) << "invalid stats on " << path;
+    }
+};
+
+TEST_F(ProgressTest, SimpleTest) {
+    Progress progress;
+    EXPECT_EQ(0, progress.Get());
+    EXPECT_EQ(Progress::kDefaultMax, progress.GetInitialMax());
+    EXPECT_EQ(Progress::kDefaultMax, progress.GetMax());
+
+    bool max_increased = progress.Inc(1);
+    EXPECT_EQ(1, progress.Get());
+    EXPECT_EQ(Progress::kDefaultMax, progress.GetInitialMax());
+    EXPECT_EQ(Progress::kDefaultMax, progress.GetMax());
+    EXPECT_FALSE(max_increased);
+
+    // Ignore negative increase.
+    max_increased = progress.Inc(-1);
+    EXPECT_EQ(1, progress.Get());
+    EXPECT_EQ(Progress::kDefaultMax, progress.GetInitialMax());
+    EXPECT_EQ(Progress::kDefaultMax, progress.GetMax());
+    EXPECT_FALSE(max_increased);
+}
+
+TEST_F(ProgressTest, MaxGrowsInsideNewRange) {
+    Progress progress = GetInstance(10, 1.2);  // 20% growth factor
+    EXPECT_EQ(0, progress.Get());
+    EXPECT_EQ(10, progress.GetInitialMax());
+    EXPECT_EQ(10, progress.GetMax());
+
+    // No increase
+    bool max_increased = progress.Inc(10);
+    EXPECT_EQ(10, progress.Get());
+    EXPECT_EQ(10, progress.GetMax());
+    EXPECT_FALSE(max_increased);
+
+    // Increase, with new value < max*20%
+    max_increased = progress.Inc(1);
+    EXPECT_EQ(11, progress.Get());
+    EXPECT_EQ(13, progress.GetMax());  // 11 average * 20% growth = 13.2 = 13
+    EXPECT_TRUE(max_increased);
+}
+
+TEST_F(ProgressTest, MaxGrowsOutsideNewRange) {
+    Progress progress = GetInstance(10, 1.2);  // 20% growth factor
+    EXPECT_EQ(0, progress.Get());
+    EXPECT_EQ(10, progress.GetInitialMax());
+    EXPECT_EQ(10, progress.GetMax());
+
+    // No increase
+    bool max_increased = progress.Inc(10);
+    EXPECT_EQ(10, progress.Get());
+    EXPECT_EQ(10, progress.GetMax());
+    EXPECT_FALSE(max_increased);
+
+    // Increase, with new value > max*20%
+    max_increased = progress.Inc(5);
+    EXPECT_EQ(15, progress.Get());
+    EXPECT_EQ(18, progress.GetMax());  // 15 average * 20% growth = 18
+    EXPECT_TRUE(max_increased);
+}
+
+TEST_F(ProgressTest, InvalidPath) {
+    Progress progress("/devil/null");
+    EXPECT_EQ(Progress::kDefaultMax, progress.GetMax());
+}
+
+TEST_F(ProgressTest, EmptyFile) {
+    Progress progress(CopyTextFileFixture("empty-file.txt"));
+    EXPECT_EQ(Progress::kDefaultMax, progress.GetMax());
+}
+
+TEST_F(ProgressTest, InvalidLine1stEntryNAN) {
+    Progress progress(CopyTextFileFixture("stats-invalid-1st-NAN.txt"));
+    EXPECT_EQ(Progress::kDefaultMax, progress.GetMax());
+}
+
+TEST_F(ProgressTest, InvalidLine2ndEntryNAN) {
+    Progress progress(CopyTextFileFixture("stats-invalid-2nd-NAN.txt"));
+    EXPECT_EQ(Progress::kDefaultMax, progress.GetMax());
+}
+
+TEST_F(ProgressTest, InvalidLineBothNAN) {
+    Progress progress(CopyTextFileFixture("stats-invalid-both-NAN.txt"));
+    EXPECT_EQ(Progress::kDefaultMax, progress.GetMax());
+}
+
+TEST_F(ProgressTest, InvalidLine1stEntryNegative) {
+    Progress progress(CopyTextFileFixture("stats-invalid-1st-negative.txt"));
+    EXPECT_EQ(Progress::kDefaultMax, progress.GetMax());
+}
+
+TEST_F(ProgressTest, InvalidLine2ndEntryNegative) {
+    Progress progress(CopyTextFileFixture("stats-invalid-2nd-negative.txt"));
+    EXPECT_EQ(Progress::kDefaultMax, progress.GetMax());
+}
+
+TEST_F(ProgressTest, InvalidLine1stEntryTooBig) {
+    Progress progress(CopyTextFileFixture("stats-invalid-1st-too-big.txt"));
+    EXPECT_EQ(Progress::kDefaultMax, progress.GetMax());
+}
+
+TEST_F(ProgressTest, InvalidLine2ndEntryTooBig) {
+    Progress progress(CopyTextFileFixture("stats-invalid-2nd-too-big.txt"));
+    EXPECT_EQ(Progress::kDefaultMax, progress.GetMax());
+}
+
+// Tests stats are properly saved when the file does not exists.
+TEST_F(ProgressTest, FirstTime) {
+    std::string path = kTestDataPath + "FirstTime.txt";
+    android::base::RemoveFileIfExists(path);
+
+    Progress run1(path);
+    EXPECT_EQ(0, run1.Get());
+    EXPECT_EQ(Progress::kDefaultMax, run1.GetInitialMax());
+    EXPECT_EQ(Progress::kDefaultMax, run1.GetMax());
+
+    bool max_increased = run1.Inc(20);
+    EXPECT_EQ(20, run1.Get());
+    EXPECT_EQ(Progress::kDefaultMax, run1.GetMax());
+    EXPECT_FALSE(max_increased);
+
+    run1.Save();
+    AssertStats(path, 1, 20);
+}
+
+// Tests what happens when the persistent settings contains the average duration of 1 run.
+// Data on file is 1 run and 109 average.
+TEST_F(ProgressTest, SecondTime) {
+    std::string path = CopyTextFileFixture("stats-one-run-no-newline.txt");
+
+    Progress run1 = GetInstance(-42, 1.2, path);
+    EXPECT_EQ(0, run1.Get());
+    EXPECT_EQ(10, run1.GetInitialMax());
+    EXPECT_EQ(10, run1.GetMax());
+
+    bool max_increased = run1.Inc(20);
+    EXPECT_EQ(20, run1.Get());
+    EXPECT_EQ(24, run1.GetMax());
+    EXPECT_TRUE(max_increased);
+
+    // Average now is 2 runs and (10 + 20)/ 2 = 15
+    run1.Save();
+    AssertStats(path, 2, 15);
+
+    Progress run2 = GetInstance(-42, 1.2, path);
+    EXPECT_EQ(0, run2.Get());
+    EXPECT_EQ(15, run2.GetInitialMax());
+    EXPECT_EQ(15, run2.GetMax());
+
+    max_increased = run2.Inc(25);
+    EXPECT_EQ(25, run2.Get());
+    EXPECT_EQ(30, run2.GetMax());
+    EXPECT_TRUE(max_increased);
+
+    // Average now is 3 runs and (15 * 2 + 25)/ 3 = 18.33 = 18
+    run2.Save();
+    AssertStats(path, 3, 18);
+
+    Progress run3 = GetInstance(-42, 1.2, path);
+    EXPECT_EQ(0, run3.Get());
+    EXPECT_EQ(18, run3.GetInitialMax());
+    EXPECT_EQ(18, run3.GetMax());
+
+    // Make sure average decreases as well
+    max_increased = run3.Inc(5);
+    EXPECT_EQ(5, run3.Get());
+    EXPECT_EQ(18, run3.GetMax());
+    EXPECT_FALSE(max_increased);
+
+    // Average now is 4 runs and (18 * 3 + 5)/ 4 = 14.75 = 14
+    run3.Save();
+    AssertStats(path, 4, 14);
+}
+
+// Tests what happens when the persistent settings contains the average duration of 2 runs.
+// Data on file is 2 runs and 15 average.
+TEST_F(ProgressTest, ThirdTime) {
+    std::string path = CopyTextFileFixture("stats-two-runs.txt");
+    AssertStats(path, 2, 15);  // Sanity check
+
+    Progress run1 = GetInstance(-42, 1.2, path);
+    EXPECT_EQ(0, run1.Get());
+    EXPECT_EQ(15, run1.GetInitialMax());
+    EXPECT_EQ(15, run1.GetMax());
+
+    bool max_increased = run1.Inc(20);
+    EXPECT_EQ(20, run1.Get());
+    EXPECT_EQ(24, run1.GetMax());
+    EXPECT_TRUE(max_increased);
+
+    // Average now is 3 runs and (15 * 2 + 20)/ 3 = 16.66 = 16
+    run1.Save();
+    AssertStats(path, 3, 16);
+}
+
+// TODO: RunCommandAsRootNonUserBuild must be the last test because it drops root, which could cause
+// other tests to fail if they're relyin on the process running as root.
+// For now this is fine, but eventually it might need to be moved to its own test case / process.
+TEST_F(DumpstateTest, RunCommandAsRootNonUserBuild) {
+    if (IsUserBuild()) {
+        ALOGI("Skipping RunCommandAsRootNonUserBuild on user builds\n");
+        return;
+    }
+
+    DropRoot();
+
+    EXPECT_EQ(0, RunCommand("", {kSimpleCommand, "--uid"},
+                            CommandOptions::WithTimeout(1).AsRoot().Build()));
+
+    EXPECT_THAT(out, StrEq("0\nstdout\n"));
+    EXPECT_THAT(err, StrEq("stderr\n"));
+}
index 34e09d7..d33460c 100644 (file)
@@ -19,6 +19,7 @@
 #include <fcntl.h>
 #include <libgen.h>
 #include <limits.h>
+#include <math.h>
 #include <poll.h>
 #include <signal.h>
 #include <stdarg.h>
@@ -42,6 +43,7 @@
 #include <android-base/file.h>
 #include <android-base/properties.h>
 #include <android-base/stringprintf.h>
+#include <android-base/strings.h>
 #include <cutils/debugger.h>
 #include <cutils/log.h>
 #include <cutils/properties.h>
@@ -67,7 +69,7 @@ static int RunCommand(const std::string& title, const std::vector<std::string>&
 static bool IsDryRun() {
     return Dumpstate::GetInstance().IsDryRun();
 }
-static void UpdateProgress(int delta) {
+static void UpdateProgress(int32_t delta) {
     ds.UpdateProgress(delta);
 }
 
@@ -90,6 +92,10 @@ static const char* native_processes_to_dump[] = {
 /* Most simple commands have 10 as timeout, so 5 is a good estimate */
 static const int WEIGHT_FILE = 5;
 
+// Reasonable value for max stats.
+static const int STATS_MAX_N_RUNS = 1000;
+static const long STATS_MAX_AVERAGE = 100000;
+
 CommandOptions CommandOptions::DEFAULT = CommandOptions::WithTimeout(10).Build();
 CommandOptions CommandOptions::DEFAULT_DUMPSYS = CommandOptions::WithTimeout(30).Build();
 CommandOptions CommandOptions::AS_ROOT_5 = CommandOptions::WithTimeout(5).AsRoot().Build();
@@ -207,6 +213,115 @@ uint64_t DurationReporter::DurationReporter::Nanotime() {
     return (uint64_t) ts.tv_sec * NANOS_PER_SEC + ts.tv_nsec;
 }
 
+const int32_t Progress::kDefaultMax = 5000;
+
+Progress::Progress(const std::string& path) : Progress(Progress::kDefaultMax, 1.1, path) {
+}
+
+Progress::Progress(int32_t initial_max, int32_t progress, float growth_factor)
+    : Progress(initial_max, growth_factor, "") {
+    progress_ = progress;
+}
+
+Progress::Progress(int32_t initial_max, float growth_factor, const std::string& path)
+    : initial_max_(initial_max),
+      progress_(0),
+      max_(initial_max),
+      growth_factor_(growth_factor),
+      n_runs_(0),
+      average_max_(0),
+      path_(path) {
+    if (!path_.empty()) {
+        Load();
+    }
+}
+
+void Progress::Load() {
+    MYLOGD("Loading stats from %s\n", path_.c_str());
+    std::string content;
+    if (!android::base::ReadFileToString(path_, &content)) {
+        MYLOGI("Could not read stats from %s; using max of %d\n", path_.c_str(), max_);
+        return;
+    }
+    if (content.empty()) {
+        MYLOGE("No stats (empty file) on %s; using max of %d\n", path_.c_str(), max_);
+        return;
+    }
+    std::vector<std::string> lines = android::base::Split(content, "\n");
+
+    if (lines.size() < 1) {
+        MYLOGE("Invalid stats on file %s: not enough lines (%d). Using max of %d\n", path_.c_str(),
+               (int)lines.size(), max_);
+        return;
+    }
+    char* ptr;
+    n_runs_ = strtol(lines[0].c_str(), &ptr, 10);
+    average_max_ = strtol(ptr, nullptr, 10);
+    if (n_runs_ <= 0 || average_max_ <= 0 || n_runs_ > STATS_MAX_N_RUNS ||
+        average_max_ > STATS_MAX_AVERAGE) {
+        MYLOGE("Invalid stats line on file %s: %s\n", path_.c_str(), lines[0].c_str());
+        initial_max_ = Progress::kDefaultMax;
+    } else {
+        initial_max_ = average_max_;
+    }
+    max_ = initial_max_;
+
+    MYLOGI("Average max progress: %d in %d runs; estimated max: %d\n", average_max_, n_runs_, max_);
+}
+
+void Progress::Save() {
+    int32_t total = n_runs_ * average_max_ + progress_;
+    int32_t runs = n_runs_ + 1;
+    int32_t average = floor(((float)total) / runs);
+    MYLOGI("Saving stats (total=%d, runs=%d, average=%d) on %s\n", total, runs, average,
+           path_.c_str());
+    if (path_.empty()) {
+        return;
+    }
+
+    std::string content = android::base::StringPrintf("%d %d\n", runs, average);
+    if (!android::base::WriteStringToFile(content, path_)) {
+        MYLOGE("Could not save stats on %s\n", path_.c_str());
+    }
+}
+
+int32_t Progress::Get() const {
+    return progress_;
+}
+
+bool Progress::Inc(int32_t delta) {
+    bool changed = false;
+    if (delta >= 0) {
+        progress_ += delta;
+        if (progress_ > max_) {
+            int32_t old_max = max_;
+            max_ = floor((float)progress_ * growth_factor_);
+            MYLOGD("Adjusting max progress from %d to %d\n", old_max, max_);
+            changed = true;
+        }
+    }
+    return changed;
+}
+
+int32_t Progress::GetMax() const {
+    return max_;
+}
+
+int32_t Progress::GetInitialMax() const {
+    return initial_max_;
+}
+
+void Progress::Dump(int fd, const std::string& prefix) const {
+    const char* pr = prefix.c_str();
+    dprintf(fd, "%sprogress: %d\n", pr, progress_);
+    dprintf(fd, "%smax: %d\n", pr, max_);
+    dprintf(fd, "%sinitial_max: %d\n", pr, initial_max_);
+    dprintf(fd, "%sgrowth_factor: %0.2f\n", pr, growth_factor_);
+    dprintf(fd, "%spath: %s\n", pr, path_.c_str());
+    dprintf(fd, "%sn_runs: %d\n", pr, n_runs_);
+    dprintf(fd, "%saverage_max: %d\n", pr, average_max_);
+}
+
 bool Dumpstate::IsDryRun() const {
     return dry_run_;
 }
@@ -224,6 +339,10 @@ std::string Dumpstate::GetPath(const std::string& suffix) const {
                                        name_.c_str(), suffix.c_str());
 }
 
+void Dumpstate::SetProgress(std::unique_ptr<Progress> progress) {
+    progress_ = std::move(progress);
+}
+
 void for_each_userid(void (*func)(int), const char *header) {
     if (IsDryRun()) return;
 
@@ -1320,26 +1439,29 @@ void dump_route_tables() {
 }
 
 // TODO: make this function thread safe if sections are generated in parallel.
-void Dumpstate::UpdateProgress(int delta) {
-    if (!update_progress_) return;
+void Dumpstate::UpdateProgress(int32_t delta) {
+    if (progress_ == nullptr) {
+        MYLOGE("UpdateProgress: progress_ not set\n");
+        return;
+    }
+
+    // Always update progess so stats can be tuned...
+    bool max_changed = progress_->Inc(delta);
 
-    progress_ += delta;
+    // ...but only notifiy listeners when necessary.
+    if (!update_progress_) return;
 
     // TODO: remove property support once Shell uses IDumpstateListener
     char key[PROPERTY_KEY_MAX];
     char value[PROPERTY_VALUE_MAX];
 
     // adjusts max on the fly
-    if (progress_ > weight_total_) {
-        int new_total = weight_total_ * 1.2;
-        MYLOGD("Adjusting total weight from %d to %d\n", weight_total_, new_total);
-        weight_total_ = new_total;
-
+    if (max_changed) {
         if (listener_ != nullptr) {
-            listener_->onMaxProgressUpdated(weight_total_);
+            listener_->onMaxProgressUpdated(progress_->GetMax());
         } else {
             snprintf(key, sizeof(key), "dumpstate.%d.max", pid_);
-            snprintf(value, sizeof(value), "%d", weight_total_);
+            snprintf(value, sizeof(value), "%d", progress_->GetMax());
             int status = property_set(key, value);
             if (status != 0) {
                 MYLOGE("Could not update max weight by setting system property %s to %s: %d\n", key,
@@ -1348,34 +1470,35 @@ void Dumpstate::UpdateProgress(int delta) {
         }
     }
 
+    int32_t progress = progress_->Get();
+    int32_t max = progress_->GetMax();
+
     if (control_socket_fd_ >= 0) {
-        dprintf(control_socket_fd_, "PROGRESS:%d/%d\n", progress_, weight_total_);
+        dprintf(control_socket_fd_, "PROGRESS:%d/%d\n", progress, max);
         fsync(control_socket_fd_);
     }
 
     if (listener_ != nullptr) {
-        if (progress_ % 100 == 0) {
+        if (progress % 100 == 0) {
             // We don't want to spam logcat, so only log multiples of 100.
-            MYLOGD("Setting progress (%s): %d/%d\n", listener_name_.c_str(), progress_,
-                   weight_total_);
+            MYLOGD("Setting progress (%s): %d/%d\n", listener_name_.c_str(), progress, max);
         } else {
             // stderr is ignored on normal invocations, but useful when calling
             // /system/bin/dumpstate directly for debuggging.
-            fprintf(stderr, "Setting progress (%s): %d/%d\n", listener_name_.c_str(), progress_,
-                    weight_total_);
+            fprintf(stderr, "Setting progress (%s): %d/%d\n", listener_name_.c_str(), progress, max);
         }
-        listener_->onProgressUpdated(progress_);
+        listener_->onProgressUpdated(progress);
     } else {
         snprintf(key, sizeof(key), "dumpstate.%d.progress", pid_);
-        snprintf(value, sizeof(value), "%d", progress_);
+        snprintf(value, sizeof(value), "%d", progress);
 
-        if (progress_ % 100 == 0) {
+        if (progress % 100 == 0) {
             // We don't want to spam logcat, so only log multiples of 100.
-            MYLOGD("Setting progress (%s): %s/%d\n", key, value, weight_total_);
+            MYLOGD("Setting progress (%s): %s/%d\n", key, value, max);
         } else {
             // stderr is ignored on normal invocations, but useful when calling
             // /system/bin/dumpstate directly for debuggging.
-            fprintf(stderr, "Setting progress (%s): %s/%d\n", key, value, weight_total_);
+            fprintf(stderr, "Setting progress (%s): %s/%d\n", key, value, max);
         }
 
         int status = property_set(key, value);