-
Notifications
You must be signed in to change notification settings - Fork 157
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
Thread pool should support CPU affinity #3463
Comments
@oleksiyskononenko if I may ask, how did U discover this? What's d implication/relevance? Is it sth you are working on (so I can see how U resolved it)? Is there a way to compare before and after the fix? Thanks |
There is nothing to be discovered here, because we are using our own thread pool and we never implemented CPU affinity in that pool. I do not work on this for the moment, but this could be an important next task. Yeah, there are a lot of ways to compare, just benchmark parallel performance before/after implementation on various systems. |
We use part of datatable's c++ code in our in-house project(s) and we noticed that when ther're a lot cpu consumption from another running processes it may affect dt's thread pool performance. Or when there were a lot of tasks scheduled and dt's thread pool was misconfigured. As @oleksiyskononenko mentioned we need precise benchmarks mimicking those scenarios to see the difference. |
I guess we saw the effect even earlier, in particular, #3329 could be related. |
@sh1ng not sure who I should reach out to, since @oleksiyskononenko is not the lead on this project. who can I reach out to , in terms of PRs and more? |
I'll discuss this with @st-pasha and some other folks. |
@samukweku thank you for your contribution. As you see there's no active development for the project. I've started an internal discussion about possible options. |
@sh1ng glad to hear. I do hope activity resumes soon. It is a lovely project |
It seems that currently our thread pool doesn't set CPU affinity, that may have serious performance implications.
The text was updated successfully, but these errors were encountered: