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

Execute background work as HPX thread #2498

Merged
merged 4 commits into from Feb 14, 2017

Conversation

Projects
None yet
2 participants
@sithhell
Copy link
Member

commented Feb 12, 2017

This patch executes the background work as HPX thread now, mainly to allow
received parcels to execute direct actions immediately now.

Execute background work as HPX thread
This patch executes the background work as HPX thread now, mainly to allow
received parcels to execute direct actions immediately now.

@sithhell sithhell force-pushed the background_hpx_thread branch from ebba590 to c75e7fa Feb 12, 2017

[&](thread_state_ex_enum) -> thread_result_type
{
while(true)
{

This comment has been minimized.

Copy link
@hkaiser

hkaiser Feb 13, 2017

Member

I think this could be changed to:

while(running) { ... }

where running is captured from the enclosing frame (by reference).

This comment has been minimized.

Copy link
@sithhell

sithhell Feb 13, 2017

Author Member

hmm, not sure, is the parcelport and AGAS down when running is false?

This comment has been minimized.

Copy link
@hkaiser

hkaiser Feb 13, 2017

Member

good point

{
scheduler.SchedulingPolicy::schedule_thread(
background_result.second.get(), num_thread);
}
}

This comment has been minimized.

Copy link
@hkaiser

hkaiser Feb 13, 2017

Member

The lines above are never executed (currently), why add them? background_result.second is always equal to nullptr.

This comment has been minimized.

Copy link
@sithhell

sithhell Feb 13, 2017

Author Member

It currently is nullptr, yes, but what if some background_result uses suspend_boost or yields a thread to be scheduled?

This comment has been minimized.

Copy link
@hkaiser

hkaiser Feb 13, 2017

Member

If we introduce that the background_thread yields to another thread we can always re-introduce what you have today. For now it's simply dead code.

This comment has been minimized.

Copy link
@sithhell

sithhell Feb 13, 2017

Author Member

let's put an assert here, at the very least.

{
scheduler.SchedulingPolicy::schedule_thread(
background_result.second.get(), num_thread);
}
}

This comment has been minimized.

Copy link
@hkaiser

hkaiser Feb 13, 2017

Member

Same as above, background_result.secondvis always nullptr, currently.

{
if (callbacks.background_())
idle_loop_count = 0;
thread_result_type background_result = (*background_thread)();

This comment has been minimized.

Copy link
@hkaiser

hkaiser Feb 13, 2017

Member

Would you see a way to still reset idle_loop_count if the background thread has performed any ('useful') work?

This comment has been minimized.

Copy link
@hkaiser

hkaiser Feb 13, 2017

Member

I take that back, I saw what you have done now.

This comment has been minimized.

Copy link
@sithhell

sithhell Feb 13, 2017

Author Member

I hope this is done within the thread functions lambda. Did you see any problems here?

@hkaiser

This comment has been minimized.

Copy link
Member

commented Feb 13, 2017

@sithhell I have merged from master and resolved the conflict. I also replaced the dead code with an assert.

@hkaiser

This comment has been minimized.

Copy link
Member

commented Feb 13, 2017

@sithhell: I just now understood what you had in mind wrt the background_result. Sorry for being dense, you were right - it's not necessarily dead code after all. I'll go ahead and put it back in.

@hkaiser hkaiser merged commit cb79d00 into master Feb 14, 2017

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@hkaiser hkaiser deleted the background_hpx_thread branch Feb 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.