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

Allow using ThreadPool synchronously #1346

Merged
merged 5 commits into from Aug 21, 2015

Conversation

Projects
None yet
4 participants
@crabmusket
Contributor

crabmusket commented Jul 5, 2015

Previously, a test was using ThreadPool::flushWorkItems as if it would wait for all work items to complete before returning. This is incorrect - see #1174. Actually, flushWorkItems just waits until all items have begun, so if you have long-running items, they may be still happening.

This PR adds two things:

  1. A method hasExecuted on WorkItem to see if the item has finished executing.
  2. A method waitForAllItems on ThreadPool to block until no items are currently executing.

This lets you use the ThreadPool completely synchronously, as demonstrated by the new tests.

@crabmusket crabmusket added Bug Tests and removed Bug labels Jul 5, 2015

@crabmusket crabmusket changed the title from Allow using the ThreadPool synchronously to Allow using ThreadPool synchronously Jul 5, 2015

@crabmusket crabmusket added this to the 3.8 milestone Jul 5, 2015

@rextimmy

This comment has been minimized.

Show comment
Hide comment
@rextimmy

rextimmy Jul 6, 2015

Contributor

Ooh great work :) i had this problem when i was playing around with the thread pools and i ended up testing with intel tbb instead.

Contributor

rextimmy commented Jul 6, 2015

Ooh great work :) i had this problem when i was playing around with the thread pools and i ended up testing with intel tbb instead.

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket Jul 6, 2015

Contributor

Huh, TBB does look cool.

Contributor

crabmusket commented Jul 6, 2015

Huh, TBB does look cool.

@rextimmy

This comment has been minimized.

Show comment
Hide comment
@rextimmy

rextimmy Jul 6, 2015

Contributor

Oh i meant i had the synchronizing problems when i was testing out a few things with T3D's ThreadPool. Yep TBB is a very nice library.

Contributor

rextimmy commented Jul 6, 2015

Oh i meant i had the synchronizing problems when i was testing out a few things with T3D's ThreadPool. Yep TBB is a very nice library.

@Areloch

This comment has been minimized.

Show comment
Hide comment
@Areloch

Areloch Aug 10, 2015

Contributor

I'll be honest. I'm not sure how to test this. Glancing at the changes, I see it adds the stuff, but I don't know how to kick off the testing of it?

Contributor

Areloch commented Aug 10, 2015

I'll be honest. I'm not sure how to test this. Glancing at the changes, I see it adds the stuff, but I don't know how to kick off the testing of it?

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket Aug 10, 2015

Contributor

You could run the unit tests ;). torque.exe runTests.cs

Contributor

crabmusket commented Aug 10, 2015

You could run the unit tests ;). torque.exe runTests.cs

@Areloch Areloch merged commit 31afbed into GarageGames:development Aug 21, 2015

@crabmusket crabmusket deleted the crabmusket:fix-threadpool-tests branch Aug 22, 2015

@JeffProgrammer

This comment has been minimized.

Show comment
Hide comment
@JeffProgrammer

JeffProgrammer Sep 30, 2015

Contributor

I kind of makes sense on how flush originally worked as intended for threadpool. at least in the case of say OpenGL, a flush command simply "flushes" the GL commands to the gpu, while swapBuffers actually is the blocking call. I kinda always assumed that a flush ment not blocking, rather just push everything out of the queue. However, I do like having the option for sync! 👍

Not completely ralated, so off topic a bit, but just clarifying what I think their intentions were behind flush, if you look at other things that use dispatchers.

Contributor

JeffProgrammer commented Sep 30, 2015

I kind of makes sense on how flush originally worked as intended for threadpool. at least in the case of say OpenGL, a flush command simply "flushes" the GL commands to the gpu, while swapBuffers actually is the blocking call. I kinda always assumed that a flush ment not blocking, rather just push everything out of the queue. However, I do like having the option for sync! 👍

Not completely ralated, so off topic a bit, but just clarifying what I think their intentions were behind flush, if you look at other things that use dispatchers.

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket Oct 1, 2015

Contributor

Thanks, that makes sense!

Contributor

crabmusket commented Oct 1, 2015

Thanks, that makes sense!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment