-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Eliminate Sequence.SubSequence + Use @_borrowed #20454
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
Conversation
Most of the stdlib's properties don't need @_borrowed because they're @inlinable, but I did find one place in an overlay where it's probably sensible to make the operation use generalized accessors even if they're resilient.
This is a performance hack: inlining a coroutine can promote heap allocations of the frame to stack allocations, which is valuable out of proportion to the normal weight. There are surely more principled ways of getting this effect, though.
Remove split customization point Eliminate SubSequence from Sequence protocol Collapse LazyCollection Collapse LazyMapCollection Eliminate _SequenceWrapper Collapse LazyFilterCollection Collapse LazyDrop/PrefixWhileCollection Fix tests, ABI stability update Collapse FlattenSequence
@swift-ci please test |
@swift-ci please benchmark |
@@ -332,7 +332,7 @@ extension IndexingIterator: IteratorProtocol, Sequence { | |||
/// or bidirectional collection must traverse the entire collection to count | |||
/// the number of contained elements, accessing its `count` property is an | |||
/// O(*n*) operation. | |||
public protocol Collection: Sequence where SubSequence: Collection { | |||
public protocol Collection: Sequence { | |||
// FIXME: ideally this would be in MigrationSupport.swift, but it needs | |||
// to be on the protocol instead of as an extension | |||
@available(*, deprecated/*, obsoleted: 5.0*/, message: "all index distances are now of type Int") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still the case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean the commented out obsolete part? I need to go through and remove all those – but that's not an ABI thing so not as urgent as some other stuff.
Build failed |
!!! Couldn't read commit file !!! |
Okay, there are some perf numbers in there if you look at the log, although they don't seem particularly credible. Makes me wonder if the benchmark can be optimized to triviality now or something. |
@DougGregor getting this really strange error on the build bot that doesn't reproduce on any other machines:
Any ideas what might cause that? It only started happening when I introduced the final bits of conditional conformance to some sequences. |
@airspeedswift that can happen if two Swift classes have the same @objc names. Prior to macOS Mojave, |
These affected very similar benchmarks so trying them together.