Skip to content
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

Performance improvements and availability updates for Collection.difference(from:using:) #25808

Merged
merged 3 commits into from Jul 1, 2019

Conversation

@numist
Copy link
Member

commented Jun 26, 2019

This PR dramatically improves the runtime performance of collection diffing. As near as I can tell the improvement is a constant factor in the neighbourhood of 50x.

It also adds benchmarks for both the Myers algorithm in addition to the collection diffing implementation itself, as the latter is expected to benefit from algorithmic improvements in the future.

Finally, it updates the availability macros now that (macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) have been announced.

The new difference(from:using:) calls out directly to a new function _myers(from:to:using:) that is a near-direct implementation of the algorithm described in http://www.xmailserver.org/diff2.pdf

@numist

This comment has been minimized.

Copy link
Member Author

commented Jun 26, 2019

@swift-ci Please test

@swift-ci

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2019

Build failed
Swift Test OS X Platform
Git Sha - 0fc5d6a

@numist

This comment has been minimized.

Copy link
Member Author

commented Jun 27, 2019

@swift-ci Please test

@swift-ci

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2019

Build failed
Swift Test OS X Platform
Git Sha - 0fc5d6a

@numist

This comment has been minimized.

Copy link
Member Author

commented Jun 27, 2019

@swift-ci Please test

@swift-ci

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2019

Build failed
Swift Test OS X Platform
Git Sha - 0fc5d6a

@swift-ci

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2019

Build failed
Swift Test Linux Platform
Git Sha - 0fc5d6a

@numist

This comment has been minimized.

Copy link
Member Author

commented Jul 1, 2019

@swift-ci please smoke test OS X

@numist numist merged commit f994fc3 into apple:master Jul 1, 2019

3 checks passed

Swift Test Linux Platform No test results found.
Details
Swift Test Linux Platform (smoke test)
Details
Swift Test OS X Platform (smoke test)
Details
@gottesmm

This comment has been minimized.

Copy link
Member

commented Jul 2, 2019

@numist can you commit the benchmarks in a separate PR next time? Then we can see the perf improvement! = )

@palimondo

This comment has been minimized.

Copy link
Collaborator

commented Jul 11, 2019

@numist …and we can properly review them, because there should be no legacyFactor on newly added benchmarks… and the let _ = ... should be a call to blackHole...

@palimondo

This comment has been minimized.

Copy link
Collaborator

commented Jul 11, 2019

I'm also a perplexed by the availability markings and especially the if #available checks in benchmarks… why are these gated to specific OS version, instead of swift version?
@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)

Isn't this SR-0240? There it said:
@available(swift, introduced: 5.1)

@lorentey

This comment has been minimized.

Copy link
Member

commented Jul 11, 2019

The language mode used at compile time is not related to the version of the stdlib that the code is actually running on. (For example, Swift 5 code can and will routinely run on 5.1 stdlib, and vice versa.) For production code, it is the runtime OS version that determines what features are actually available.

The situation is slightly different for us contributors, because we are building our own toolchains from GitHub, and features aren't necessarily tied to OS releases. However, it's a good enough approximation to suffice for now.

Having a separate version number for the stdlib isn't necessarily a bad idea, but I see it mostly as a convenience feature for stdlib contributors -- an extra indirection that would get rid of the need to list every OS on every single new API (not to mention 9999 placeholders). All app developers want to know is which OS version they need to target to start using specific stdlib features. E.g., to use CollectionDifference, the app needs to be running on iOS 13+.

@tonyarnold

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

I'm not sure I follow. Does this feature rely on something that's not backward deployable? If I build my library/app targeting macOS 10.14 using the Xcode 11/Swift 5.1 toolchain, I assumed that I could make use of improvements in that toolchain?

@lorentey

This comment has been minimized.

Copy link
Member

commented Jul 11, 2019

Some (member-level) improvements can be backward deployable, but any use of newly added types will need availability checks (or a minimum deployment target that matches the type's availability).

The standard library that shipped in macOS 10.14.4 does not include a CollectionDifference type, so this functionality won't be available there.

@jrose-apple

This comment has been minimized.

Copy link
Member

commented Jul 11, 2019

@tonyarnold

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

Ah, that's a shame - I guess the cases where I'd taken advantage of this in the past were possible due to flexibility prior to ABI/module stabilisation.

Ok, thanks for taking the time to explain it to me @lorentey and @jrose-apple!

@palimondo

This comment has been minimized.

Copy link
Collaborator

commented Jul 12, 2019

What about Linux? Does this not exist there?

@lorentey

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

Linux and all other platforms exist in the fallback *.

We only have binary stability on Apple platforms in this release; on other platforms, Swift binaries need to be deployed with their own dedicated copy of the Swift runtime, like iOS/macOS apps did before this year. @available only matters if the code may run on multiple versions of the stdlib, and that is only possible with a stable ABI.

Because Swift binaries on Linux can only ever run on the particular stdlib version they were compiled for, availability concerns don't apply there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.