From e3b7c2e6ee47fd4009cdb6ef4d346c33ad05b8ce Mon Sep 17 00:00:00 2001
From: Mark Olesen <Mark.Olesen@esi-group.com>
Date: Fri, 2 Feb 2024 15:28:05 +0100
Subject: [PATCH] ENH: replace masterUncollatedFileOperation::scatterList()

- use Pstream::listScatterValues() instead of the old hand-rolled
  method.

  Reduces code and since it is mostly used with primitives it
  will use MPI_Scatter directly (see #3087)

COMP: fix some inconsistent masterOp return types
---
 .../masterUncollatedFileOperation.C           |  46 ++++----
 .../masterUncollatedFileOperation.H           |   9 +-
 .../masterUncollatedFileOperationTemplates.C  | 110 +++++++-----------
 3 files changed, 69 insertions(+), 96 deletions(-)

diff --git a/src/OpenFOAM/global/fileOperations/masterUncollatedFileOperation/masterUncollatedFileOperation.C b/src/OpenFOAM/global/fileOperations/masterUncollatedFileOperation/masterUncollatedFileOperation.C
index 182b526706..ef9b1faac1 100644
--- a/src/OpenFOAM/global/fileOperations/masterUncollatedFileOperation/masterUncollatedFileOperation.C
+++ b/src/OpenFOAM/global/fileOperations/masterUncollatedFileOperation/masterUncollatedFileOperation.C
@@ -6,7 +6,7 @@
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
     Copyright (C) 2017-2018 OpenFOAM Foundation
-    Copyright (C) 2019-2023 OpenCFD Ltd.
+    Copyright (C) 2019-2024 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -875,11 +875,11 @@ bool Foam::fileOperations::masterUncollatedFileOperation::mkDir
     mode_t mode
 ) const
 {
-    return masterOp<mode_t>
+    return masterOp<bool>
     (
         dir,
         mkDirOp(mode),
-        Pstream::msgType(),
+        UPstream::msgType(),
         comm_
     );
 }
@@ -891,11 +891,11 @@ bool Foam::fileOperations::masterUncollatedFileOperation::chMod
     mode_t mode
 ) const
 {
-    return masterOp<mode_t>
+    return masterOp<bool>
     (
         fName,
         chModOp(mode),
-        Pstream::msgType(),
+        UPstream::msgType(),
         comm_
     );
 }
@@ -911,7 +911,7 @@ mode_t Foam::fileOperations::masterUncollatedFileOperation::mode
     (
         fName,
         modeOp(followLink),
-        Pstream::msgType(),
+        UPstream::msgType(),
         comm_
     );
 }
@@ -925,11 +925,11 @@ Foam::fileName::Type Foam::fileOperations::masterUncollatedFileOperation::type
 {
     return fileName::Type
     (
-        masterOp<label>
+        masterOp<int>
         (
             fName,
             typeOp(followLink),
-            Pstream::msgType(),
+            UPstream::msgType(),
             comm_
         )
     );
@@ -947,7 +947,7 @@ bool Foam::fileOperations::masterUncollatedFileOperation::exists
     (
         fName,
         existsOp(checkGzip, followLink),
-        Pstream::msgType(),
+        UPstream::msgType(),
         comm_
     );
 }
@@ -963,7 +963,7 @@ bool Foam::fileOperations::masterUncollatedFileOperation::isDir
     (
         fName,
         isDirOp(followLink),
-        Pstream::msgType(),
+        UPstream::msgType(),
         comm_
     );
 }
@@ -980,7 +980,7 @@ bool Foam::fileOperations::masterUncollatedFileOperation::isFile
     (
         fName,
         isFileOp(checkGzip, followLink),
-        Pstream::msgType(),
+        UPstream::msgType(),
         comm_
     );
 }
@@ -996,7 +996,7 @@ off_t Foam::fileOperations::masterUncollatedFileOperation::fileSize
     (
         fName,
         fileSizeOp(followLink),
-        Pstream::msgType(),
+        UPstream::msgType(),
         comm_
     );
 }
@@ -1012,7 +1012,7 @@ time_t Foam::fileOperations::masterUncollatedFileOperation::lastModified
     (
         fName,
         lastModifiedOp(followLink),
-        Pstream::msgType(),
+        UPstream::msgType(),
         UPstream::worldComm
     );
 }
@@ -1028,7 +1028,7 @@ double Foam::fileOperations::masterUncollatedFileOperation::highResLastModified
     (
         fName,
         highResLastModifiedOp(followLink),
-        Pstream::msgType(),
+        UPstream::msgType(),
         UPstream::worldComm
     );
 }
