Deprecate ParamMap (#7121)

src/Func.h

@@ -785,7 +785,9 @@ class Func {
      *
      * One can explicitly construct a ParamMap and
      * use its set method to insert Parameter to scalar or Buffer
-     * value mappings:
+     * value mappings. (NOTE: ParamMap is deprecated in Halide 16 and
+     * will be removed in Halide 17. Callers requiring threadsafe JIT
+     * calls should migrate to use compile_to_callable() instead.)
      *
      \code
      Param<int32> p(42);

src/ParamMap.h

@@ -70,9 +70,13 @@ class ParamMap {
 public:
     ParamMap() = default;
 
+    HALIDE_ATTRIBUTE_DEPRECATED("ParamMap is deprecated in Halide 16 and will be removed in Halide 17. "
+                                "Callers requiring threadsafe JIT calls should migrate to use compile_to_callable() instead.")
     ParamMap(const std::initializer_list<ParamMapping> &init);
 
     template<typename T>
+    HALIDE_ATTRIBUTE_DEPRECATED("ParamMap is deprecated in Halide 16 and will be removed in Halide 17. "
+                                "Callers requiring threadsafe JIT calls should migrate to use compile_to_callable() instead.")
     void set(const Param<T> &p, T val) {
         Internal::Parameter v(p.type(), false, 0, p.name());
         v.set_scalar<T>(val);
@@ -82,6 +86,8 @@ class ParamMap {
         mapping[p.parameter()] = pa;
     }
 
+    HALIDE_ATTRIBUTE_DEPRECATED("ParamMap is deprecated in Halide 16 and will be removed in Halide 17. "
+                                "Callers requiring threadsafe JIT calls should migrate to use compile_to_callable() instead.")
     void set(const ImageParam &p, const Buffer<> &buf) {
         set(p, buf, nullptr);
     }

test/correctness/param_map.cpp

@@ -2,6 +2,11 @@
 #include <iostream>
 #include <stdio.h>
 
+#ifdef __GNUC__
+// We don't want to emit deprecation warnings for this test
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#endif
+
 using namespace Halide;
 
 int main(int argc, char **argv) {

test/performance/CMakeLists.txt

@@ -39,7 +39,8 @@ tests(GROUPS performance multithreaded
       rfactor.cpp
       sort.cpp
       stack_vs_heap.cpp
-      thread_safe_jit.cpp
+      thread_safe_jit_callable.cpp
+      thread_safe_jit_param_map.cpp
       )
 
 # Make sure that performance tests do not run in parallel with other tests,

test/performance/thread_safe_jit_callable.cpp

@@ -0,0 +1,129 @@
+#include "Halide.h"
+#include "halide_benchmark.h"
+#include <cstdio>
+#include <functional>
+#include <thread>
+
+/** \file Test to demonstrate using JIT across multiple threads with
+ * varying parameters passed to realizations. Performance is tested
+ * by comparing a technique that recompiles vs one that should not.
+ */
+
+using namespace Halide;
+using namespace Halide::Tools;
+
+struct test_func {
+    Param<int32_t> p;
+    ImageParam in{Int(32), 1};
+    Func func;
+    Var x;
+    std::function<int(Buffer<int32_t, 1>, int32_t, Buffer<int32_t, 1>)> f;
+
+    test_func() {
+        Expr big = 0;
+        for (int i = 0; i < 75; i++) {
+            big += p;
+        }
+        Func inner;
+        inner(x) = x * in(clamp(x, 0, 9)) + big;
+        func(x) = inner(x - 1) + inner(x) + inner(x + 1);
+        inner.compute_at(func, x);
+
+        // The Halide compiler is threadsafe, with the important caveat
+        // that mutable objects like Funcs and ImageParams cannot be
+        // shared across thread boundaries without being guarded by a
+        // mutex. Since we don't share any such objects here, we don't
+        // need any synchronization
+        f = func.compile_to_callable({in, p}).make_std_function<Buffer<int32_t, 1>, int32_t, Buffer<int32_t, 1>>();
+    }
+
+    test_func(const test_func &copy) = delete;
+    test_func &operator=(const test_func &) = delete;
+    test_func(test_func &&) = delete;
+    test_func &operator=(test_func &&) = delete;
+};
+
+Buffer<int32_t> bufs[16];
+
+void separate_func_per_thread_executor(int index) {
+    test_func test;
+
+    Buffer<int32_t> output(10);
+    for (int i = 0; i < 10; i++) {
+        int result = test.f(bufs[index], index, output);
+        assert(result == 0);
+        for (int j = 0; j < 10; j++) {
+            int64_t left = ((j - 1) * (int64_t)bufs[index](std::min(std::max(0, j - 1), 9)) + index * 75);
+            int64_t middle = (j * (int64_t)bufs[index](std::min(std::max(0, j), 9)) + index * 75);
+            int64_t right = ((j + 1) * (int64_t)bufs[index](std::min(std::max(0, j + 1), 9)) + index * 75);
+            assert(output(j) == (int32_t)(left + middle + right));
+        }
+    }
+}
+
+void separate_func_per_thread() {
+    std::thread threads[16];
+
+    for (auto &thread : threads) {
+        thread = std::thread(separate_func_per_thread_executor,
+                             (int)(&thread - threads));
+    }
+
+    for (auto &thread : threads) {
+        thread.join();
+    }
+}
+
+void same_func_per_thread_executor(int index, test_func &test) {
+    Buffer<int32_t> output(10);
+    for (int i = 0; i < 10; i++) {
+        int result = test.f(bufs[index], index, output);
+        assert(result == 0);
+        for (int j = 0; j < 10; j++) {
+            int64_t left = ((j - 1) * (int64_t)bufs[index](std::min(std::max(0, j - 1), 9)) + index * 75);
+            int64_t middle = (j * (int64_t)bufs[index](std::min(std::max(0, j), 9)) + index * 75);
+            int64_t right = ((j + 1) * (int64_t)bufs[index](std::min(std::max(0, j + 1), 9)) + index * 75);
+            assert(output(j) == (int32_t)(left + middle + right));
+        }
+    }
+}
+
+void same_func_per_thread() {
+    std::thread threads[16];
+    test_func test;
+
+    for (auto &thread : threads) {
+        thread = std::thread(same_func_per_thread_executor,
+                             (int)(&thread - threads), std::ref(test));
+    }
+
+    for (auto &thread : threads) {
+        thread.join();
+    }
+}
+
+int main(int argc, char **argv) {
+    Target target = get_jit_target_from_environment();
+    if (target.arch == Target::WebAssembly) {
+        printf("[SKIP] Performance tests are meaningless and/or misleading under WebAssembly interpreter.\n");
+        return 0;
+    }
+
+    for (auto &buf : bufs) {
+        buf = Buffer<int32_t>(10);
+        for (int i = 0; i < 10; i++) {
+            buf(i) = std::rand();
+        }
+    }
+
+    double separate_time = benchmark(separate_func_per_thread);
+    printf("Separate compilations time: %fs.\n", separate_time);
+
+    double same_time = benchmark(same_func_per_thread);
+    printf("One compilation time: %fs.\n", same_time);
+
+    assert(same_time < separate_time);
+
+    printf("Success!\n");
+    return 0;
+}

test/performance/thread_safe_jit.cpp → test/performance/thread_safe_jit_param_map.cpp

@@ -4,6 +4,11 @@
 #include <functional>
 #include <thread>
 
+#ifdef __GNUC__
+// We don't want to emit deprecation warnings for this test
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#endif
+
 /** \file Test to demonstrate using JIT across multiple threads with
  * varying parameters passed to realizations. Performance is tested
  * by comparing a technique that recompiles vs one that should not.
@@ -27,23 +32,26 @@ struct test_func {
         inner(x) = x * in(clamp(x, 0, 9)) + big;
         f(x) = inner(x - 1) + inner(x) + inner(x + 1);
         inner.compute_at(f, x);
+
+        // The Halide compiler is threadsafe, with the important caveat
+        // that mutable objects like Funcs and ImageParams cannot be
+        // shared across thread boundaries without being guarded by a
+        // mutex. Since we don't share any such objects here, we don't
+        // need any synchronization
+        f.compile_jit();
     }
-};
 
-// The Halide compiler is currently not guaranteed to be thread safe.
-std::mutex compiler_mutex;
+    test_func(const test_func &copy) = delete;
+    test_func &operator=(const test_func &) = delete;
+    test_func(test_func &&) = delete;
+    test_func &operator=(test_func &&) = delete;
+};
 
 Buffer<int32_t> bufs[16];
 
 void separate_func_per_thread_executor(int index) {
     test_func test;
 
-    {
-        std::lock_guard<std::mutex> lock(compiler_mutex);
-
-        test.f.compile_jit();
-    }
-
     test.p.set(index);
     test.in.set(bufs[index]);
     for (int i = 0; i < 10; i++) {
@@ -88,17 +96,6 @@ void same_func_per_thread() {
     std::thread threads[16];
     test_func test;
 
-    // In this program, only one thread can call into the compiler
-    // at this point. The mutex guard is still included both to show that in
-    // general Halide compilation is not thread ssafe and also to keep
-    // the performance comparison slightly more equal by including
-    // (minimal) mutex cost on both paths.
-    {
-        std::lock_guard<std::mutex> lock(compiler_mutex);
-
-        test.f.compile_jit();
-    }
-
     for (auto &thread : threads) {
         thread = std::thread(same_func_per_thread_executor,
                              (int)(&thread - threads), std::ref(test));