[Runtime] Fix swift_slowAlloc to respect its alignMask parameter. #14375
Conversation
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
Instead of calling malloc, call AlignedAlloc. That calls posix_memalign on platforms where it's available. The man page cautions to use it judiciously, but Apple OSes and Linux implement it to call through to malloc when the alignment is suitable. Presumably/hopefully other OSes do the same. rdar://problem/22975669
|
@swift-ci please test |
|
@swift-ci please benchmark |
jrose-apple
reviewed
Feb 6, 2018
stdlib/public/runtime/Heap.cpp
Outdated
| @@ -23,12 +23,11 @@ | |||
| using namespace swift; | |||
|
|
|||
| void *swift::swift_slowAlloc(size_t size, size_t alignMask) { | |||
| // FIXME: use posix_memalign if alignMask is larger than the system guarantee. | |||
| void *p = malloc(size); | |||
| void *p = AlignedAlloc(size, alignMask + 1); | |||
There was a problem hiding this comment.
Is alignMask guaranteed to be a power-of-two-minus-one?
There was a problem hiding this comment.
That should be fine. What is a problem is that posix_memalign requires that the alignment be no smaller than sizeof(void*). We have at least one place that calls swift_slowAlloc(size, 0) because it needs no alignment constraints.
There was a problem hiding this comment.
AlignedAlloc makes sure the alignment is sufficiently large before calling through to posix_memalign.
There was a problem hiding this comment.
That is unfortunate for performance. It would be better if we could satisfy posix_memalign's constraints directly. That would be a much wider change.
Build comment file:Optimized (O)Regression (2)
Improvement (3)
No Changes (355)
Unoptimized (Onone)Regression (5)
Improvement (24)
No Changes (331)
Hardware Overview |
…fficiently small. rdar://problem/22975669
|
Let's see how it does with a direct call to malloc for alignments where that's suitable. |
|
@swift-ci please benchmark |
Build comment file:Optimized (O)Regression (1)
Improvement (1)
No Changes (358)
Unoptimized (Onone)Regression (6)
Improvement (3)
No Changes (351)
Hardware Overview |
|
@swift-ci please test |
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.
Instead of calling malloc, call AlignedAlloc. That calls posix_memalign on platforms where it's available. The man page cautions to use it judiciously, but Apple OSes and Linux implement it to call through to malloc when the alignment is suitable. Presumably/hopefully other OSes do the same.
rdar://problem/22975669