search

deplinenoise / tundra

Tundra is a code build system that tries to be accurate and fast for incremental builds
MIT License
438 stars 74 forks source link

Fixed incorrect threads count on Linux #339

Closed emoon closed 1 year ago

emoon commented 1 year ago

On my Linux system (ArchLinux) using sysconf(_SC_NPROCESSORS_CONF); will return 128 while my actual thread count is 32. This will cause Tundra to print an warning that number of threads gets clamped to 64, but after that will then crash.

This PR doesn't fix the underlying crash, but instead uses C++11 std::thread::hardware_concurrency(); to get the number of hw threads in the system and this returns the correct value and now everything works as it should, but there is still another issue that makes tundra crash when thread count ends up being 128.

I know we use C++11 for Linux, but I'm unsure about the other targets. If that is true in all cases then it might be worth considering using this code path on all targets.

deplinenoise commented 1 month ago

According to a new issue, this is now again returning 128 on machines where it shouldn't. No idea why.

deplinenoise commented 1 month ago

See https://github.com/deplinenoise/tundra/pull/347

The root issue seems to be that on some Linux kernels there is some "hotswap" CPU path that allocates a massive number of fake CPUs in case you add some later. And for whatever reason that's what both the std::thread concurrency number and sysconf reports.

However there is a way to ask sysconf for "online" CPUs, and that should be a sane number.

questor commented 1 month ago

sorry for confusion, I have my own tundra-fork with a small thing for myself added (honestly: currently I have forgotten were I used my patch) and it might be that I used the code before emoons patch applied with my machine reporting 128 cores. I will test your latest fix tomorrow on my machine and will report if it's working (for me) or not.

questor commented 1 month ago

integrated your fixes and can confirm that it does not crash anymore and detects the correct amount of cores here in my environment with the latest (current) version. thanks for your help!