Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't set worker thread CPU affinity #1884

Merged
merged 1 commit into from
Aug 28, 2024
Merged

Don't set worker thread CPU affinity #1884

merged 1 commit into from
Aug 28, 2024

Conversation

plajjan
Copy link
Contributor

@plajjan plajjan commented Aug 28, 2024

This is triggered right now by a problem with Zig build concurrency. Zig gets the number of parallel worker threads to run by inspecting its affinity. This is a great approach since it works in containers / cgroups, unlike when just inspecting /proc/cpuinfo or similar. However, zig is run as a subprocess to actonc which in turn is run as a subprocess to acton, which is an Acton program. The Acton RTS sets the thread affinity in order to pin each worker thread to a CPU core. This affinity is apparently inherited from the acton RTS worker threads to actonc and finally to zig, so it thinks it can only run on a single CPU and thus.. it's rather slow.

I have been pondering the thread affinity topic in general for quite some time. I don't have any hard evidence, but I think that pinning to cores might be a premature optimization. It certainly should work better for server workloads, but for desktop workloads, which frankly we are doing more of right now (in development), I wouldn't be surprised if it works better without affinity.

Anyway, turning it off isn't a biggie right now. Zig parallelism is the most important thing and no worker affinity is fine. Let's figure out how to move forward on this in the longer term.

I did look into getting libuv to set affinity for the new process etc but there's nothing like that in libuv 1.x. I tried upgrading to master but then tlsuv fails.

I also quickly tried to fiddle with thread affinity when I realized that we have our own pthread_setaffinity_np for macos.. and libuv has some thread affinity functions but I don't think they work on Macos. Until we sort this out, we disable thread affinity!

This should speed up our cross-compilation tests in CI by quite a bit!

Fixes #1880

This is triggered right now by a problem with Zig build concurrency. Zig
gets the number of parallel worker threads to run by inspecting its
affinity. This is a great approach since it works in containers /
cgroups, unlike when just inspecting /proc/cpuinfo or similar. However,
zig is run as a subprocess to actonc which in turn is run as a
subprocess to acton, which is an Acton program. The Acton RTS sets the
thread affinity in order to pin each worker thread to a CPU core. This
affinity is apparently inherited from the acton RTS worker threads to
actonc and finally to zig, so it thinks it can only run on a single CPU
and thus.. it's rather slow.

I have been pondering the thread affinity topic in general for quite
some time. I don't have any hard evidence, but I think that pinning to
cores might be a premature optimization. It certainly should work better
for server workloads, but for desktop workloads, which frankly we are
doing more of right now (in development), I wouldn't be surprised if it
works better without affinity.

Anyway, turning it off isn't a biggie right now. Zig parallelism is the
most important thing and no worker affinity is fine. Let's figure out
how to move forward on this in the longer term.

I did look into getting libuv to set affinity for the new process etc
but there's nothing like that in libuv 1.x. I tried upgrading to master
but then tlsuv fails.

I also quickly tried to fiddle with thread affinity when I realized that
we have our own pthread_setaffinity_np for macos.. and libuv has some
thread affinity functions but I don't think they work on Macos. Until we
sort this out, we disable thread affinity!

This should speed up our cross-compilation tests in CI by quite a bit!
@plajjan plajjan merged commit e8a233b into main Aug 28, 2024
23 checks passed
@plajjan plajjan deleted the disable-thread-affinity branch August 28, 2024 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Verify Zig build concurrency
1 participant