Sort contact constraints for determinism when parallel feature is enabled#712
Merged
Sort contact constraints for determinism when parallel feature is enabled#712
parallel feature is enabled#712Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Objective
Multithreaded determinism is currently broken! This is because #699 accidentally resulted in contact constraint order being non-deterministic. CI didn't catch this, because it currently doesn't use the
parallelfeature.Solution
When the
parallelfeature is enabled, store the pair index for eachContactConstraint, and just sort the constraints based on these indices.I suspect that a better approach that doesn't require sorting exists, but for now, this works as a simple hot-fix. I also tried the simple approach of returning
Vecs from Bevy'spar_splat_map_mut, but that would mean that we can't reuse the constraint buffers, and have to allocate from scratch every time. Either way, there's more to try and experiment with here.Because the constraints within chunks are still largely sorted, the sorting ends up being quite fast. In the
pyramid_2dexample, it increased the time of "Update Contacts" from about 0.42 ms to about 0.44 ms with ~3025 constraints.I also enabled the
parallelfeature for CI to hopefully catch multithreading problems like this automatically in the future!