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

[Andrew Ng via Phabricator via llvm-commits]

andrewng added a comment.

In D147493#4281429 <https://reviews.llvm.org/D147493#4281429>, @MaskRay wrote:

> 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);
>   }

Yes, this LGTM for this case, but I think something also needs to be done for the explicit setting of the `--threads` option just above. I think it needs: `config->threadCount = parallel::strategy.compute_thread_count()` after the setting of `parallel::strategy`. IIUC, `compute_thread_count()` will take into account any CPU affinity settings. Having now looked more closely at that code, I do wonder if the default value for `config->thinLTOJobs` should be the same as `config->threadCount`, i.e. take into account any CPU affinity settings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147493