[PATCH] D25564: Add interface to compute number of physical cores on host system

[Mehdi AMINI via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: lib/Support/Host.cpp:1197
+// implementation reads the /proc/cpuinfo format on an x86_64 system.
+int sys::getHostNumPhysicalCores() {
+  // Read /proc/cpuinfo as a stream (until EOF reached). It cannot be
----------------
I think we should read the file once:

```
static int computeHostNumPhysicalCores() {
  // all your stuff
}
int sys::getHostNumPhysicalCores() {
  static int NumCores = computeHostNumPhysicalCores();
  return NumCores;
}
```


================
Comment at: lib/Support/Host.cpp:1212
+  SmallSet<std::pair<int, int>, 32> UniqueItems;
+  for (auto &Line : strs) {
+    Line = Line.trim();
----------------
Not that it matters that much, but have you considered reading the file line by line instead of reading it as whole, splitting the lines, and then processing them in memory?


================
Comment at: unittests/Support/Host.cpp:29
+    if (is_contained(SupportedArchs, Host.getArch()))
+      return true;
+
----------------
May want to check tuple `<OS, arch>`, but that can be changed when it'll be needed.


https://reviews.llvm.org/D25564