fix(oiiotool): Allow thread control for --parallel-frames (#4818)

Fixes #4811

This patch makes oiiotool's initial cursory scan of the command line
notice a `--threads` and carfully honor it when parallelizing the frame
loop.

Explanatory background info:

The cited issue shows something subtle happening with oiiotool's
`--parallel-frames` argument. The symptom was that it was using too many
threads and running out of memory, and seemed impervious to the use of
`--threads` to properly control it.

The reason is that oiiotool first scans the command line args in a
cursory way looking for signs of a "sequence", like a `--frames` or any
filename containing frame wildcards (like `linear.%04d.exr`) and also
noting if there is a `--parallel-frames`, but not actually taking any
actions for any of the commands yet.

Having seen that there are frame wildcards, what it then does is execute
the whole command line, once for each frame number that matched the
pattern. And since it noted that a `--parallel-frames` argument was
present somewhere, it does it with a parallel for loop. But how many
threads does it use? Well, enough for all the cores, since in some sense
it has not yet noticed the `--threads` argument. Moving the `--threads`
to be placed prior to the `--parallel-frames` doesn't help, either.

The suggested workaround was to set the default number of threads with
the environment variable `OPENIMAGEIO_THREADS` prior to launching
oiiotool. But this patch is a more robust solution.

Signed-off-by: Larry Gritz <lg@larrygritz.com>

Author: lgritz

src/oiiotool/oiiotool.cpp

@@ -28,6 +28,7 @@
 #include <OpenImageIO/deepdata.h>
 #include <OpenImageIO/filesystem.h>
 #include <OpenImageIO/filter.h>
+#include <OpenImageIO/fmath.h>
 #include <OpenImageIO/imagebuf.h>
 #include <OpenImageIO/imagebufalgo.h>
 #include <OpenImageIO/imagebufalgo_util.h>
@@ -616,9 +617,11 @@ Oiiotool::extract_options(string_view command)
 
 // --threads
 static void
-set_threads(Oiiotool&, cspan<const char*> argv)
+set_threads(Oiiotool& ot, cspan<const char*> argv)
 {
     OIIO_DASSERT(argv.size() == 2);
+    if (ot.in_parallel_frame_loop())
+        return;
     int nthreads = Strutil::stoi(argv[1]);
     OIIO::attribute("threads", nthreads);
     OIIO::attribute("exr_threads", nthreads);
@@ -7228,9 +7231,10 @@ one_sequence_iteration(Oiiotool& otmain, size_t i, int frame_number,
     }
 
     Oiiotool otit;  // Oiiotool for this iteration
-    otit.imagecache      = otmain.imagecache;
-    otit.frame_number    = frame_number;
-    otit.parent_oiiotool = &otmain;
+    otit.imagecache               = otmain.imagecache;
+    otit.frame_number             = frame_number;
+    otit.parent_oiiotool          = &otmain;
+    otit.m_in_parallel_frame_loop = otmain.in_parallel_frame_loop();
     otit.getargs((int)seq_argv.size(), (char**)&seq_argv[0]);
 
     if (otit.ap.aborted()) {
@@ -7305,8 +7309,9 @@ handle_sequence(Oiiotool& ot, int argc, const char** argv)
     int framepadding = 0;
     std::vector<int> sequence_args;  // Args with sequence numbers
     std::vector<bool> sequence_is_output;
-    bool is_sequence = false;
-    bool wildcard_on = true;
+    int parallel_frame_threads = OIIO::get_int_attribute("threads");
+    bool is_sequence           = false;
+    bool wildcard_on           = true;
     for (int a = 1; a < argc; ++a) {
         bool is_output     = false;
         bool is_output_all = false;
@@ -7336,6 +7341,11 @@ handle_sequence(Oiiotool& ot, int argc, const char** argv)
         } else if ((strarg == "--views" || strarg == "-views")
                    && a < argc - 1) {
             Strutil::split(argv[++a], views, ",");
+        } else if ((strarg == "--threads" || strarg == "-threads")
+                   && a < argc - 1) {
+            int t = Strutil::stoi(argv[++a]);
+            t     = OIIO::clamp(t, 0, int(Sysutil::hardware_concurrency()));
+            parallel_frame_threads = t;
         } else if (strarg == "--wildcardoff" || strarg == "-wildcardoff") {
             wildcard_on = false;
         } else if (strarg == "--parallel-frames"
@@ -7441,17 +7451,21 @@ handle_sequence(Oiiotool& ot, int argc, const char** argv)
     // every time.
     // Note: nfilenames really means, number of frame number iterations.
     if (ot.parallel_frames) {
-        // If --parframes was used, run the iterations in parallel.
+        // If --parframes was used, run the iterations in parallel, but
+        // each iteration should itself not try to internally parallelize.
         if (ot.debug)
-            print("Running {} frames in parallel\n", nfilenames);
+            print("Running {} frames in parallel with {} threads\n", nfilenames,
+                  parallel_frame_threads);
+        ot.begin_parallel_frame_loop(parallel_frame_threads);
         parallel_for(
             uint64_t(0), uint64_t(nfilenames),
             [&](uint64_t i) {
                 one_sequence_iteration(ot, i, frame_numbers[0][i],
                                        sequence_args, filenames,
                                        { argv, argv + argc });
             },
-            paropt().minitems(1));
+            paropt().minitems(1).maxthreads(parallel_frame_threads));
+        ot.end_parallel_frame_loop();
     } else {
         // Fully serialized over the frame range, multithreaded for each frame
         // individually.
@@ -7465,6 +7479,24 @@ handle_sequence(Oiiotool& ot, int argc, const char** argv)
 
 
 
+void
+Oiiotool::begin_parallel_frame_loop(int nthreads)
+{
+    OIIO::attribute("threads", nthreads);
+    OIIO::attribute("exr_threads", 1);
+    m_in_parallel_frame_loop = true;
+}
+
+
+
+void
+Oiiotool::end_parallel_frame_loop()
+{
+    m_in_parallel_frame_loop = false;
+}
+
+
+
 int
 main(int argc, char* argv[])
 {

src/oiiotool/oiiotool.h

@@ -158,7 +158,8 @@ class Oiiotool {
     int frame_number            = 0;
     bool enable_function_timing = true;
     bool input_config_set       = false;
-    bool printed_info           = false;  // printed info at some point
+    bool printed_info           = false;    // printed info at some point
+    bool m_in_parallel_frame_loop = false;  // True when in parallel frame loop
     // Remember the first input dataformats we encountered
     TypeDesc input_dataformat;
     int input_bitspersample = 0;
@@ -397,6 +398,10 @@ class Oiiotool {
             m_first_input_dimensions.copy_dimensions(dims);
     }
 
+    void begin_parallel_frame_loop(int nthreads);
+    void end_parallel_frame_loop();
+    bool in_parallel_frame_loop() const { return m_in_parallel_frame_loop; }
+
 private:
     CallbackFunction m_pending_callback;
     std::vector<const char*> m_pending_argv;