Always check in the code for explicit parallel/serial choice (#980)

* Check at compile time that MANIFOLD_PAR is set to either P (parallel) or S (series) mode. * Switch from 'P' and 'S' to 1 and -1 * fix formatting * Put the MANIFOLD_PAR poison pill in an internal header. We want this check for Manifold compilation as a cross check on our CMake logic, but this way client codes will work by default without being forced to set it explicitly.
elalish · Oct 7, 2024 · 3febd48 · 3febd48
1 parent 658d3e0
commit 3febd48
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 29 deletions.
diff --git a/extras/minimize_testcase.cpp b/extras/minimize_testcase.cpp
@@ -5,7 +5,7 @@
 #include <sstream>
 #include <string>
 
-#if defined(MANIFOLD_PAR) && __has_include(<pstl/glue_execution_defs.h>)
+#if (MANIFOLD_PAR == 1) && __has_include(<pstl/glue_execution_defs.h>)
 #include <execution>
 #endif
 
@@ -73,7 +73,7 @@ bool safeToRemove(const Polygons &polys, size_t i, size_t j, double precision) {
     };
     const vec2 *polysk = polys[k].data();
     if (!std::all_of(
-#if defined(MANIFOLD_PAR) && __has_include(<pstl/glue_execution_defs.h>)
+#if (MANIFOLD_PAR == 1) && __has_include(<pstl/glue_execution_defs.h>)
             std::execution::par,
 #endif
             countAt(0_uz), countAt(polys[k].size()), [=](size_t l) {
@@ -150,7 +150,7 @@ std::vector<int> getChildren(const Polygons &polys, size_t i) {
       return k == (polys[i].size() - 1) ? 0 : (k + 1);
     };
     int count = std::count_if(
-#if defined(MANIFOLD_PAR) && __has_include(<pstl/glue_execution_defs.h>)
+#if (MANIFOLD_PAR == 1) && __has_include(<pstl/glue_execution_defs.h>)
         std::execution::par,
 #endif
         countAt((size_t)0), countAt(polys[i].size()),

diff --git a/include/manifold/parallel.h b/include/manifold/parallel.h
@@ -16,7 +16,8 @@
 // Iterators must be RandomAccessIterator.
 
 #pragma once
-#if defined(MANIFOLD_PAR)
+
+#if (MANIFOLD_PAR == 1)
 #include <tbb/combinable.h>
 #include <tbb/parallel_for.h>
 #include <tbb/parallel_invoke.h>
@@ -62,7 +63,7 @@ void copy(ExecutionPolicy policy, InputIter first, InputIter last,
 template <typename InputIter, typename OutputIter>
 void copy(InputIter first, InputIter last, OutputIter d_first);
 
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
 namespace details {
 using manifold::kSeqThreshold;
 // implementation from
@@ -293,7 +294,7 @@ template <typename Iterator,
           typename Comp = decltype(std::less<T>())>
 void mergeSort(ExecutionPolicy policy, Iterator first, Iterator last,
                Comp comp) {
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
   if (policy == ExecutionPolicy::Par) {
     // apparently this prioritizes threads inside here?
     tbb::this_task_arena::isolate([&] {
@@ -347,7 +348,7 @@ struct SortFunctor<
     static_assert(std::is_trivially_destructible_v<T>,
                   "Our simple implementation does not support types that are "
                   "not trivially destructable.");
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
     if (policy == ExecutionPolicy::Par) {
       radix_sort(&*first, static_cast<size_t>(std::distance(first, last)));
       return;
@@ -368,7 +369,7 @@ void for_each(ExecutionPolicy policy, Iter first, Iter last, F f) {
                     typename std::iterator_traits<Iter>::iterator_category,
                     std::random_access_iterator_tag>,
                 "You can only parallelize RandomAccessIterator.");
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
   if (policy == ExecutionPolicy::Par) {
     tbb::parallel_for(tbb::blocked_range<Iter>(first, last),
                       [&f](const tbb::blocked_range<Iter> &range) {
@@ -404,7 +405,7 @@ T reduce(ExecutionPolicy policy, InputIter first, InputIter last, T init,
                     typename std::iterator_traits<InputIter>::iterator_category,
                     std::random_access_iterator_tag>,
                 "You can only parallelize RandomAccessIterator.");
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
   if (policy == ExecutionPolicy::Par) {
     // should we use deterministic reduce here?
     return tbb::parallel_reduce(
@@ -480,7 +481,7 @@ void inclusive_scan(ExecutionPolicy policy, InputIter first, InputIter last,
           typename std::iterator_traits<OutputIter>::iterator_category,
           std::random_access_iterator_tag>,
       "You can only parallelize RandomAccessIterator.");
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
   if (policy == ExecutionPolicy::Par) {
     tbb::parallel_scan(
         tbb::blocked_range<size_t>(0, std::distance(first, last)),
@@ -542,7 +543,7 @@ void exclusive_scan(ExecutionPolicy policy, InputIter first, InputIter last,
           typename std::iterator_traits<OutputIter>::iterator_category,
           std::random_access_iterator_tag>,
       "You can only parallelize RandomAccessIterator.");
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
   if (policy == ExecutionPolicy::Par) {
     details::ScanBody<T, InputIter, OutputIter, BinOp> body(init, identity, f,
                                                             first, d_first);
@@ -595,7 +596,7 @@ void transform(ExecutionPolicy policy, InputIter first, InputIter last,
           typename std::iterator_traits<OutputIter>::iterator_category,
           std::random_access_iterator_tag>,
       "You can only parallelize RandomAccessIterator.");
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
   if (policy == ExecutionPolicy::Par) {
     tbb::parallel_for(tbb::blocked_range<size_t>(
                           0, static_cast<size_t>(std::distance(first, last))),
@@ -639,7 +640,7 @@ void copy(ExecutionPolicy policy, InputIter first, InputIter last,
           typename std::iterator_traits<OutputIter>::iterator_category,
           std::random_access_iterator_tag>,
       "You can only parallelize RandomAccessIterator.");
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
   if (policy == ExecutionPolicy::Par) {
     tbb::parallel_for(tbb::blocked_range<size_t>(
                           0, static_cast<size_t>(std::distance(first, last)),
@@ -696,7 +697,7 @@ void fill(ExecutionPolicy policy, OutputIter first, OutputIter last, T value) {
           typename std::iterator_traits<OutputIter>::iterator_category,
           std::random_access_iterator_tag>,
       "You can only parallelize RandomAccessIterator.");
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
   if (policy == ExecutionPolicy::Par) {
     tbb::parallel_for(tbb::blocked_range<OutputIter>(first, last),
                       [&](const tbb::blocked_range<OutputIter> &range) {
@@ -719,7 +720,7 @@ void fill(OutputIter first, OutputIter last, T value) {
 template <typename InputIter, typename P>
 size_t count_if(ExecutionPolicy policy, InputIter first, InputIter last,
                 P pred) {
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
   if (policy == ExecutionPolicy::Par) {
     return reduce(policy, TransformIterator(first, pred),
                   TransformIterator(last, pred), 0, std::plus<size_t>());
@@ -743,7 +744,7 @@ bool all_of(ExecutionPolicy policy, InputIter first, InputIter last, P pred) {
                     typename std::iterator_traits<InputIter>::iterator_category,
                     std::random_access_iterator_tag>,
                 "You can only parallelize RandomAccessIterator.");
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
   if (policy == ExecutionPolicy::Par) {
     // should we use deterministic reduce here?
     return tbb::parallel_reduce(
@@ -790,7 +791,7 @@ OutputIter copy_if(ExecutionPolicy policy, InputIter first, InputIter last,
           typename std::iterator_traits<OutputIter>::iterator_category,
           std::random_access_iterator_tag>,
       "You can only parallelize RandomAccessIterator.");
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
   if (policy == ExecutionPolicy::Par) {
     auto pred2 = [&](size_t i) { return pred(first[i]); };
     details::CopyIfScanBody body(pred2, first, d_first);
@@ -837,7 +838,7 @@ Iter remove_if(ExecutionPolicy policy, Iter first, Iter last, P pred) {
   static_assert(std::is_trivially_destructible_v<T>,
                 "Our simple implementation does not support types that are "
                 "not trivially destructable.");
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
   if (policy == ExecutionPolicy::Par) {
     T *tmp = new T[std::distance(first, last)];
     auto back =
@@ -884,7 +885,7 @@ Iter remove(ExecutionPolicy policy, Iter first, Iter last, T value) {
   static_assert(std::is_trivially_destructible_v<T>,
                 "Our simple implementation does not support types that are "
                 "not trivially destructable.");
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
   if (policy == ExecutionPolicy::Par) {
     T *tmp = new T[std::distance(first, last)];
     auto back =
@@ -931,7 +932,7 @@ Iter unique(ExecutionPolicy policy, Iter first, Iter last) {
   static_assert(std::is_trivially_destructible_v<T>,
                 "Our simple implementation does not support types that are "
                 "not trivially destructable.");
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
   if (policy == ExecutionPolicy::Par && first != last) {
     Iter newSrcStart = first;
     // cap the maximum buffer size, proved to be beneficial for unique with huge
@@ -984,7 +985,7 @@ Iter unique(Iter first, Iter last) {
 template <typename Iterator,
           typename T = typename std::iterator_traits<Iterator>::value_type>
 void stable_sort(ExecutionPolicy policy, Iterator first, Iterator last) {
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
   details::SortFunctor<Iterator, T>()(policy, first, last);
 #else
   std::stable_sort(first, last);
@@ -1015,7 +1016,7 @@ template <typename Iterator,
           typename Comp = decltype(std::less<T>())>
 void stable_sort(ExecutionPolicy policy, Iterator first, Iterator last,
                  Comp comp) {
-#if defined(MANIFOLD_PAR)
+#if (MANIFOLD_PAR == 1)
   details::mergeSort(policy, first, last, comp);
 #else
   std::stable_sort(first, last, comp);

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -112,13 +112,18 @@ set(
   MANIFOLD_DEBUG
   MANIFOLD_CROSS_SECTION
   MANIFOLD_EXPORT
-  MANIFOLD_PAR
 )
 foreach(OPT IN LISTS OPTIONS)
   if(${${OPT}})
     target_compile_options(manifold PUBLIC -D${OPT})
   endif()
 endforeach()
+if(MANIFOLD_PAR)
+  target_compile_options(manifold PUBLIC -DMANIFOLD_PAR=1)
+else()
+  target_compile_options(manifold PUBLIC -DMANIFOLD_PAR=-1)
+endif()
+
 target_compile_features(manifold PUBLIC cxx_std_17)
 
 install(TARGETS manifold EXPORT manifoldTargets)

diff --git a/src/boolean_result.cpp b/src/boolean_result.cpp
@@ -20,7 +20,7 @@
 #include "./utils.h"
 #include "manifold/parallel.h"
 
-#if defined(MANIFOLD_PAR) && __has_include(<tbb/concurrent_map.h>)
+#if (MANIFOLD_PAR == 1) && __has_include(<tbb/concurrent_map.h>)
 #define TBB_PREVIEW_CONCURRENT_ORDERED_CONTAINERS 1
 #include <tbb/concurrent_map.h>
 #include <tbb/parallel_for.h>
@@ -243,7 +243,7 @@ void AddNewEdgeVerts(
       direction = !direction;
     }
   };
-#if defined(MANIFOLD_PAR) && __has_include(<tbb/tbb.h>)
+#if (MANIFOLD_PAR == 1) && __has_include(<tbb/tbb.h>)
   // parallelize operations, requires concurrent_map so we can only enable this
   // with tbb
   if (!ManifoldParams().deterministic && p1q2.size() > kParallelThreshold) {

diff --git a/src/csg_tree.cpp b/src/csg_tree.cpp
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#if defined(MANIFOLD_PAR) && __has_include(<tbb/concurrent_priority_queue.h>)
+#if (MANIFOLD_PAR == 1) && __has_include(<tbb/concurrent_priority_queue.h>)
 #include <tbb/tbb.h>
 #define TBB_PREVIEW_CONCURRENT_ORDERED_CONTAINERS 1
 #include <tbb/concurrent_priority_queue.h>
@@ -465,7 +465,7 @@ std::shared_ptr<Manifold::Impl> CsgOpNode::BatchBoolean(
     Boolean3 boolean(*results[0], *results[1], operation);
     return std::make_shared<Manifold::Impl>(boolean.Result(operation));
   }
-#if defined(MANIFOLD_PAR) && __has_include(<tbb/tbb.h>)
+#if (MANIFOLD_PAR == 1) && __has_include(<tbb/tbb.h>)
   if (!ManifoldParams().deterministic) {
     tbb::task_group group;
     tbb::concurrent_priority_queue<SharedImpl, MeshCompare> queue(

diff --git a/src/face_op.cpp b/src/face_op.cpp
@@ -12,14 +12,15 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#if defined(MANIFOLD_PAR) && __has_include(<tbb/concurrent_map.h>)
+#if (MANIFOLD_PAR == 1) && __has_include(<tbb/concurrent_map.h>)
 #include <tbb/tbb.h>
 #define TBB_PREVIEW_CONCURRENT_ORDERED_CONTAINERS 1
 #include <tbb/concurrent_map.h>
 #endif
 #include <unordered_set>
 
 #include "./impl.h"
+#include "manifold/parallel.h"
 #include "manifold/polygon.h"
 
 namespace manifold {
@@ -131,7 +132,7 @@ void Manifold::Impl::Face2Tri(const Vec<int>& faceEdge,
                       halfedge_.cbegin() + faceEdge[face + 1], projection);
     return TriangulateIdx(polys, precision_);
   };
-#if defined(MANIFOLD_PAR) && __has_include(<tbb/tbb.h>)
+#if (MANIFOLD_PAR == 1) && __has_include(<tbb/tbb.h>)
   tbb::task_group group;
   // map from face to triangle
   tbb::concurrent_unordered_map<int, std::vector<ivec3>> results;

diff --git a/src/utils.h b/src/utils.h
@@ -27,6 +27,18 @@
 
 #include "./vec.h"
 #include "manifold/common.h"
+
+#ifndef MANIFOLD_PAR
+#error "MANIFOLD_PAR must be defined to either 1 (parallel) or -1 (series)"
+#else
+#if (MANIFOLD_PAR != 1) && (MANIFOLD_PAR != -1)
+#define XSTR(x) STR(x)
+#define STR(x) #x
+#pragma message "Current value of MANIFOLD_PAR is: " XSTR(MANIFOLD_PAR)
+#error "MANIFOLD_PAR must be defined to either 1 (parallel) or -1 (series)"
+#endif
+#endif
+
 #include "manifold/parallel.h"
 
 #if __has_include(<tracy/Tracy.hpp>)