SpinLock: Overhaul false sharing prevention #99168
Merged
Conversation
This file contains 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
|
@yretenai, following up here. Could you please test this one? |
RandomShaper
force-pushed
the
even_better_spinlock
branch
3 times, most recently
from
058a1f8 to
5eb30dd
Compare
akien-mga
reviewed
Nov 13, 2024
RandomShaper
force-pushed
the
even_better_spinlock
branch
3 times, most recently
from
557c73e to
67737d5
Compare
|
1000 force-pushes later... 😅 |
RandomShaper
force-pushed
the
even_better_spinlock
branch
from
67737d5 to
a5caeeb
Compare
|
AVX2 and AVX512 builds are fixed with this patchset. Please check if performance is improved or degraded with the new cacheline stuff. |
|
@Calinou, could you please give this a go? |
RandomShaper
added
the
cherrypick:4.3
|
Tested myself. Performance stays compared to prior to this PR, noise aside. |
akien-mga
approved these changes
Dec 6, 2024
|
Thanks! |
|
Still, for the records, we would really like to be able to rely on alignment and base this on it. @hpvb is looking into making Godot allocation aware of alignment. |
|
@RandomShaper I'll make a pr for memory.cpp/h asap. It should be a relatively small fix. |
akien-mga
added
regression
and removed
cherrypick:4.3
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.
This changes how false sharing in
SpinLockis prevented. Formerly, alignment attributes were used. The problem with that is objects containingSpinLocks could be allocated in a way that the alignment is not honored, leading to crashes.This PR also applies the new approach to the Apple implementation.
This approach could be refactored into a more general one that we may want to use in other sync primitives. However, the point is now to fix the only case we have at the moment.
Fixes crashes stated in #85167 (comment).
Bugsquad edit: