-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[donotmerge] always bounds check in UnsafeBufferPointer #70794
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
base: main
Are you sure you want to change the base?
[donotmerge] always bounds check in UnsafeBufferPointer #70794
Conversation
|
@swift-ci please benchmark |
Performance (x86_64): -O
Code size: -O
Performance (x86_64): -Osize
Code size: -Osize
Performance (x86_64): -Onone
Code size: -swiftlibsHow to read the dataThe tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.If you see any unexpected regressions, you should consider fixing the Noise: Sometimes the performance results (not code size!) contain false Hardware Overview |
- hoist bounds-checking out of the insertion sort loop
19ad1f0 to
4348300
Compare
|
@swift-ci please benchmark |
|
@swift-ci smoke test macOS platform |
Performance (x86_64): -O
Code size: -O
Performance (x86_64): -Osize
Code size: -Osize
Performance (x86_64): -Onone
Code size: -swiftlibsHow to read the dataThe tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.If you see any unexpected regressions, you should consider fixing the Noise: Sometimes the performance results (not code size!) contain false Hardware Overview |
|
For -fbounds-safety, we add the ConstraintsElimination pass to the LLVM optimization pipeline to eliminate bounds checks. Is Swift already doing it? Would we consider doing it if the results are good? |
|
I'm totally new to this programming context, so I apologize if I'm missing something obvious. Does this change cover UnsafeMutableRawBufferPointer's copyBytes and copyMemory functions? (Or did they do bounds checking already before this patch?) Same question for UnsafeRawBufferPointer's load...(...) functions. How does this change interact with copyBytes(from:) and copyBytes(to:from:) style functions in Data and DataProtocol? Does the type's public API boundary automatically enforce checking in those cases? |
|
@apple-fcloutier The Swift compiler already uses a constraints-elimination pass. It doesn't catch everything, but it's already pretty good, as illustrated by how mild the effect was here. A notable regression involved sorting, which performs a backwards-iteration (an atypical pattern.) @geoffreygaren We will consider separately whether to turn it on for raw buffers, and it would involve every access. This PR hasn't yet tackled everything in |
|
These are really encouraging results. I was a little nervous about this change, but if the regressions are this modest, then I'm all for it! |
The bounds-checking approach of
UnsafeBufferPointeris to bounds-check only in debug mode. We wonder whether it would be more principled to bounds-check in all cases, with an available API to intentionally elide bounds-checking entirely in performance-sensitive cases. This PR enables release-mode bounds-checking in the functions and subscripts ofUnsafeBufferPointerandUnsafeMutableBufferPointer. No API for unchecked access is added at this time.Addresses rdar://93206377