aganea marked an inline comment as done.
aganea added inline comments.

================
Comment at: llvm/lib/Support/Threading.cpp:94
+  // uselessly induce more context-switching and cache eviction.
+  if (!ThreadsRequested || ThreadsRequested > (unsigned)MaxThreadCount)
+    return MaxThreadCount;
----------------
@mehdi_amini You mean this? Testing was showing degraded performance if 
ThreadsRequested > MaxThreadCount, so I thought it'd maybe better to prevent 
that situation. More soft threads than hardware threads means you'd pay for 
context switching, for the cache eviction and for extra memory pressure (even 
more if the allocator has per-thread pools).
Do you see a cases where not having this test would be valuable? Providing 
`--threads=50`, and creating 50-threads ThreadPool when your CPU only supports 
12 hardware threads? The only case I can think of is usage of async operations 
in threads, but then your ThreadPool sounds more like a queue, and maybe it's 
not the right hammer for the nail? Should we support that case and explicitly 
tag the ThreadPool constructor in client code with something like 
ThreadPool(50, AcknowledgeDegradedPerformance)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71775/new/

https://reviews.llvm.org/D71775



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to