From abc60d4d20fcccb6c47238e862fd0c498e6a70df Mon Sep 17 00:00:00 2001 From: Mark Olesen Date: Tue, 6 Dec 2022 10:47:48 +0100 Subject: [PATCH] ENH: use refPtr to manage current fileHandler - for special cases it can simplify sharing of processor communication patterns, but no visible change for most code. - make fileHandler communicator modifiable (mutable), for special cases. The changes from 9711b7f1b941 now make this safer to do. Continue to support legacy global function using an autoPtr: autoPtr Foam::fileHandler(autoPtr&&); However, new code using refPtr uses the following static method since swapping out file handlers is an infrequent operation that should also stand out a bit more. fileOperation::fileHandler(...); --- src/OpenFOAM/Make/files | 1 + src/OpenFOAM/db/Time/TimeIO.C | 16 +- src/OpenFOAM/global/argList/argList.C | 7 +- .../fileOperation/fileOperation.C | 121 +---------- .../fileOperation/fileOperation.H | 127 +++++++++--- .../fileOperation/fileOperationNew.C | 195 ++++++++++++++++++ 6 files changed, 311 insertions(+), 156 deletions(-) create mode 100644 src/OpenFOAM/global/fileOperations/fileOperation/fileOperationNew.C diff --git a/src/OpenFOAM/Make/files b/src/OpenFOAM/Make/files index 93f42ea300..f2236c6352 100644 --- a/src/OpenFOAM/Make/files +++ b/src/OpenFOAM/Make/files @@ -19,6 +19,7 @@ global/etcFiles/etcFiles.C fileOps = global/fileOperations $(fileOps)/fileOperation/fileOperation.C $(fileOps)/fileOperation/fileOperationBroadcast.C +$(fileOps)/fileOperation/fileOperationNew.C $(fileOps)/fileOperationInitialise/fileOperationInitialise.C $(fileOps)/uncollatedFileOperation/uncollatedFileOperation.C $(fileOps)/masterUncollatedFileOperation/masterUncollatedFileOperation.C diff --git a/src/OpenFOAM/db/Time/TimeIO.C b/src/OpenFOAM/db/Time/TimeIO.C index 5f54fc28dc..3a7fa55ae4 100644 --- a/src/OpenFOAM/db/Time/TimeIO.C +++ b/src/OpenFOAM/db/Time/TimeIO.C @@ -166,18 +166,20 @@ void Foam::Time::readDict() } controlDict_.watchIndices().clear(); - // The new handler, with verbosity - autoPtr newHandler = - fileOperation::New(fileHandlerName, true); + // The new handler, create with some verbosity + refPtr newHandler + ( + fileOperation::New(fileHandlerName, true) + ); - if (TimePaths::distributed() && newHandler) + // Install the new handler + (void) fileOperation::fileHandler(newHandler); + + if (TimePaths::distributed()) { newHandler->distributed(true); } - // Installing the new handler - Foam::fileHandler(std::move(newHandler)); - // Reinstall old watches fileHandler().addWatches(controlDict_, oldWatched); } diff --git a/src/OpenFOAM/global/argList/argList.C b/src/OpenFOAM/global/argList/argList.C index 9478b95bf3..a19a57e842 100644 --- a/src/OpenFOAM/global/argList/argList.C +++ b/src/OpenFOAM/global/argList/argList.C @@ -1231,7 +1231,10 @@ void Foam::argList::parse handlerType = fileOperation::defaultFileHandler; } - Foam::fileHandler(fileOperation::New(handlerType, bannerEnabled())); + (void) fileOperation::fileHandler + ( + fileOperation::New(handlerType, bannerEnabled()) + ); } @@ -1830,7 +1833,7 @@ Foam::argList::~argList() jobInfo.stop(); // Normal job termination // Delete file handler to flush any remaining IO - (void)Foam::fileHandler(nullptr); + (void) fileOperation::fileHandler(nullptr); } diff --git a/src/OpenFOAM/global/fileOperations/fileOperation/fileOperation.C b/src/OpenFOAM/global/fileOperations/fileOperation/fileOperation.C index 4fcef621cf..a69acf2514 100644 --- a/src/OpenFOAM/global/fileOperations/fileOperation/fileOperation.C +++ b/src/OpenFOAM/global/fileOperations/fileOperation/fileOperation.C @@ -27,7 +27,6 @@ License \*---------------------------------------------------------------------------*/ #include "fileOperation.H" -#include "uncollatedFileOperation.H" #include "regIOobject.H" #include "argList.H" #include "HashSet.H" @@ -81,8 +80,6 @@ Foam::fileOperation::pathTypeNames_ Foam::word Foam::fileOperation::processorsBaseDir = "processors"; -Foam::autoPtr Foam::fileOperation::fileHandlerPtr_; - // * * * * * * * * * * * * * * * Local Functions * * * * * * * * * * * * * * // @@ -219,6 +216,7 @@ void sortProcessorDirs(Foam::UList& dirs) } // End anonymous namespace #endif + // * * * * * * * * * * * * * Static Member Functions * * * * * * * * * * * * // Foam::labelList Foam::fileOperation::ioRanks() @@ -739,54 +737,8 @@ Foam::fileOperation::fileOperation {} -Foam::autoPtr -Foam::fileOperation::New -( - const word& handlerType, - bool verbose -) -{ - if (handlerType.empty()) - { - if (fileOperation::defaultFileHandler.empty()) - { - FatalErrorInFunction - << "defaultFileHandler name is undefined" << nl - << abort(FatalError); - } - - return fileOperation::New(fileOperation::defaultFileHandler, verbose); - } - - DebugInFunction - << "Constructing fileHandler" << endl; - - auto* ctorPtr = wordConstructorTable(handlerType); - - if (!ctorPtr) - { - FatalErrorInLookup - ( - "fileHandler", - handlerType, - *wordConstructorTablePtr_ - ) << abort(FatalError); - } - - return autoPtr(ctorPtr(verbose)); -} - - // * * * * * * * * * * * * * * * Member Functions * * * * * * * * * * * * * // -bool Foam::fileOperation::distributed(bool on) const noexcept -{ - bool old(distributed_); - distributed_ = on; - return old; -} - - Foam::fileName Foam::fileOperation::objectPath ( const IOobject& io, @@ -1542,75 +1494,4 @@ Foam::label Foam::fileOperation::detectProcessorPath(const fileName& fName) } -// * * * * * * * * * * * * * Static Member Functions * * * * * * * * * * * * // - -Foam::autoPtr Foam::fileOperation::NewUncollated() -{ - return autoPtr - ( - new fileOperations::uncollatedFileOperation(false) - ); -} - - -// * * * * * * * * * * * * * * * Global Functions * * * * * * * * * * * * * // - -const Foam::fileOperation& Foam::fileHandler() -{ - if (!fileOperation::fileHandlerPtr_) - { - word handlerType(Foam::getEnv("FOAM_FILEHANDLER")); - - if (handlerType.empty()) - { - handlerType = fileOperation::defaultFileHandler; - } - - fileOperation::fileHandlerPtr_ = fileOperation::New(handlerType, true); - } - - return *fileOperation::fileHandlerPtr_; -} - - -Foam::autoPtr -Foam::fileHandler(std::nullptr_t) -{ - return autoPtr(fileOperation::fileHandlerPtr_.release()); -} - - -Foam::autoPtr -Foam::fileHandler(autoPtr&& newHandler) -{ - // - do nothing if newHandler is empty. Does not delete current - // - do nothing if newHandler is identical to current handler - - // Change ownership as atomic operations - - // If newHandler and current handler are actually identical, we - // have a bit problem somewhere else since this means that the pointer - // is managed is done in two places! - // Should flag as a FatalError (in the future), but there may still be - // some place where we would like to fake shared pointers? - - // TBD: add a flush() operation on the old handler first, - // instead of waiting for it to be run on destruction? - - autoPtr old; - - if - ( - newHandler.get() != nullptr - && newHandler.get() != fileOperation::fileHandlerPtr_.get() - ) - { - old.reset(newHandler.release()); - old.swap(fileOperation::fileHandlerPtr_); - } - - return old; -} - - // ************************************************************************* // diff --git a/src/OpenFOAM/global/fileOperations/fileOperation/fileOperation.H b/src/OpenFOAM/global/fileOperations/fileOperation/fileOperation.H index 7f020e8878..a62ad6644c 100644 --- a/src/OpenFOAM/global/fileOperations/fileOperation/fileOperation.H +++ b/src/OpenFOAM/global/fileOperations/fileOperation/fileOperation.H @@ -43,10 +43,9 @@ Description #include "ISstream.H" #include "Ostream.H" -#include "autoPtr.H" +#include "fileMonitor.H" #include "fileNameList.H" #include "instantList.H" -#include "fileMonitor.H" #include "refPtr.H" #include "Enum.H" #include "Tuple2.H" @@ -105,13 +104,12 @@ public: //- identical to UPstream::rangeType typedef IntRange procRangeType; - protected: // Protected Data //- Communicator to use - const label comm_; + mutable label comm_; //- Distributed roots (parallel run) mutable bool distributed_; @@ -120,7 +118,7 @@ protected: mutable HashTable procsDirs_; //- File-change monitor for all registered files - mutable autoPtr monitorPtr_; + mutable std::unique_ptr monitorPtr_; // Protected Member Functions @@ -180,11 +178,8 @@ public: TypeName("fileOperation"); - //- Static fileOperation - static autoPtr fileHandlerPtr_; - - //- Static construct the commonly used uncollatedFileOperation - static autoPtr NewUncollated(); + //- The currently active file handler. Avoid accessing directly + static refPtr fileHandlerPtr_; // Constructors @@ -229,6 +224,60 @@ public: virtual ~fileOperation() = default; + // Factory Methods, Singleton-type Functions + + //- The commonly used uncollatedFileOperation + static autoPtr NewUncollated(); + + //- Return the current file handler. + //- Will create the default file handler if necessary. + static const fileOperation& fileHandler(); + + //- Delete current file handler. + // \returns the old handler. + // Should have [[nodiscard]], but gcc ignores void casting. + static refPtr fileHandler(std::nullptr_t); + + //- Replace the current file handler. + // The following are considered no-ops: + // - an empty/invalid newHandler does \b not delete, use a literal + // \c nullptr (std::nullptr_t) for that + // - if new handler and current handler are identical (same pointer). + // . + // \returns the old handler (on change), nullptr otherwise + // Should have [[nodiscard]], but gcc ignores void casting. + static refPtr fileHandler + ( + refPtr& newHandler + ); + + //- Replace the current file handler. + // The following are considered no-ops: + // - an empty/invalid newHandler does \b not delete, use a literal + // \c nullptr (std::nullptr_t) for that + // - if new handler and current handler are identical (same pointer). + // . + // \returns the old handler (on change), nullptr otherwise + // Should have [[nodiscard]], but gcc ignores void casting. + static refPtr fileHandler + ( + refPtr&& newHandler + ); + + //- Replace the current file handler. + // The following are considered no-ops: + // - an empty/invalid newHandler does \b not delete, use a literal + // \c nullptr (std::nullptr_t) for that + // - if new handler and current handler are identical (same pointer). + // . + // \returns the old handler (on change), nullptr otherwise + // Should have [[nodiscard]], but gcc ignores void casting. + static refPtr fileHandler + ( + autoPtr&& newHandler + ); + + // Static Functions //- Sort directory entries according to time value, @@ -248,17 +297,41 @@ public: // Member Functions + // Characteristics + + //- Communicator to use + label comm() const noexcept + { + return comm_; + } + + //- Set communicator to use [mutable]. Negative values are a no-op. + // \return old value + label comm(label communicator) const noexcept + { + label old(comm_); + if (communicator >= 0) comm_ = communicator; + return old; + } + //- Distributed roots (parallel run) bool distributed() const noexcept { return distributed_; } - //- Set distributed roots on/off (mutable) + //- Set distributed roots on/off [mutable] // \return old value - bool distributed(bool on) const noexcept; + bool distributed(bool on) const noexcept + { + bool old(distributed_); + distributed_ = on; + return old; + } + // Member Functions + // OSSpecific equivalents //- Make directory @@ -653,23 +726,23 @@ inline Ostream& operator<<(Ostream& os, const fileOperation::pathType b) // * * * * * * * * * * * * * * * Global Functions * * * * * * * * * * * * * // -// Note: defined in fileOperation.C +//- Return the current file handler +//- (will create default file handler if necessary). +//- Forwards to fileOperation::handler() +inline const fileOperation& fileHandler() +{ + return fileOperation::fileHandler(); +} -//- Return the current file handler. -//- Will create the default file handler if necessary. -const fileOperation& fileHandler(); +//- Delete current file handler - forwards to fileOperation::handler() +// Should have [[nodiscard]], but gcc ignores void casting. +inline refPtr fileHandler(std::nullptr_t) +{ + return fileOperation::fileHandler(nullptr); +} -//- Delete current file handler. -// \returns the old handler -autoPtr fileHandler(std::nullptr_t); - -//- Replace the current file handler. -// The following are considered no-ops: -// - an empty/invalid newHandler does \b not delete, -// use fileHandler(std::nullptr_t) - ie, a literal \c nullptr for that -// - if new handler and current handler are identical (same pointer). -// . -// \returns the old handler (on change), nullptr otherwise +//- Replace the current file handler - forwards to fileOperation::handler(). +// \note legacy behaviour, so returns autoPtr instead of refPtr! autoPtr fileHandler(autoPtr&& newHandler); diff --git a/src/OpenFOAM/global/fileOperations/fileOperation/fileOperationNew.C b/src/OpenFOAM/global/fileOperations/fileOperation/fileOperationNew.C new file mode 100644 index 0000000000..ef7da7d6dd --- /dev/null +++ b/src/OpenFOAM/global/fileOperations/fileOperation/fileOperationNew.C @@ -0,0 +1,195 @@ +/*---------------------------------------------------------------------------*\ + ========= | + \\ / F ield | OpenFOAM: The Open Source CFD Toolbox + \\ / O peration | + \\ / A nd | www.openfoam.com + \\/ M anipulation | +------------------------------------------------------------------------------- + Copyright (C) 2022 OpenCFD Ltd. +------------------------------------------------------------------------------- +License + This file is part of OpenFOAM. + + OpenFOAM is free software: you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + OpenFOAM is distributed in the hope that it will be useful, but WITHOUT + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + for more details. + + You should have received a copy of the GNU General Public License + along with OpenFOAM. If not, see . + +\*---------------------------------------------------------------------------*/ + +#include "fileOperation.H" +#include "uncollatedFileOperation.H" + +// * * * * * * * * * * * * * * Static Data Members * * * * * * * * * * * * * // + +Foam::refPtr Foam::fileOperation::fileHandlerPtr_; + + +// * * * * * * * * * * * * * Static Member Functions * * * * * * * * * * * * // + +const Foam::fileOperation& Foam::fileOperation::fileHandler() +{ + if (!fileOperation::fileHandlerPtr_) + { + word handlerType(Foam::getEnv("FOAM_FILEHANDLER")); + + if (handlerType.empty()) + { + handlerType = defaultFileHandler; + } + + fileOperation::fileHandlerPtr_ = fileOperation::New(handlerType, true); + } + + return *fileOperation::fileHandlerPtr_; +} + + +Foam::refPtr +Foam::fileOperation::fileHandler(std::nullptr_t) +{ + return refPtr(std::move(fileHandlerPtr_)); +} + + +Foam::refPtr +Foam::fileOperation::fileHandler(refPtr& newHandler) +{ + // - do nothing if newHandler is empty. Does not delete current + // - do nothing if newHandler is identical to current handler + + // Change ownership as atomic operations + + // If newHandler and current handler are actually identical, we + // have a bit problem somewhere else since this means that the pointer + // is managed is done in two places! + // Should flag as a FatalError (in the future), but there may still be + // some place where we would like to fake shared pointers? + + // TBD: add a flush() operation on the old handler first, + // instead of waiting for it to be run on destruction? + + refPtr old; + + if + ( + newHandler.get() != nullptr + && newHandler.get() != fileOperation::fileHandlerPtr_.get() + ) + { + old.swap(newHandler); + old.swap(fileOperation::fileHandlerPtr_); + } + + return old; +} + + +Foam::refPtr +Foam::fileOperation::fileHandler(autoPtr&& newHandler) +{ + // Same logic as refPtr version + + refPtr old; + + if + ( + newHandler.get() != nullptr + && newHandler.get() != fileOperation::fileHandlerPtr_.get() + ) + { + old.reset(newHandler.release()); + old.swap(fileOperation::fileHandlerPtr_); + } + + return old; +} + + +Foam::refPtr +Foam::fileOperation::fileHandler(refPtr&& newHandler) +{ + return fileOperation::fileHandler(newHandler); +} + + +Foam::autoPtr Foam::fileOperation::NewUncollated() +{ + return autoPtr + ( + new fileOperations::uncollatedFileOperation(false) + ); +} + + +// * * * * * * * * * * * * * * * * Selectors * * * * * * * * * * * * * * * * // + +Foam::autoPtr +Foam::fileOperation::New +( + const word& handlerType, + bool verbose +) +{ + if (handlerType.empty()) + { + if (fileOperation::defaultFileHandler.empty()) + { + FatalErrorInFunction + << "Default file-handler name is undefined" << nl + << abort(FatalError); + } + + return fileOperation::New(fileOperation::defaultFileHandler, verbose); + } + + DebugInFunction + << "Constructing fileHandler" << endl; + + auto* ctorPtr = wordConstructorTable(handlerType); + + if (!ctorPtr) + { + FatalErrorInLookup + ( + "fileHandler", + handlerType, + *wordConstructorTablePtr_ + ) << abort(FatalError); + } + + return autoPtr(ctorPtr(verbose)); +} + + +// * * * * * * * * * * * * * * * Global Functions * * * * * * * * * * * * * // + +Foam::autoPtr +Foam::fileHandler(autoPtr&& newHandler) +{ + refPtr oldHandler + ( + fileOperation::fileHandler(std::move(newHandler)) + ); + + autoPtr old; + + // Can return as autoPtr if handler was also a pointer (not a reference) + if (oldHandler.is_pointer()) + { + old.reset(oldHandler.release()); + } + + return old; +} + + +// ************************************************************************* //