Scheduler for grand central libdispatch queue #1258
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This is a good contribution! In the short-term a free-standing scheduler may make sense but optimally we would hide this under the system scheduler as the default implementation on apple platforms (and configurable when libdispatch is installed elsewhere through the build system). That way nobody needs to think about the availability in the code. Do you think it's worth merging separately anyway or should we wait and hide this and the static_thread_pool under the system scheduler? |
|
/ok to test |
|
We'll need to add some MacOS test runners. |
|
@LeeHowes I just saw the system executor proposal and the PR here and I have to say its awesome. I guess its fine to wait for system executor PR to progress and let it merge with system executor PR. Its going to make things platform independent. |
|
I don't think there's a reason to wait for the system scheduler. I would land this right now, except that without any MacOS test runners, it's not being tested in CI and that's not ok. I'm no GH Actions ninja. I'll see if I can find out how to get some MacOS runners. |
|
… On Sun, Feb 25, 2024, 10:07 PM Eric Niebler ***@***.***> wrote:
I don't think there's a reason to wait for the system scheduler. I would
land this right now, except that without any MacOS test runners, it's not
being tested in CI and that's not ok. I'm no GH Actions ninja. I'll see if
I can find out how to get some MacOS runners.
—
Reply to this email directly, view it on GitHub
<#1258 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AARMSLBFA64XGDERNSUST23YVQRCHAVCNFSM6AAAAABDXCO54OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRTGM4DGNZTHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@RishabhRD am i correct in assuming that you do MacOS development? i have a PR that adds a MacOS test runner, and it seems that a lot of stdexec's tests are SEGFAULT'ing there (see here). Could I trouble you to grab a stack trace and post it here? |
|
@ericniebler I don't do mac os development (I do linux development). But luckily I had setup a mac yesterday itself for C++ 😅 . Pasting a stacktrace generated using lldb |
|
Also pasting some tests that are not segfaulting but failing on my mac: It can be possible that it is due to the above issue only. But can be useful to have a look. |
|
@ericniebler regarding the failing test cases without segfault, with a little effort I printed the values for {?} == {?} in output. It seems like set_value is being called with some garbage value. |
|
Hrm. Try with Once we get the macos tests green in CI, I can merge this PR. |
|
@ericniebler no its not a trouble. I am doing this because I find it fun. (Believe me, its way better than work at my day job 😅). It makes me feel I am doing something significant and interesting. I think there is problem with connect call for schedule_from sender. auto snd = ex::schedule_from(inline_scheduler{}, ex::just(13));
auto op = ex::connect(std::move(snd), expect_value_receiver{13});This is leading to a crash when using address sanitizer. However, it was not producing any stacktrace as such to my surprise. auto snd = ex::schedule_from(inline_scheduler{}, ex::just(13));
ex::sync_wait(snd);Stacktrace: I have disabled other test cases to focus on this one only, that's why there is only one assertion. (I have modified this test case also to inspect the error). Didn't get chance to look much into the stacktrace right now. Will look at it tomorrow. |
|
Thanks for this. I would bet money that this is a problem with the uniqueness of lambda identifiers across translation units. I bet there is a name collision in the __sexpr code due to its use of lambdas. The implementation of one __sexpr is accidentally linked with the implementation of an unrelated __sexpr from another TU, and chaos ensues. The question is how to teach the LLVM toolchain on MacOS to avoid creating symbol collisions across TUs with lambdas defined at namespace scope. EDIT: template <
class _Descriptor,
auto _DescriptorFn =
[] {
return _Descriptor();
}>
inline constexpr auto __descriptor_fn_v = _DescriptorFn;
template <class _Tag, class _Data, class... _Child>
inline constexpr auto __descriptor_fn() {
return __descriptor_fn_v<__detail::__desc<_Tag, _Data, _Child...>>;
} |
|
/ok to test |
2 similar comments
|
/ok to test |
|
/ok to test |
ericniebler
force-pushed
the
libdispatch_impl
branch
from
f4ed191
to
d3b07df
Compare
|
/ok to test |
|
/ok to test |
|
Hooray! stdexec has a GCD scheduler on MacOS. Thank you @RishabhRD. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In his talk "Better Code: Concurrency", @sean-parent said that a tasking system built upon libdispatch would be his 1.0 for the performance. So, just thought implement the same as I needed it for one of my project.
This can be helpful for people who want to use library and have high performance requirements.
I have profiled the libdispatch queue code over a ray tracing application against already existing static_thread_pool implementation. My usecase may not cover all the scenarios but can be good to look into some real applications.
My machine have 8 cores with 8 gigs ram.
libdispatch
static_thread_pool
PS: I know the current bulk implementation of static_thread_pool is not the most optimal approach for my usecase.