From 9a9fcf0ac11bbffb0c203d300c052be8b249083d Mon Sep 17 00:00:00 2001 From: Matthew Territo Date: Thu, 13 Jul 2017 20:16:01 -0600 Subject: [PATCH] Alex's recommended changes in pull #357. Add check for log directory existence and creation --- .../performance/performancemanager.h | 11 +++-- .../src/guiperformancecomponent.cpp | 2 + src/engine/configurationmanager_doc.inl | 6 ++- src/performance/performancemanager.cpp | 48 ++++++++++++++----- 4 files changed, 49 insertions(+), 18 deletions(-) diff --git a/include/openspace/performance/performancemanager.h b/include/openspace/performance/performancemanager.h index 6033a1e4c7..21e7c64b27 100644 --- a/include/openspace/performance/performancemanager.h +++ b/include/openspace/performance/performancemanager.h @@ -60,20 +60,20 @@ public: void outputLogs(); - void writeData(std::ofstream& out, const std::vector data); + void writeData(std::ofstream& out, const std::vector& data); - const std::string formatLogName(std::string nodeName); + std::string formatLogName(std::string nodeName); void logDir(std::string dir); - std::string logDir(); + std::string logDir() const; void prefix(std::string prefix); - std::string prefix(); + std::string prefix() const; void enableLogging(); void disableLogging(); void toggleLogging(); void setLogging(bool enabled); - bool loggingEnabled(); + bool loggingEnabled() const; PerformanceLayout* performanceData(); private: @@ -92,6 +92,7 @@ private: size_t _tick; void tick(); + bool createLogDir(); }; } // namespace performance diff --git a/modules/onscreengui/src/guiperformancecomponent.cpp b/modules/onscreengui/src/guiperformancecomponent.cpp index 4e8771d726..86893cc93e 100644 --- a/modules/onscreengui/src/guiperformancecomponent.cpp +++ b/modules/onscreengui/src/guiperformancecomponent.cpp @@ -86,6 +86,8 @@ void GuiPerformanceComponent::render() { v = _outputLogs; ImGui::Checkbox("Output Logs", &v); OsEng.renderEngine().performanceManager()->setLogging(v); + // Need to catch if it's unsuccessful + v = OsEng.renderEngine().performanceManager()->loggingEnabled(); _outputLogs = v; ImGui::Spacing(); diff --git a/src/engine/configurationmanager_doc.inl b/src/engine/configurationmanager_doc.inl index 45c60c8868..9446e54a16 100644 --- a/src/engine/configurationmanager_doc.inl +++ b/src/engine/configurationmanager_doc.inl @@ -89,13 +89,15 @@ documentation::Documentation ConfigurationManager::Documentation() { { ConfigurationManager::PartLogDir, new StringVerifier, - "The directory for logs", + "The directory for logs." + "Default value is \"${BASE_PATH}\"", Optional::Yes }, { ConfigurationManager::PartLogPerformancePrefix, new StringVerifier, - "A string to prefix PerformanceMeasurement logfiles", + "A string to prefix PerformanceMeasurement logfiles." + "Default value is \"PM-\"", Optional::Yes }, { diff --git a/src/performance/performancemanager.cpp b/src/performance/performancemanager.cpp index 6368ee352e..beaa9cba0d 100644 --- a/src/performance/performancemanager.cpp +++ b/src/performance/performancemanager.cpp @@ -137,14 +137,12 @@ PerformanceManager::PerformanceManager() , _loggingEnabled(false) , _logDir(absPath("${BASE_PATH}")) , _prefix("PM-") - , _suffix("") , _ext("log") { - using ghoul::SharedMemory; PerformanceManager::createGlobalSharedMemory(); - SharedMemory sharedMemory(GlobalSharedMemoryName); + ghoul::SharedMemory sharedMemory(GlobalSharedMemoryName); sharedMemory.acquireLock(); OnExit([&](){sharedMemory.releaseLock();}); @@ -160,7 +158,7 @@ PerformanceManager::PerformanceManager() const int totalSize = sizeof(PerformanceLayout); LINFO("Create shared memory '" + localName + "' of " << totalSize << " bytes"); - if (SharedMemory::exists(localName)) { + if (ghoul::SharedMemory::exists(localName)) { throw ghoul::RuntimeError( "Shared Memory '" + localName + "' block already existed" ); @@ -247,14 +245,14 @@ void PerformanceManager::outputLogs() { } } -void PerformanceManager::writeData(std::ofstream& out, const std::vector data) { +void PerformanceManager::writeData(std::ofstream& out, const std::vector& data) { for (size_t i = 0; i < data.size() - 1; i++) { out << data[i] << ","; } out << data[data.size() - 1] << "\n"; } -const std::string PerformanceManager::formatLogName(std::string nodeName) { + std::string PerformanceManager::formatLogName(std::string nodeName) { // Replace any colons with dashes std::replace(nodeName.begin(), nodeName.end(), ':', '-'); return _logDir + "/" + _prefix + nodeName + _suffix + "." + _ext; @@ -264,7 +262,7 @@ void PerformanceManager::logDir(std::string dir) { _logDir = absPath(dir); } -std::string PerformanceManager::logDir() { +std::string PerformanceManager::logDir() const { return _logDir; } @@ -272,7 +270,7 @@ void PerformanceManager::prefix(std::string prefix) { _prefix = prefix; } -std::string PerformanceManager::prefix() { +std::string PerformanceManager::prefix() const { return _prefix; } @@ -285,13 +283,39 @@ void PerformanceManager::disableLogging() { } void PerformanceManager::toggleLogging() { - _loggingEnabled = !_loggingEnabled; + setLogging(!_loggingEnabled); } + void PerformanceManager::setLogging(bool enabled) { + // Create the log directory if it doesn't exist. Do it here, so that it + // only tests once each time output is enabled + if (enabled) { + // If it can't create the directory, it's not logging so set false + enabled = createLogDir(); + } + _loggingEnabled = enabled; } -bool PerformanceManager::loggingEnabled() { +bool PerformanceManager::createLogDir() { + // Done if it exists + ghoul::filesystem::Directory dir(_logDir); + if (FileSys.directoryExists(dir)) { + return true; + } + + // Error and set false if can't create + try { + FileSys.createDirectory(dir, ghoul::filesystem::FileSystem::Recursive::Yes); + } + catch (const ghoul::filesystem::FileSystem::FileSystemException& e) { + LERROR("Could not create log directory: " << e.message); + return false; + } + return true; +} + +bool PerformanceManager::loggingEnabled() const { return _loggingEnabled; } @@ -404,7 +428,9 @@ void PerformanceManager::storeScenePerformanceMeasurements( } _performanceMemory->releaseLock(); - if (_loggingEnabled && _tick == PerformanceLayout::NumberValues - 1) outputLogs(); + if (_loggingEnabled && _tick == PerformanceLayout::NumberValues - 1) { + outputLogs(); + } tick(); }