Bugfix in Semaphore::Acquire for non-windows platforms #1456
Conversation
The count was updated before waiting, meaning that the counter would become -(number of waiting threads) and the semaphore would not wake up until at least the same amount of releases was done. In practice this meant that the main thread had to do the last (number of threads) jobs, slowing down the simulation a bit.
|
| @@ -35,6 +35,7 @@ For breaking API changes see [this document](https://github.com/jrouwe/JoltPhysi | |||
| * Fixed CharacterVirtual::Contact::mIsSensorB not being persisted in SaveState. | |||
| * Fixed CharacterVirtual::Contact::mHadContact not being true for collisions with sensors. They will still be marked as mWasDiscarded to prevent any further interaction. | |||
| * Fixed numerical inaccuracy in penetration depth calculation when CollideShapeSettings::mMaxSeparationDistance was set to a really high value (e.g. 1000). | |||
| * Bugfix in Semaphore::Acquire for non-windows platform. The count was updated before waiting, meaning that the counter would become -(number of waiting threads) and the semaphore would not wake up until at least the same amount of releases was done. In practice this meant that the main thread had to do the last (number of threads) jobs, slowing down the simulation a bit. | |||
There was a problem hiding this comment.
How bad is this exactly? How many jobs can you expect to be run during an update? If someone were to run with a GetMaxConcurrency of say 64, would you then end up running the large majority of jobs on the main thread?
There was a problem hiding this comment.
The best improvement I saw was 3% on the Ragdoll Performance test and this was roughly when using 8 threads. When using 20 threads (I have 20 hyper threaded cores) the difference was not measurable. I only noticed it because I have a Vulkan/Metal renderer now and it deadlocked in the MultithreadedTest in the samples app (which I wasn't able to run before on non-windows platforms).
There was a problem hiding this comment.
(This deadlock is only possible in this particular test because it starts its own jobs outside the physics update and blocks the main thread when shutting down)
No description provided.