@@ -1044,7 +1044,7 @@ bool Foam::fileOperations::masterUncollatedFileOperation::mvBak
     (
         fName,
         mvBakOp(ext),
-        Pstream::msgType(),
+        UPstream::msgType(),
         comm_
     );
 }
@@ -1059,7 +1059,7 @@ bool Foam::fileOperations::masterUncollatedFileOperation::rm
     (
         fName,
         rmOp(),
-        Pstream::msgType(),
+        UPstream::msgType(),
         comm_
     );
 }
@@ -1076,7 +1076,7 @@ bool Foam::fileOperations::masterUncollatedFileOperation::rmDir
     (
         dir,
         rmDirOp(silent, emptyOnly),
-        Pstream::msgType(),
+        UPstream::msgType(),
         comm_
     );
 }
@@ -1094,7 +1094,7 @@ Foam::fileNameList Foam::fileOperations::masterUncollatedFileOperation::readDir
     (
         dir,
         readDirOp(type, filtergz, followLink),
-        Pstream::msgType(),
+        UPstream::msgType(),
         comm_
     );
 }
@@ -1112,7 +1112,7 @@ bool Foam::fileOperations::masterUncollatedFileOperation::cp
         src,
         dst,
         cpOp(followLink),
-        Pstream::msgType(),
+        UPstream::msgType(),
         comm_
     );
 }
@@ -1129,7 +1129,7 @@ bool Foam::fileOperations::masterUncollatedFileOperation::ln
         src,
         dst,
         lnOp(),
-        Pstream::msgType(),
+        UPstream::msgType(),
         comm_
     );
 }
@@ -1147,7 +1147,7 @@ bool Foam::fileOperations::masterUncollatedFileOperation::mv
         src,
         dst,
         mvOp(followLink),
-        Pstream::msgType(),
+        UPstream::msgType(),
         comm_
     );
 }
@@ -1293,7 +1293,7 @@ Foam::fileName Foam::fileOperations::masterUncollatedFileOperation::filePath
                 (
                     io.objectPath(),
                     fileOrNullOp(true), // isFile=true
-                    Pstream::msgType(),
+                    UPstream::msgType(),
                     comm_
                 );
             }
@@ -1451,7 +1451,7 @@ Foam::fileName Foam::fileOperations::masterUncollatedFileOperation::dirPath
                 (
                     io.objectPath(),
                     fileOrNullOp(false), // isFile=false
-                    Pstream::msgType(),
+                    UPstream::msgType(),
                     comm_
                 );
             }
diff --git a/src/OpenFOAM/global/fileOperations/masterUncollatedFileOperation/masterUncollatedFileOperation.H b/src/OpenFOAM/global/fileOperations/masterUncollatedFileOperation/masterUncollatedFileOperation.H
index b705312b4f..c2fff0d611 100644
--- a/src/OpenFOAM/global/fileOperations/masterUncollatedFileOperation/masterUncollatedFileOperation.H
+++ b/src/OpenFOAM/global/fileOperations/masterUncollatedFileOperation/masterUncollatedFileOperation.H
@@ -6,7 +6,7 @@
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
     Copyright (C) 2017 OpenFOAM Foundation
-    Copyright (C) 2019-2023 OpenCFD Ltd.
+    Copyright (C) 2019-2024 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -162,8 +162,8 @@ protected:
                 followLink_(followLink)
             {}
 
-            // Returns label not fileName::Type for reductions
-            label operator()(const fileName& f) const
+            // Returns int (for reductions) instead of fileName::Type
+            int operator()(const fileName& f) const
             {
                 return Foam::type(f, followLink_);
             }
@@ -390,9 +390,6 @@ protected:
 
     // Private Member Functions
 
-        template<class Type>
-        Type scatterList(const UList<Type>&, const int, const label comm) const;
-
         template<class Type, class FileOp>
         Type masterOp
         (
diff --git a/src/OpenFOAM/global/fileOperations/masterUncollatedFileOperation/masterUncollatedFileOperationTemplates.C b/src/OpenFOAM/global/fileOperations/masterUncollatedFileOperation/masterUncollatedFileOperationTemplates.C
index 7ff45acaca..949676b0f3 100644
--- a/src/OpenFOAM/global/fileOperations/masterUncollatedFileOperation/masterUncollatedFileOperationTemplates.C
+++ b/src/OpenFOAM/global/fileOperations/masterUncollatedFileOperation/masterUncollatedFileOperationTemplates.C
@@ -6,7 +6,7 @@
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
     Copyright (C) 2017-2018 OpenFOAM Foundation
-    Copyright (C) 2020-2022 OpenCFD Ltd.
+    Copyright (C) 2020-2024 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -31,41 +31,6 @@ License
 
 // * * * * * * * * * * * * * * * Member Functions  * * * * * * * * * * * * * //
 
-template<class Type>
-Type Foam::fileOperations::masterUncollatedFileOperation::scatterList
-(
-    const UList<Type>& allValues,
-    const int tag,
-    const label comm
-) const
-{
-    // TBD: more efficient scatter
-    PstreamBuffers pBufs(UPstream::commsTypes::nonBlocking, tag, comm);
-    if (Pstream::master(comm))
-    {
-        for (const int proci : Pstream::subProcs(comm))
-        {
-            UOPstream os(proci, pBufs);
-            os << allValues[proci];
-        }
-    }
-    pBufs.finishedScatters();
-
-    Type value;
-
-    if (Pstream::master(comm))
-    {
-        value = allValues[0];
-    }
-    else
-    {
-        UIPstream is(Pstream::masterNo(), pBufs);
-        is >> value;
-    }
-    return value;
-}
-
-
 template<class Type, class FileOp>
 Type Foam::fileOperations::masterUncollatedFileOperation::masterOp
 (
@@ -81,17 +46,24 @@ Type Foam::fileOperations::masterUncollatedFileOperation::masterOp
             << typeid(FileOp).name()
             << " on " << fName << endl;
     }
-    if (Pstream::parRun())
-    {
-        List<fileName> filePaths(Pstream::nProcs(comm));
-        filePaths[Pstream::myProcNo(comm)] = fName;
-        Pstream::gatherList(filePaths, tag, comm);
 
-        List<Type> result(filePaths.size());
-        if (Pstream::master(comm))
+    if (UPstream::is_parallel(comm))
+    {
+        const label myProci = UPstream::myProcNo(comm);
+        const label numProc = UPstream::nProcs(comm);
+
+        List<fileName> filePaths(numProc);
+        filePaths[myProci] = fName;
+        Pstream::gatherList(filePaths, tag, comm);
+        // OR filePaths = Pstream::listGatherValues(fName, comm, tag)
+
+        List<Type> result;
+        if (UPstream::master(comm))
         {
+            result.resize(numProc);
             result = fop(filePaths[0]);
-            for (label i = 1; i < filePaths.size(); i++)
+
+            for (label i = 1; i < numProc; ++i)
             {
                 if (filePaths[i] != filePaths[0])
                 {
@@ -100,12 +72,10 @@ Type Foam::fileOperations::masterUncollatedFileOperation::masterOp
             }
         }
 
-        return scatterList(result, tag, comm);
-    }
-    else
-    {
-        return fop(fName);
+        return Pstream::listScatterValues(result, comm, tag);
     }
+
+    return fop(fName);
 }
 
 
@@ -124,35 +94,41 @@ Type Foam::fileOperations::masterUncollatedFileOperation::masterOp
         Pout<< "masterUncollatedFileOperation : Operation on src:" << src
             << " dest:" << dest << endl;
     }
-    if (Pstream::parRun())
+
+    if (UPstream::is_parallel(comm))
     {
-        List<fileName> srcs(Pstream::nProcs(comm));
-        srcs[Pstream::myProcNo(comm)] = src;
-        Pstream::gatherList(srcs, tag, comm);
+        const label myProci = UPstream::myProcNo(comm);
+        const label numProc = UPstream::nProcs(comm);
 
-        List<fileName> dests(srcs.size());
-        dests[Pstream::myProcNo(comm)] = dest;
-        Pstream::gatherList(dests, tag, comm);
+        List<Pair<fileName>> filePaths(numProc);
+        filePaths[myProci].first() = src;
+        filePaths[myProci].second() = dest;
+        Pstream::gatherList(filePaths, tag, comm);
+        // OR
+        // Pair<fileName> tup(src, dest);
+        // filePaths = Pstream::listGatherValues(tup, comm, tag)
 
-        List<Type> result(Pstream::nProcs(comm));
-        if (Pstream::master(comm))
+        List<Type> result;
+        if (UPstream::master(comm))
         {
-            result = fop(srcs[0], dests[0]);
-            for (label i = 1; i < srcs.size(); i++)
+            result.resize(numProc);
+            result = fop(filePaths[0].first(), filePaths[0].second());
+
+            for (label i = 1; i < numProc; ++i)
             {
-                if (srcs[i] != srcs[0])
+                // TBD: also check second() ?
+                if (filePaths[i].first() != filePaths[0].first())
                 {
-                    result[i] = fop(srcs[i], dests[i]);
+                    result[i] =
+                        fop(filePaths[i].first(), filePaths[i].second());
                 }
             }
         }
 
-        return scatterList(result, tag, comm);
-    }
-    else
-    {
-        return fop(src, dest);
+        return Pstream::listScatterValues(result, comm, tag);
     }
+
+    return fop(src, dest);
 }