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

Adding basic support for stackless threads #4145

Merged
merged 1 commit into from Oct 22, 2019

Conversation

@hkaiser
Copy link
Member

hkaiser commented Oct 17, 2019

Fixes #4030

@hkaiser hkaiser added this to the 1.4.0 milestone Oct 17, 2019
@hkaiser hkaiser force-pushed the fixing_4030 branch 3 times, most recently from 247dafc to 5605bfb Oct 17, 2019
Copy link
Contributor

msimberg left a comment

Very nice! Note that this conflicts a bit with #4142 (register_thread functions), but let's go with this one first as it's smaller and I'll deal with the conflicts in the other PR.

@hkaiser

This comment has been minimized.

Copy link
Member Author

hkaiser commented Oct 18, 2019

@msimberg I think that this requires a bit more experimentation/time. Let's wait with this one until we are sure that we see a gain and do not introduce undue overheads to the common case. The code as is decreases the performance of running 500000 tasks by about 3% compared to master (for the stack-full case), but improves things for the new stack-less threads by 7% (compared to master). I would like to do a better analysis and avoid any decrease in performance compared to what we have.

@hkaiser hkaiser force-pushed the fixing_4030 branch from 5605bfb to f620f28 Oct 18, 2019
@hkaiser

This comment has been minimized.

Copy link
Member Author

hkaiser commented Oct 19, 2019

FWIW, the latest changes ensure that stack-full threads perform no worse than what we have on master. Stack-less threads give a performance win of about 7%. I think this can go ahead once all tests have been cleaned up.

@hkaiser hkaiser force-pushed the fixing_4030 branch 4 times, most recently from a1026b6 to b82b08a Oct 20, 2019
@hkaiser hkaiser marked this pull request as ready for review Oct 20, 2019
@hkaiser

This comment has been minimized.

Copy link
Member Author

hkaiser commented Oct 20, 2019

@msimberg, @sithhell: this is good to be merged, I think.

Copy link
Contributor

msimberg left a comment

Thanks! What got rid of the added overhead with stackful coroutines that you had initially?

@msimberg

This comment has been minimized.

Copy link
Contributor

msimberg commented Oct 21, 2019

This is extreme bikeshedding, but I think thread_data_stackfull should be spelled thread_data_stackful. If you don't mind the change I volunteer to do the change.

@hkaiser

This comment has been minimized.

Copy link
Member Author

hkaiser commented Oct 21, 2019

This is extreme bikeshedding, but I think thread_data_stackfull should be spelled thread_data_stackful. If you don't mind the change I volunteer to do the change.

Good point. I'll do that change as I need to touch up on the TSS stuff anyways.

@hkaiser

This comment has been minimized.

Copy link
Member Author

hkaiser commented Oct 21, 2019

What got rid of the added overhead with stackful coroutines that you had initially?

Replacing the virtual operator()() with this removed the overhead:
https://github.com/STEllAR-GROUP/hpx/pull/4145/files#diff-11f383c8f29752a630098984712b40bbR586-R593

@hkaiser

This comment has been minimized.

Copy link
Member Author

hkaiser commented Oct 21, 2019

@msimberg I have renamed the stackful thread_data as requested.

@msimberg msimberg merged commit 458e59a into master Oct 22, 2019
16 checks passed
16 checks passed
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
build-and-test Workflow: build-and-test
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
pycicle daint-clang-newest Build errors 0
Details
pycicle daint-clang-newest Config errors 0
Details
pycicle daint-clang-newest Test errors 0
Details
pycicle daint-clang-oldest Build errors 0
Details
pycicle daint-clang-oldest Config errors 0
Details
pycicle daint-clang-oldest Test errors 0
Details
pycicle daint-gcc-newest Build errors 0
Details
pycicle daint-gcc-newest Config errors 0
Details
pycicle daint-gcc-newest Test errors 0
Details
pycicle daint-gcc-oldest Build errors 0
Details
pycicle daint-gcc-oldest Config errors 0
Details
pycicle daint-gcc-oldest Test errors 0
Details
@msimberg msimberg deleted the fixing_4030 branch Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.