Fix getOptimalThreadCount segfault (#714)

This uses `/proc` instead of `cpuinfo` as it seems more stable across old kernels.

Author: DiamonDinoia

include/finufft/finufft_utils.hpp

@@ -61,9 +61,9 @@ class FINUFFT_EXPORT CNTime {
   double initial;
 };
 
-FINUFFT_NEVER_INLINE int getOptimalThreadCount();
-
-FINUFFT_NEVER_INLINE int get_num_threads_parallel_block();
+#ifdef _OPENMP
+FINUFFT_NEVER_INLINE unsigned getOptimalThreadCount();
+#endif
 
 } // namespace finufft::utils
 

src/finufft_core.cpp

@@ -548,12 +548,6 @@ void finufft_default_opts_t(finufft_opts *o)
   // sphinx tag (don't remove): @defopts_end
 }
 
-// Wrapper to cache the optimal thread count using a static variable.
-int getCachedOptimalThreadCount() {
-  static const int cached_value = getOptimalThreadCount();
-  return cached_value;
-}
-
 // PPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPP
 template<typename TF>
 FINUFFT_PLAN_T<TF>::FINUFFT_PLAN_T(int type_, int dim_, const BIGINT *n_modes, int iflag,
@@ -600,9 +594,13 @@ FINUFFT_PLAN_T<TF>::FINUFFT_PLAN_T(int type_, int dim_, const BIGINT *n_modes, i
   // get stuff from args...
   fftSign = (iflag >= 0) ? 1 : -1; // clean up flag input
 
+  CNTime timer{};
+  if (opts.debug > 1) {
+    timer.start();
+  }
 #ifdef _OPENMP
   // choose overall # threads...
-  int ompmaxnthr = getCachedOptimalThreadCount();
+  int ompmaxnthr = static_cast<int>(getOptimalThreadCount());
   int nthr       = ompmaxnthr; // default: use as many physical cores as possible
   // (the above could be set, or suggested set, to 1 for small enough problems...)
   if (opts.nthreads > 0) {
@@ -632,6 +630,11 @@ FINUFFT_PLAN_T<TF>::FINUFFT_PLAN_T(int type_, int dim_, const BIGINT *n_modes, i
             __func__);
     throw int(FINUFFT_ERR_NTHREADS_NOTVALID);
   }
+  if (opts.debug > 1) {
+    const auto sec = timer.elapsedsec();
+    fprintf(stdout, "[%s] detected %d threads in %.3g sec.\n", __func__, nthr, sec);
+  }
+
   // (this sets/limits all downstream spread/interp, 1dkernel, and FFT thread counts...)
 
   // choose batchSize for types 1,2 or 3... (uses int ceil(b/a)=1+(b-1)/a trick)
@@ -728,8 +731,7 @@ FINUFFT_PLAN_T<TF>::FINUFFT_PLAN_T(int type_, int dim_, const BIGINT *n_modes, i
       }
 
       // STEP 0: get Fourier coeffs of spreading kernel along each fine grid dim
-      CNTime timer;
-      timer.start();
+      timer.restart();
       for (int idim = 0; idim < dim; ++idim)
         onedim_fseries_kernel(nfdim[idim], phiHat[idim], spopts);
       if (opts.debug)

src/finufft_utils.cpp

@@ -135,10 +135,12 @@ double CNTime::elapsedsec() const
   return nowsec - initial;
 }
 
+#ifdef _OPENMP
 namespace {
 #if defined(_WIN32)
 // Returns the number of physical CPU cores on Windows (excluding hyper-threaded cores)
-static int getPhysicalCoreCount() {
+unsigned getPhysicalCoreCount() {
+#if defined(__i386__) || defined(__x86_64__) || defined(_M_IX86) || defined(_M_X64)
   int physicalCoreCount = 0;
 
   // Determine the required buffer size.
@@ -161,9 +163,13 @@ static int getPhysicalCoreCount() {
     return MY_OMP_GET_MAX_THREADS();
   }
   return physicalCoreCount;
+#else
+  // On non-x86 architectures, there should be no hyper-threading
+  return MY_OMP_GET_MAX_THREADS();
+#endif
 }
 
-static int getAllowedCoreCount() {
+unsigned getAllowedCoreCount() {
   DWORD_PTR processMask, systemMask;
   if (!GetProcessAffinityMask(GetCurrentProcess(), &processMask, &systemMask)) {
     return 0; // API call failed (should rarely happen for the current process)
@@ -180,7 +186,7 @@ static int getAllowedCoreCount() {
 #elif defined(__APPLE__)
 
 // Returns the number of physical CPU cores on macOS (excluding hyper-threaded cores)
-static int getPhysicalCoreCount() {
+unsigned getPhysicalCoreCount() {
   int physicalCoreCount = 0;
   int cores             = 0;
   size_t size           = sizeof(cores);
@@ -194,81 +200,76 @@ static int getPhysicalCoreCount() {
   return physicalCoreCount;
 }
 
-static int getAllowedCoreCount() {
+unsigned getAllowedCoreCount() {
   // MacOS does not support CPU affinity, so we return the maximum number of threads.
   return MY_OMP_GET_MAX_THREADS();
 }
 
 #elif defined(__linux__)
 // Returns the number of physical CPU cores on Linux (excluding hyper-threaded cores)
-static int getPhysicalCoreCount() {
-  int physicalCoreCount = 0;
-  std::ifstream cpuinfo("/proc/cpuinfo");
-  if (!cpuinfo.is_open()) return MY_OMP_GET_MAX_THREADS();
-
-  std::set<std::string> coreSet;
-  std::string line;
-  int physicalId = -1, coreId = -1;
-  bool foundPhysical = false, foundCore = false;
-
-  while (std::getline(cpuinfo, line)) {
-    // An empty line indicates the end of a processor block.
-    if (line.empty()) {
-      if (foundPhysical && foundCore)
-        coreSet.insert(std::to_string(physicalId) + "-" + std::to_string(coreId));
-      // Reset for the next processor block.
-      foundPhysical = foundCore = false;
-      physicalId = coreId = -1;
-    } else {
-      auto colonPos = line.find(':');
-      if (colonPos == std::string::npos) continue;
-      std::string key   = line.substr(0, colonPos);
-      std::string value = line.substr(colonPos + 1);
-      // Trim whitespace.
-      key.erase(key.find_last_not_of(" \t") + 1);
-      value.erase(0, value.find_first_not_of(" \t"));
-
-      if (key == "physical id") {
-        physicalId    = std::stoi(value);
-        foundPhysical = true;
-      } else if (key == "core id") {
-        coreId    = std::stoi(value);
-        foundCore = true;
+// Compatibility:
+// - Linux kernels 2.6 and later (provides /sys/devices/system/cpu topology interface)
+// - Any CPU architecture supported by the kernel (Intel, AMD, ARM, POWER, etc.)
+// - Works in containers or cgroups (reflects host topology)
+unsigned getPhysicalCoreCount() {
+  // only x86_64 and x86_32 architectures support HT (hyper-threading)
+  // in all other cases, we assume no HT and return MY_OMP_GET_MAX_THREADS()
+#if defined(__i386__) || defined(__x86_64__)
+  // Parse strings like "0-3,5,7-9" → {0,1,2,3,5,7,8,9}
+  auto parseCpuList = [](const std::string &s) {
+    std::vector<int> cpus;
+    std::istringstream iss(s);
+    for (std::string tok; std::getline(iss, tok, ',');) {
+      auto dash = tok.find('-');
+      if (dash == std::string::npos) {
+        cpus.push_back(std::stoi(tok));
+      } else {
+        int start = std::stoi(tok.substr(0, dash));
+        int end   = std::stoi(tok.substr(dash + 1));
+        for (int i = start; i <= end; ++i) cpus.push_back(i);
       }
     }
+    return cpus;
+  };
+
+  // Read a single integer from the given sysfs file
+  auto readInt = [&](const std::string &path, int &out) -> bool {
+    std::ifstream f(path);
+    if (!f.is_open()) return false;
+    f >> out;
+    return !f.fail();
+  };
+
+  // 1) Read list of present CPUs
+  std::ifstream presentF("/sys/devices/system/cpu/present");
+  if (!presentF.is_open()) {
+    return MY_OMP_GET_MAX_THREADS();
   }
-  // In case the file doesn't end with an empty line.
-  if (foundPhysical && foundCore)
-    coreSet.insert(std::to_string(physicalId) + "-" + std::to_string(coreId));
-
-  if (!coreSet.empty()) {
-    physicalCoreCount = static_cast<int>(coreSet.size());
-  } else {
-    // Fallback: try reading "cpu cores" from the first processor block.
-    cpuinfo.clear();
-    cpuinfo.seekg(0, std::ios::beg);
-    while (std::getline(cpuinfo, line)) {
-      auto colonPos = line.find(':');
-      if (colonPos != std::string::npos) {
-        std::string key   = line.substr(0, colonPos);
-        std::string value = line.substr(colonPos + 1);
-        key.erase(key.find_last_not_of(" \t") + 1);
-        value.erase(0, value.find_first_not_of(" \t"));
-        if (key == "cpu cores") {
-          physicalCoreCount = std::stoi(value);
-          break;
-        }
-      }
-    }
+  std::string presentLine;
+  std::getline(presentF, presentLine);
+  auto cpus = parseCpuList(presentLine);
+
+  // 2) For each CPU, read its package & core IDs
+  std::set<std::pair<int, int>> physicalCores;
+  for (int cpu : cpus) {
+    std::string topoBase =
+        "/sys/devices/system/cpu/cpu" + std::to_string(cpu) + "/topology/";
+    int pkg = -1, core = -1;
+    if (!readInt(topoBase + "physical_package_id", pkg)) continue;
+    if (!readInt(topoBase + "core_id", core)) continue;
+    physicalCores.emplace(pkg, core);
   }
 
-  if (physicalCoreCount == 0) {
-    return MY_OMP_GET_MAX_THREADS();
+  // 3) Return count if successful, else fallback
+  if (!physicalCores.empty()) {
+    return static_cast<unsigned>(physicalCores.size());
   }
-  return physicalCoreCount;
+#endif
+  // in ARM and RISKV we only need this
+  return MY_OMP_GET_MAX_THREADS();
 }
 
-static int getAllowedCoreCount() {
+unsigned getAllowedCoreCount() {
   cpu_set_t cpuSet;
   CPU_ZERO(&cpuSet);
   if (sched_getaffinity(0, sizeof(cpu_set_t), &cpuSet) != 0) {
@@ -287,44 +288,45 @@ static int getAllowedCoreCount() {
 
 #warning "Unknown platform. Impossible to detect the number of physical cores."
 // Fallback version if none of the above platforms is detected.
-static int getPhysicalCoreCount() { return MY_OMP_GET_MAX_THREADS(); }
-static int getAllowedCoreCount() { return MY_OMP_GET_MAX_THREADS(); }
+unsigned getPhysicalCoreCount() { return MY_OMP_GET_MAX_THREADS(); }
+unsigned getAllowedCoreCount() { return MY_OMP_GET_MAX_THREADS(); }
 
 #endif
 
 } // namespace
 
-int getOptimalThreadCount() {
+unsigned getOptimalThreadCount() {
   // if the user has set the OMP_NUM_THREADS environment variable, use that value
-  const auto OMP_THREADS = std::getenv("OMP_NUM_THREADS");
-  if (OMP_THREADS) {
-    return std::stoi(OMP_THREADS);
-  }
-  // otherwise, use the min between number of physical cores or the number of allowed
-  // cores (e.g. by taskset)
-  const auto physicalCores = getPhysicalCoreCount();
-  const auto allowedCores  = getAllowedCoreCount();
-  if (physicalCores < allowedCores) {
-    return physicalCores;
-  }
-  return allowedCores;
-}
+  static const auto cached_threads = []() -> unsigned {
+    const auto OMP_THREADS = std::getenv("OMP_NUM_THREADS");
+    if (OMP_THREADS) {
+      try {
+        return std::stoi(OMP_THREADS);
+      } catch (...) {
+        std::cerr << "Invalid OMP_NUM_THREADS value: " << OMP_THREADS
+                  << ". using default thread count." << std::endl;
+      }
+    }
+    // otherwise, use the min between number of physical cores or the number of allowed
+    // cores (e.g. by taskset)
+    try {
+      const auto physicalCores = getPhysicalCoreCount();
+      const auto allowedCores  = getAllowedCoreCount();
+      if (physicalCores < allowedCores) {
+        return physicalCores;
+      }
+      return allowedCores;
+    } catch (const std::exception &e) {
+      std::cerr << "Error determining optimal thread count: " << e.what()
+                << ". Using OpenMP default thread count." << std::endl;
+    }
+    return MY_OMP_GET_MAX_THREADS();
+  }();
 
-// -------------------------- openmp helpers -------------------------------
-int get_num_threads_parallel_block()
-// return how many threads an omp parallel block would use.
-// omp_get_max_threads() does not report this; consider case of NESTED=0.
-// Why is there no such routine?   Barnett 5/22/20
-{
-  int nth_used;
-#pragma omp parallel
-  {
-#pragma omp single
-    nth_used = MY_OMP_GET_NUM_THREADS();
-  }
-  return nth_used;
+  return cached_threads;
 }
 
+#endif // _OPENMP
 // ---------- thread-safe rand number generator for Windows platform ---------
 // (note this is used by macros in test_defs.h, and supplied in linux/macosx)
 #ifdef _WIN32