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

question : support sleep-waiting ? #31

Closed
zhaijialong opened this issue Aug 27, 2019 · 15 comments
Closed

question : support sleep-waiting ? #31

zhaijialong opened this issue Aug 27, 2019 · 15 comments
Assignees
Labels

Comments

@zhaijialong
Copy link

Hi,
It looks like all WaitFor* methods in enkiTS is busy waiting. In some situations they will result in long spinning time. So what is the preferred/proper way to use sleep-waiting for tasks finishing in enkiTS ?

@dougbinks
Copy link
Owner

All task threads implement sleep-waiting when there is no work for the threads, however the Waitfor* methods do not as you've noted.

I have that on my list of things to implement - the tricky part here is ensuring decent performance whilst also ensuring the waiting thread is woken when either new work is available or the the task being waited for is complete. It's not that tricky but it needs a fair amount of testing to make sure everything works well.

In practice for my own needs and that of other code I've looked at using enkiTS the benefit of adding a sleep-wait to Waitfor was minimal.

Using something like std::condition_variable in your own code would be possible here, but it would also prevent the thread from doing work as I don't expose whether there is work available so you'd potentially loose some performance.

Task dependencies / continuations will also help solve this, as then you'd not need to call WaitforTask.

@zhaijialong
Copy link
Author

Thanks for your detailed reply.

My current usecase is very simple. I have two threads, the "main thread" continually spawns tasks which is pinned to another "worker thread". The tasks are actually some D3D11 api calls, so they must be pinned to a single thread. At some point, I need to wait for all tasks finishing so that I can invoke D3D11 api in the main thread. (I know that's not optimal, but is simple for such as gpu data readback.)

I think a sleep-waiting makes sense in my situation. It saves some CPU cycles for other things, such as audio, physics, etc.

@dougbinks
Copy link
Owner

dougbinks commented Aug 27, 2019

Note that using a task manager like enkiTS sleep waiting doesn't save significant CPU cycles for other things in your program, it mainly improves power efficiency by allowing the core to sleep. If there is available work that work will be undertaken by the thread during the Waitfor* function.

So you can put your audio, physics etc. into tasks and then when a Waitfor* function is called they will be run if they are not being run by another task thread.

@zhaijialong
Copy link
Author

Unfortunately, the audio, physics have there own threading system( they are from external SDK, such as fmod/physX). So I can't put those "ogg decoding"/ "rigid body simulating" work to enkiTS.

@dougbinks
Copy link
Owner

For PhysX it looks like you can overload physx::PxCpuDispatcher to use enkiTS and spawn tasks which call task->Run() then task->Release() as per the threading documentation.

FMOD doesn't appear to have a tasking interface, and if you're loading compressed files on the fly there may be significant overhead. If so you might find using enkiTS but reducing the number of worker threads below that of the number of hardware threads works well.

Depending on various factors, you might find that enkiTS works well enough for you in this circumstance. It depends on how much work there is for the threading system versus other factors.

@dougbinks
Copy link
Owner

I'm going to think about some solutions for this, but one potential is to expose an event system so this can be done safely. If you're interested I can develop an experimental test for this.

Note that part of the problem here is enkiTS tries to make the task scheduler lock free, as locking can cause the entire task system to stall. Without a locking completion event, sleep waiting for task completion can potentially deadlock.

@zhaijialong
Copy link
Author

Sure, I'm interested in the "event system". Sounds like what I want.

@dougbinks
Copy link
Owner

I'm away from the computer a lot at the moment but I'll get around to this as soon as I can and get back to you on this thread/issue.

@dougbinks
Copy link
Owner

I've just added an implementation of sleep waiting in WaitforTask() to the branch dev_WaitforTaskEvent. This approach needs no change to user code, but may have a minor performance impact.

This is very experimental at this stage, and I haven't created a proper set of tests yet. There's a danger the task system may deadlock if there's an error in my design or code.

I've not implemented a blocking wait in WaitforAll as the design relies on the ordering of reading whether a task is complete and incrementing an atomic before a blocking wait. I'll have a think about a design for this next.

@dougbinks
Copy link
Owner

A workaround for your particular requirements might be to submit a very low priority pinned task to the thread you've submitted the other pinned tasks you are waiting on, then wait for that as it should complete last.

A variant of this is something I might use in the generic WaitforAll() blocking wait implementation.

@zhaijialong
Copy link
Author

zhaijialong commented Aug 29, 2019

Thank you,I’ll have a try.

@dougbinks
Copy link
Owner

The latest update resolves a potential deadlock, so if you've tried this branch I would update. I also have a WaitforAll() blocking wait implementation now but am still running tests on that before I push.

It may be some time before I merge to master, as I need to run a lot more tests before I'm confident that we can't get deadlocks.

@zhaijialong
Copy link
Author

zhaijialong commented Sep 2, 2019

The latest update works well for me.

And yes, as you said, I didn't get lots of performance(FPS) improvments, but got lots of power efficiency improvments.

before:
before
after:
after

I'm still very satisfied with the result, thank you so much for your help.

Should I close this issue ? Or you maybe want to keep it open for task tracking?

@dougbinks
Copy link
Owner

Excellent, thank you for trying this.

I'll keep this issue open until I finish the WaitforAll work, and merge to master.

@dougbinks dougbinks self-assigned this Sep 2, 2019
@dougbinks
Copy link
Owner

This is now available in the master branch as of version v1.2.

Many thanks for the feature request and follow up testing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants