From 424600746803a54fb4f8a7e4e3ebdcbd454a6ebd Mon Sep 17 00:00:00 2001 From: Alex Lindsay Date: Wed, 26 Jun 2024 07:06:33 -0700 Subject: [PATCH 1/3] Make ConsoleStream thread-safe Use the same mutex for both `operator<<` overloads. Refs CIVET failure https://civet.inl.gov/job/2295949/ on libmesh/libmesh#3883 --- framework/include/outputs/ConsoleStream.h | 5 +++-- framework/src/outputs/ConsoleStream.C | 6 ++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/framework/include/outputs/ConsoleStream.h b/framework/include/outputs/ConsoleStream.h index db38c514595f..fc97a3fb9ac6 100644 --- a/framework/include/outputs/ConsoleStream.h +++ b/framework/include/outputs/ConsoleStream.h @@ -96,9 +96,10 @@ class ConsoleStream /// of something in AutomaticMortarGeneration that requires /// this to be trivially copyable. mutable std::shared_ptr _oss; -}; -extern std::mutex _stream_mutex; + /// Mutex to prevent concurrent read/writes, write/writes + static std::mutex _stream_mutex; +}; template const ConsoleStream & diff --git a/framework/src/outputs/ConsoleStream.C b/framework/src/outputs/ConsoleStream.C index 43da52150cc8..28e5f3d5f843 100644 --- a/framework/src/outputs/ConsoleStream.C +++ b/framework/src/outputs/ConsoleStream.C @@ -12,19 +12,17 @@ #include "MooseUtils.h" #include "OutputWarehouse.h" -std::mutex _stream_mutex; +std::mutex ConsoleStream::_stream_mutex; ConsoleStream::ConsoleStream(OutputWarehouse & output_warehouse) : _output_warehouse(output_warehouse), _oss(std::make_shared()) { } -static std::mutex manip_mutex; - const ConsoleStream & ConsoleStream::operator<<(const StandardEndLine & manip) const { - const std::lock_guard lock(manip_mutex); + const std::lock_guard lock(_stream_mutex); if (manip == (std::basic_ostream & (*)(std::basic_ostream &)) & std::endl) (*_oss) << '\n'; From 1cfee76385e1fa643c428760fa6e29c51fd2c11e Mon Sep 17 00:00:00 2001 From: Alex Lindsay Date: Wed, 26 Jun 2024 14:11:53 -0700 Subject: [PATCH 2/3] Don't use underscore for method local variable --- framework/include/interfaces/SolutionInvalidInterface.h | 2 +- framework/src/interfaces/SolutionInvalidInterface.C | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/framework/include/interfaces/SolutionInvalidInterface.h b/framework/include/interfaces/SolutionInvalidInterface.h index 82c66e8a28cc..c1029ef53bae 100644 --- a/framework/include/interfaces/SolutionInvalidInterface.h +++ b/framework/include/interfaces/SolutionInvalidInterface.h @@ -33,7 +33,7 @@ class SolutionInvalidInterface SolutionInvalidInterface(MooseObject * const moose_object); protected: - void flagInvalidSolutionInternal(InvalidSolutionID _invalid_solution_id) const; + void flagInvalidSolutionInternal(InvalidSolutionID invalid_solution_id) const; // Register invalid solution with a message InvalidSolutionID registerInvalidSolutionInternal(const std::string & message) const; diff --git a/framework/src/interfaces/SolutionInvalidInterface.C b/framework/src/interfaces/SolutionInvalidInterface.C index 063e215c6344..7ba22a0897ad 100644 --- a/framework/src/interfaces/SolutionInvalidInterface.C +++ b/framework/src/interfaces/SolutionInvalidInterface.C @@ -23,12 +23,12 @@ SolutionInvalidInterface::SolutionInvalidInterface(MooseObject * const moose_obj /// Set solution invalid mark for the given solution ID void -SolutionInvalidInterface::flagInvalidSolutionInternal(InvalidSolutionID _invalid_solution_id) const +SolutionInvalidInterface::flagInvalidSolutionInternal(InvalidSolutionID invalid_solution_id) const { auto & solution_invalidity = _si_moose_object.getMooseApp().solutionInvalidity(); if (_si_problem.immediatelyPrintInvalidSolution()) - solution_invalidity.printDebug(_invalid_solution_id); - return solution_invalidity.flagInvalidSolutionInternal(_invalid_solution_id); + solution_invalidity.printDebug(invalid_solution_id); + return solution_invalidity.flagInvalidSolutionInternal(invalid_solution_id); } InvalidSolutionID From 174d8a92154a459c8614c46c5663d61b50d82baa Mon Sep 17 00:00:00 2001 From: Alex Lindsay Date: Wed, 26 Jun 2024 14:22:18 -0700 Subject: [PATCH 3/3] Use a deque instead of a vector for registry items This way when we release references into the wild we don't have to worry about them ever being invalidated --- framework/include/utils/GeneralRegistry.h | 17 ++--------------- .../src/utils_nonunity/PerfGraphRegistry.C | 4 ---- 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/framework/include/utils/GeneralRegistry.h b/framework/include/utils/GeneralRegistry.h index b7ba5e514742..513f4ef920ba 100644 --- a/framework/include/utils/GeneralRegistry.h +++ b/framework/include/utils/GeneralRegistry.h @@ -11,7 +11,7 @@ #include #include -#include +#include #include "MooseError.h" @@ -63,18 +63,13 @@ class GeneralRegistry template std::size_t registerItem(const Key & key, CreateItem & create_item); - /** - * Reserves \p size entires in the item vector, _id_to_item - */ - void reserve(const std::size_t size); - /// The name of this registry; used in error handling const std::string _name; /// Map of keys to IDs std::unordered_map _key_to_id; /// Vector of IDs to Items - std::vector _id_to_item; + std::deque _id_to_item; /// Mutex for locking access to _key_to_id /// NOTE: These can be changed to shared_mutexes once we get C++17 @@ -160,11 +155,3 @@ GeneralRegistry::registerItem(const Key & key, CreateItem & _id_to_item.emplace_back(std::move(create_item(id))); return id; } - -template -void -GeneralRegistry::reserve(const std::size_t size) -{ - std::lock_guard lock(_id_to_item_mutex); - _id_to_item.reserve(size); -} diff --git a/framework/src/utils_nonunity/PerfGraphRegistry.C b/framework/src/utils_nonunity/PerfGraphRegistry.C index d3342c65cc6b..ca1ae701dbb7 100644 --- a/framework/src/utils_nonunity/PerfGraphRegistry.C +++ b/framework/src/utils_nonunity/PerfGraphRegistry.C @@ -28,10 +28,6 @@ getPerfGraphRegistry() PerfGraphRegistry::PerfGraphRegistry() : GeneralRegistry("PerfGraphRegistry") { - // Reserve space so that re-allocation doesn't need to happen much - // This does not take much memory and, for most cases, will keep a single - // reallocation from happening - reserve(5000); } unsigned int