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

Completely removing stackless threads #1777

Merged
merged 3 commits into from Oct 1, 2015
Merged

Conversation

sithhell
Copy link
Member

@sithhell sithhell commented Oct 1, 2015

This patch removes all notions of stackless threads. In addition thread_data_base
has been removed because there was no need for it anymore.

In addition, this fixes the currently failing unhandled_exception_582 test.

@AntonBikineev
Copy link
Contributor

Isn't thread_data_base supposed to be extendable?

@sithhell
Copy link
Member Author

sithhell commented Oct 1, 2015

I think the main motivation was to have stackless threads. thread_data_base was introduced when implementing those (#804): 75f9a92#diff-11f383c8f29752a630098984712b40bb

I wouldn't think we still need the base class. Right now we don't have a usecase for this, it's an internal class anyway, so we can switch back whenever the need to have it customizable again arises, IMHO.

public:
typedef thread_function_type function_type;

struct tag {};
typedef util::spinlock_pool<tag> mutex_type;

struct pool_base
struct pool_type
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wrapper over lockfree::pool_type is not needed now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, thanks.

@hkaiser
Copy link
Member

hkaiser commented Oct 1, 2015

Please add a note to the whats_new.qbk explaining that we removed this and why.

This patch removes all notions of stackless threads. In addition thread_data_base
has been removed because there was no need for it anymore.
@@ -715,8 +715,7 @@ namespace hpx { namespace actions
#define HPX_ACTION_USES_HUGE_STACK(action) \
HPX_ACTION_USES_STACK(action, threads::thread_stacksize_huge) \
/**/
#define HPX_ACTION_DOES_NOT_SUSPEND(action) \
HPX_ACTION_USES_STACK(action, threads::thread_stacksize_nostack) \
#define HPX_ACTION_DOES_NOT_SUSPEND(action)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we turn this into a warning saying that the feature has been removed and the macro will be removed eventually as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On 10/01/2015 12:20 PM, Hartmut Kaiser wrote:

@@ -715,8 +715,7 @@ namespace hpx { namespace actions
#define HPX_ACTION_USES_HUGE_STACK(action)
HPX_ACTION_USES_STACK(action, threads::thread_stacksize_huge)
/**/
-#define HPX_ACTION_DOES_NOT_SUSPEND(action) \

  • HPX_ACTION_USES_STACK(action, threads::thread_stacksize_nostack)
    +#define HPX_ACTION_DOES_NOT_SUSPEND(action)

Should we turn this into a warning saying that the feature has been removed and the macro will be removed eventually as well?

It might be save to remove this completely. We didn't announce that
feature and it is not documented.

@hkaiser
Copy link
Member

hkaiser commented Oct 1, 2015

Thanks! LGTM now!

sithhell added a commit that referenced this pull request Oct 1, 2015
@sithhell sithhell merged commit 0598a44 into master Oct 1, 2015
@sithhell sithhell deleted the remove_stackless_threads branch October 1, 2015 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants