[PATCH] D147493: [ELF] Cap parallel::strategy to 16 threads when --threads= is unspecified

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 19 13:20:11 PDT 2023


MaskRay added a comment.

In D147493#4279652 <https://reviews.llvm.org/D147493#4279652>, @andrewng wrote:

> Sorry, I was so focused on the value of the limit that I didn't really look at the details of the code change. Was it deliberate to have `config->threadCount` not match the number of threads in the `parallel::strategy`? IIUC, this probably doesn't affect correctness but might have an impact on efficiency.

Good catch. The code should set `config->threadCount`. `config->threadCount` is currently only used for parallel relocation scanning whether an over-sized value still works.
I think not setting `config->threadCount` is correct but weird. How about?

  } else if (config->threadCount > 16) {
    log("set maximum concurrency to 16, specify --threads= to change");
    config->threadCount = 16;
    parallel::strategy = hardware_concurrency(config->threadCount);
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147493



More information about the llvm-commits mailing list