From 23e969dfdc5cf9a2c2099d3cbbb15ff546f1f38e Mon Sep 17 00:00:00 2001 From: James Rowe Date: Wed, 14 Aug 2019 21:03:11 -0600 Subject: [PATCH] Address review comments --- src/citra/default_ini.h | 2 +- src/citra_qt/configuration/config.cpp | 6 ++++-- src/core/core.cpp | 5 +++-- src/core/perf_stats.cpp | 25 +++++++++++++------------ src/core/perf_stats.h | 2 +- 5 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/citra/default_ini.h b/src/citra/default_ini.h index 4a17ad2d5..76996a0ea 100644 --- a/src/citra/default_ini.h +++ b/src/citra/default_ini.h @@ -261,7 +261,7 @@ camera_inner_flip = log_filter = *:Info [Debugging] -# Record frame time data, can be found in the log directory +# Record frame time data, can be found in the log directory. Boolean value record_frame_times = # Port for listening to GDB connections. use_gdbstub=false diff --git a/src/citra_qt/configuration/config.cpp b/src/citra_qt/configuration/config.cpp index 1a56e96b2..8ee51b907 100644 --- a/src/citra_qt/configuration/config.cpp +++ b/src/citra_qt/configuration/config.cpp @@ -255,7 +255,8 @@ void Config::ReadValues() { qt_config->endGroup(); qt_config->beginGroup("Debugging"); - Settings::values.record_frame_times = ReadSetting("record_frame_times", false).toBool(); + // Intentionally not using the QT default setting as this is intended to be changed in the ini + Settings::values.record_frame_times = qt_config->value("record_frame_times", false).toBool(); Settings::values.use_gdbstub = ReadSetting("use_gdbstub", false).toBool(); Settings::values.gdbstub_port = ReadSetting("gdbstub_port", 24689).toInt(); @@ -545,7 +546,8 @@ void Config::SaveValues() { qt_config->endGroup(); qt_config->beginGroup("Debugging"); - WriteSetting("record_frame_times", Settings::values.record_frame_times, false); + // Intentionally not using the QT default setting as this is intended to be changed in the ini + qt_config->setValue("record_frame_times", Settings::values.record_frame_times); WriteSetting("use_gdbstub", Settings::values.use_gdbstub, false); WriteSetting("gdbstub_port", Settings::values.gdbstub_port, 24689); diff --git a/src/core/core.cpp b/src/core/core.cpp index 3aa26d456..56fd0798d 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -183,8 +183,8 @@ System::ResultStatus System::Init(Frontend::EmuWindow& emu_window, u32 system_mo timing = std::make_unique(); - kernel = std::make_unique( - *memory, *timing, [this] { PrepareReschedule(); }, system_mode); + kernel = std::make_unique(*memory, *timing, + [this] { PrepareReschedule(); }, system_mode); if (Settings::values.use_cpu_jit) { #ifdef ARCHITECTURE_x86_64 @@ -321,6 +321,7 @@ void System::Shutdown() { VideoCore::Shutdown(); HW::Shutdown(); telemetry_session.reset(); + perf_stats.reset(); rpc_server.reset(); cheat_engine.reset(); service_manager.reset(); diff --git a/src/core/perf_stats.cpp b/src/core/perf_stats.cpp index c9769761d..21e33d753 100644 --- a/src/core/perf_stats.cpp +++ b/src/core/perf_stats.cpp @@ -21,22 +21,23 @@ using std::chrono::microseconds; // Purposefully ignore the first five frames, as there's a significant amount of overhead in // booting that we shouldn't account for -constexpr size_t IGNORE_FRAMES = 5; +constexpr std::size_t IgnoreFrames = 5; namespace Core { PerfStats::PerfStats(u64 title_id) : title_id(title_id) {} PerfStats::~PerfStats() { - if (Settings::values.record_frame_times && title_id != 0) { - std::ostringstream stream; - std::copy(perf_history.begin() + IGNORE_FRAMES, perf_history.begin() + current_index, - std::ostream_iterator(stream, "\n")); - std::string path = FileUtil::GetUserPath(FileUtil::UserPath::LogDir); - std::string filename = fmt::format("{}/{:X}.csv", path, title_id); - FileUtil::IOFile file(filename, "w"); - file.WriteString(stream.str()); + if (!Settings::values.record_frame_times || title_id == 0) { + return; } + std::ostringstream stream; + std::copy(perf_history.begin() + IgnoreFrames, perf_history.begin() + current_index, + std::ostream_iterator(stream, "\n")); + std::string path = FileUtil::GetUserPath(FileUtil::UserPath::LogDir); + std::string filename = fmt::format("{}/{:X}.csv", path, title_id); + FileUtil::IOFile file(filename, "w"); + file.WriteString(stream.str()); } void PerfStats::BeginSystemFrame() { @@ -68,12 +69,12 @@ void PerfStats::EndGameFrame() { } double PerfStats::GetMeanFrametime() { - if (current_index < 2) { + if (current_index <= IgnoreFrames) { return 0; } - double sum = std::accumulate(perf_history.begin() + IGNORE_FRAMES, + double sum = std::accumulate(perf_history.begin() + IgnoreFrames, perf_history.begin() + current_index, 0); - return sum / (current_index - 1 - IGNORE_FRAMES); + return sum / (current_index - IgnoreFrames); } PerfStats::Results PerfStats::GetAndResetStats(microseconds current_system_time_us) { diff --git a/src/core/perf_stats.h b/src/core/perf_stats.h index 39abaaa65..e0b023059 100644 --- a/src/core/perf_stats.h +++ b/src/core/perf_stats.h @@ -59,7 +59,7 @@ private: /// Title ID for the game that is running. 0 if there is no game running yet u64 title_id{0}; /// Current index for writing to the perf_history array - size_t current_index{0}; + std::size_t current_index{0}; /// Stores an hour of historical frametime data useful for processing and tracking performance /// regressions with code changes. std::array perf_history = {};