diff options
| -rw-r--r-- | cmds/dumpstate/Android.bp | 1 | ||||
| -rw-r--r-- | cmds/dumpstate/DumpstateSectionReporter.cpp | 42 | ||||
| -rw-r--r-- | cmds/dumpstate/DumpstateSectionReporter.h | 65 | ||||
| -rw-r--r-- | cmds/dumpstate/DumpstateService.cpp | 3 | ||||
| -rw-r--r-- | cmds/dumpstate/binder/android/os/IDumpstateListener.aidl | 17 | ||||
| -rw-r--r-- | cmds/dumpstate/dumpstate.cpp | 25 | ||||
| -rw-r--r-- | cmds/dumpstate/dumpstate.h | 8 | ||||
| -rw-r--r-- | cmds/dumpstate/tests/dumpstate_smoke_test.cpp | 21 | ||||
| -rw-r--r-- | cmds/dumpstate/tests/dumpstate_test.cpp | 72 |
9 files changed, 22 insertions, 232 deletions
diff --git a/cmds/dumpstate/Android.bp b/cmds/dumpstate/Android.bp index 8d383f501a..93bbe902c0 100644 --- a/cmds/dumpstate/Android.bp +++ b/cmds/dumpstate/Android.bp @@ -93,7 +93,6 @@ cc_defaults { "libutils", ], srcs: [ - "DumpstateSectionReporter.cpp", "DumpstateService.cpp", ], static_libs: [ diff --git a/cmds/dumpstate/DumpstateSectionReporter.cpp b/cmds/dumpstate/DumpstateSectionReporter.cpp deleted file mode 100644 index f814bde26d..0000000000 --- a/cmds/dumpstate/DumpstateSectionReporter.cpp +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright (C) 2018 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#define LOG_TAG "dumpstate" - -#include "DumpstateSectionReporter.h" - -namespace android { -namespace os { -namespace dumpstate { - -DumpstateSectionReporter::DumpstateSectionReporter(const std::string& title, - sp<android::os::IDumpstateListener> listener, - bool sendReport) - : title_(title), listener_(listener), sendReport_(sendReport), status_(OK), size_(-1) { - started_ = std::chrono::steady_clock::now(); -} - -DumpstateSectionReporter::~DumpstateSectionReporter() { - if ((listener_ != nullptr) && (sendReport_)) { - auto elapsed = std::chrono::duration_cast<std::chrono::milliseconds>( - std::chrono::steady_clock::now() - started_); - listener_->onSectionComplete(title_, status_, size_, (int32_t)elapsed.count()); - } -} - -} // namespace dumpstate -} // namespace os -} // namespace android diff --git a/cmds/dumpstate/DumpstateSectionReporter.h b/cmds/dumpstate/DumpstateSectionReporter.h deleted file mode 100644 index e971de84c5..0000000000 --- a/cmds/dumpstate/DumpstateSectionReporter.h +++ /dev/null @@ -1,65 +0,0 @@ -/* - * Copyright (C) 2018 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef ANDROID_OS_DUMPSTATESECTIONREPORTER_H_ -#define ANDROID_OS_DUMPSTATESECTIONREPORTER_H_ - -#include <android/os/IDumpstateListener.h> -#include <utils/StrongPointer.h> - -namespace android { -namespace os { -namespace dumpstate { - - -/* - * Helper class used to report per section details to a listener. - * - * Typical usage: - * - * DumpstateSectionReporter sectionReporter(title, listener, sendReport); - * sectionReporter.setSize(5000); - * - */ -class DumpstateSectionReporter { - public: - DumpstateSectionReporter(const std::string& title, sp<android::os::IDumpstateListener> listener, - bool sendReport); - - ~DumpstateSectionReporter(); - - void setStatus(status_t status) { - status_ = status; - } - - void setSize(int size) { - size_ = size; - } - - private: - std::string title_; - android::sp<android::os::IDumpstateListener> listener_; - bool sendReport_; - status_t status_; - int size_; - std::chrono::time_point<std::chrono::steady_clock> started_; -}; - -} // namespace dumpstate -} // namespace os -} // namespace android - -#endif // ANDROID_OS_DUMPSTATESECTIONREPORTER_H_ diff --git a/cmds/dumpstate/DumpstateService.cpp b/cmds/dumpstate/DumpstateService.cpp index 37ba4f906e..f98df99534 100644 --- a/cmds/dumpstate/DumpstateService.cpp +++ b/cmds/dumpstate/DumpstateService.cpp @@ -200,8 +200,7 @@ status_t DumpstateService::dump(int fd, const Vector<String16>&) { dprintf(fd, "id: %d\n", ds_->id_); dprintf(fd, "pid: %d\n", ds_->pid_); dprintf(fd, "update_progress: %s\n", ds_->options_->do_progress_updates ? "true" : "false"); - dprintf(fd, "update_progress_threshold: %d\n", ds_->update_progress_threshold_); - dprintf(fd, "last_updated_progress: %d\n", ds_->last_updated_progress_); + dprintf(fd, "last_percent_progress: %d\n", ds_->last_reported_percent_progress_); dprintf(fd, "progress:\n"); ds_->progress_->Dump(fd, " "); dprintf(fd, "args: %s\n", ds_->options_->args.c_str()); diff --git a/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl b/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl index ea1e467dca..e486460753 100644 --- a/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl +++ b/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl @@ -61,21 +61,4 @@ interface IDumpstateListener { * Called when taking bugreport finishes successfully. */ void onFinished(); - - // TODO(b/111441001): Remove old methods when not used anymore. - void onProgressUpdated(int progress); - void onMaxProgressUpdated(int maxProgress); - - /** - * Called after every section is complete. - * - * @param name section name - * @param status values from status_t - * {@code OK} section completed successfully - * {@code TIMEOUT} dump timed out - * {@code != OK} error - * @param size size in bytes, may be invalid if status != OK - * @param durationMs duration in ms - */ - void onSectionComplete(@utf8InCpp String name, int status, int size, int durationMs); } diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index e158eb9ac2..79f345b697 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -80,7 +80,6 @@ #include <serviceutils/PriorityDumper.h> #include <utils/StrongPointer.h> #include "DumpstateInternal.h" -#include "DumpstateSectionReporter.h" #include "DumpstateService.h" #include "dumpstate.h" @@ -105,7 +104,6 @@ using android::base::StringPrintf; using android::os::IDumpstateListener; using android::os::dumpstate::CommandOptions; using android::os::dumpstate::DumpFileToFd; -using android::os::dumpstate::DumpstateSectionReporter; using android::os::dumpstate::PropertiesHelper; // Keep in sync with @@ -1047,7 +1045,6 @@ static Dumpstate::RunStatus RunDumpsysTextByPriority(const std::string& title, i RETURN_IF_USER_DENIED_CONSENT(); std::string path(title); path.append(" - ").append(String8(service).c_str()); - DumpstateSectionReporter section_reporter(path, ds.listener_, ds.report_section_); size_t bytes_written = 0; status_t status = dumpsys.startDumpThread(service, args); if (status == OK) { @@ -1055,12 +1052,10 @@ static Dumpstate::RunStatus RunDumpsysTextByPriority(const std::string& title, i std::chrono::duration<double> elapsed_seconds; status = dumpsys.writeDump(STDOUT_FILENO, service, service_timeout, /* as_proto = */ false, elapsed_seconds, bytes_written); - section_reporter.setSize(bytes_written); dumpsys.writeDumpFooter(STDOUT_FILENO, service, elapsed_seconds); bool dump_complete = (status == OK); dumpsys.stopDumpThread(dump_complete); } - section_reporter.setStatus(status); auto elapsed_duration = std::chrono::duration_cast<std::chrono::milliseconds>( std::chrono::steady_clock::now() - start); @@ -1123,7 +1118,6 @@ static Dumpstate::RunStatus RunDumpsysProto(const std::string& title, int priori path.append("_HIGH"); } path.append(kProtoExt); - DumpstateSectionReporter section_reporter(path, ds.listener_, ds.report_section_); status_t status = dumpsys.startDumpThread(service, args); if (status == OK) { status = ds.AddZipEntryFromFd(path, dumpsys.getDumpFd(), service_timeout); @@ -1132,8 +1126,6 @@ static Dumpstate::RunStatus RunDumpsysProto(const std::string& title, int priori } ZipWriter::FileEntry file_entry; ds.zip_writer_->GetLastEntry(&file_entry); - section_reporter.setSize(file_entry.compressed_size); - section_reporter.setStatus(status); auto elapsed_duration = std::chrono::duration_cast<std::chrono::milliseconds>( std::chrono::steady_clock::now() - start); @@ -2808,6 +2800,7 @@ int run_main(int argc, char* argv[]) { Dumpstate::Dumpstate(const std::string& version) : pid_(getpid()), options_(new Dumpstate::DumpOptions()), + last_reported_percent_progress_(0), version_(version), now_(time(nullptr)) { } @@ -3570,31 +3563,25 @@ void Dumpstate::UpdateProgress(int32_t delta_sec) { } // Always update progess so stats can be tuned... - bool max_changed = progress_->Inc(delta_sec); + progress_->Inc(delta_sec); // ...but only notifiy listeners when necessary. if (!options_->do_progress_updates) return; int progress = progress_->Get(); int max = progress_->GetMax(); + int percent = 100 * progress / max; - // adjusts max on the fly - if (max_changed && listener_ != nullptr) { - listener_->onMaxProgressUpdated(max); - } - - int32_t last_update_delta = progress - last_updated_progress_; - if (last_updated_progress_ > 0 && last_update_delta < update_progress_threshold_) { + if (last_reported_percent_progress_ > 0 && percent <= last_reported_percent_progress_) { return; } - last_updated_progress_ = progress; + last_reported_percent_progress_ = percent; if (control_socket_fd_ >= 0) { dprintf(control_socket_fd_, "PROGRESS:%d/%d\n", progress, max); fsync(control_socket_fd_); } - int percent = 100 * progress / max; if (listener_ != nullptr) { if (percent % 5 == 0) { // We don't want to spam logcat, so only log multiples of 5. @@ -3606,8 +3593,6 @@ void Dumpstate::UpdateProgress(int32_t delta_sec) { fprintf(stderr, "Setting progress (%s): %d/%d (%d%%)\n", listener_name_.c_str(), progress, max, percent); } - // TODO(b/111441001): Remove in favor of onProgress - listener_->onProgressUpdated(progress); listener_->onProgress(percent); } diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h index ae6a72171a..f137fc927a 100644 --- a/cmds/dumpstate/dumpstate.h +++ b/cmds/dumpstate/dumpstate.h @@ -400,12 +400,8 @@ class Dumpstate { // Runtime options. std::unique_ptr<DumpOptions> options_; - // How frequently the progess should be updated;the listener will only be notificated when the - // delta from the previous update is more than the threshold. - int32_t update_progress_threshold_ = 100; - - // Last progress that triggered a listener updated - int32_t last_updated_progress_; + // Last progress that was sent to the listener [0-100]. + int last_reported_percent_progress_ = 0; // Whether it should take an screenshot earlier in the process. bool do_early_screenshot_ = false; diff --git a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp index f7acaf1b9b..181046a7a7 100644 --- a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp +++ b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp @@ -167,26 +167,6 @@ class DumpstateListener : public BnDumpstateListener { return binder::Status::ok(); } - binder::Status onProgressUpdated(int32_t progress) override { - dprintf(out_fd_, "\rIn progress %d/%d", progress, max_progress_); - return binder::Status::ok(); - } - - binder::Status onMaxProgressUpdated(int32_t max_progress) override { - std::lock_guard<std::mutex> lock(lock_); - max_progress_ = max_progress; - return binder::Status::ok(); - } - - binder::Status onSectionComplete(const ::std::string& name, int32_t, int32_t size_bytes, - int32_t) override { - std::lock_guard<std::mutex> lock(lock_); - if (sections_.get() != nullptr) { - sections_->push_back({name, size_bytes}); - } - return binder::Status::ok(); - } - bool getIsFinished() { std::lock_guard<std::mutex> lock(lock_); return is_finished_; @@ -199,7 +179,6 @@ class DumpstateListener : public BnDumpstateListener { private: int out_fd_; - int max_progress_ = 5000; int error_code_ = -1; bool is_finished_ = false; std::shared_ptr<std::vector<SectionInfo>> sections_; diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp index 4e6b084ff6..cff1d439d9 100644 --- a/cmds/dumpstate/tests/dumpstate_test.cpp +++ b/cmds/dumpstate/tests/dumpstate_test.cpp @@ -62,10 +62,6 @@ class DumpstateListenerMock : public IDumpstateListener { MOCK_METHOD1(onProgress, binder::Status(int32_t progress)); MOCK_METHOD1(onError, binder::Status(int32_t error_code)); MOCK_METHOD0(onFinished, binder::Status()); - MOCK_METHOD1(onProgressUpdated, binder::Status(int32_t progress)); - MOCK_METHOD1(onMaxProgressUpdated, binder::Status(int32_t max_progress)); - MOCK_METHOD4(onSectionComplete, binder::Status(const ::std::string& name, int32_t status, - int32_t size, int32_t durationMs)); protected: MOCK_METHOD0(onAsBinder, IBinder*()); @@ -590,7 +586,6 @@ class DumpstateTest : public DumpstateBaseTest { SetDryRun(false); SetBuildType(android::base::GetProperty("ro.build.type", "(unknown)")); ds.progress_.reset(new Progress()); - ds.update_progress_threshold_ = 0; ds.options_.reset(new Dumpstate::DumpOptions()); } @@ -615,10 +610,9 @@ class DumpstateTest : public DumpstateBaseTest { return status; } - void SetProgress(long progress, long initial_max, long threshold = 0) { + void SetProgress(long progress, long initial_max) { + ds.last_reported_percent_progress_ = 0; ds.options_->do_progress_updates = true; - ds.update_progress_threshold_ = threshold; - ds.last_updated_progress_ = 0; ds.progress_.reset(new Progress(initial_max, progress, 1.2)); } @@ -796,73 +790,36 @@ TEST_F(DumpstateTest, RunCommandProgress) { ds.listener_name_ = "FoxMulder"; SetProgress(0, 30); - EXPECT_CALL(*listener, onProgressUpdated(20)); EXPECT_CALL(*listener, onProgress(66)); // 20/30 % 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_CALL(*listener, onProgress(100)); // 35/35 % - 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(37)); - EXPECT_CALL(*listener, onProgress(83)); // 31/37 % - EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(1).Build())); - progress_message = GetProgressMessage(ds.listener_name_, 31, 37, 30); // 20% increase + EXPECT_CALL(*listener, onProgress(80)); // 24/30 % + EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(4).Build())); + progress_message = GetProgressMessage(ds.listener_name_, 24, 30); 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_CALL(*listener, onProgress(94)); // 35/37 % - EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(4).Build())); - progress_message = GetProgressMessage(ds.listener_name_, 35, 37); + EXPECT_CALL(*listener, onProgress(90)); // 27/30 % + EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(3).Build())); + progress_message = GetProgressMessage(ds.listener_name_, 27, 30); EXPECT_THAT(out, IsEmpty()); EXPECT_THAT(err, StrEq(progress_message)); - ds.listener_.clear(); -} - -TEST_F(DumpstateTest, RunCommandProgressIgnoreThreshold) { - sp<DumpstateListenerMock> listener(new DumpstateListenerMock()); - ds.listener_ = listener; - ds.listener_name_ = "FoxMulder"; - SetProgress(0, 8, 5); // 8 max, 5 threshold - - // First update should always be sent. - EXPECT_CALL(*listener, onProgressUpdated(1)); - EXPECT_CALL(*listener, onProgress(12)); // 1/12 % - EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(1).Build())); - std::string progress_message = GetProgressMessage(ds.listener_name_, 1, 8); + SetDryRun(false); + EXPECT_CALL(*listener, onProgress(96)); // 29/30 % + EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(2).Build())); + progress_message = GetProgressMessage(ds.listener_name_, 29, 30); EXPECT_THAT(out, StrEq("stdout\n")); EXPECT_THAT(err, StrEq("stderr\n" + progress_message)); - // Fourth update should be ignored because it's between the threshold (5 -1 = 4 < 5). - EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(4).Build())); - EXPECT_THAT(out, StrEq("stdout\n")); - EXPECT_THAT(err, StrEq("stderr\n")); - - // Third update should be sent because it reaches threshold (6 - 1 = 5). - EXPECT_CALL(*listener, onProgressUpdated(6)); - EXPECT_CALL(*listener, onProgress(75)); // 6/8 % + EXPECT_CALL(*listener, onProgress(100)); // 30/30 % EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(1).Build())); - progress_message = GetProgressMessage(ds.listener_name_, 6, 8); - EXPECT_THAT(out, StrEq("stdout\n")); - EXPECT_THAT(err, StrEq("stderr\n" + progress_message)); - - // Fourth update should be ignored because it's between the threshold (9 - 6 = 3 < 5). - // But max update should be sent. - EXPECT_CALL(*listener, onMaxProgressUpdated(10)); // 9 * 120% = 10.8 = 10 - EXPECT_EQ(0, RunCommand("", {kSimpleCommand}, CommandOptions::WithTimeout(3).Build())); - progress_message = GetProgressMessage(ds.listener_name_, 9, 10, 8, false); + progress_message = GetProgressMessage(ds.listener_name_, 30, 30); EXPECT_THAT(out, StrEq("stdout\n")); EXPECT_THAT(err, StrEq("stderr\n" + progress_message)); @@ -1090,7 +1047,6 @@ TEST_F(DumpstateTest, DumpFileUpdateProgress) { ds.listener_name_ = "FoxMulder"; SetProgress(0, 30); - EXPECT_CALL(*listener, onProgressUpdated(5)); EXPECT_CALL(*listener, onProgress(16)); // 5/30 % EXPECT_EQ(0, DumpFile("", kTestDataPath + "single-line.txt")); |