[stdlib] Deprecate MutableCollection._withUnsafeMutableBufferPointerIfSupported #36003

lorentey · 2021-02-16T22:06:16Z

In Swift 5.0, SE-0237 introduced the public MutableCollection.withContiguousMutableStorageIfAvailable method. It’s time we migrated off the old, underscored variant and deprecated it.

The default MutableCollection.sort and .partition(by:) implementations are currently calling this hidden method rather than the documented interface, preventing custom Collection implementations from achieving good performance, even if they have contiguous storage.

…fSupported In Swift 5.0, [SE-0237] introduced the public `MutableCollection.withContiguousMutableStorageIfAvailable` method. It’s time we migrated off the old, underscored variant and deprecated it. The default `MutableCollection.sort` and `.partition(by:)` implementations are currently calling this hidden method rather than the documented interface, preventing custom Collection implementations from achieving good performance, even if they have contiguous storage. [SE-0237]: https://github.com/apple/swift-evolution/blob/master/proposals/0237-contiguous-collection.md

lorentey · 2021-02-16T22:06:26Z

@swift-ci test

lorentey · 2021-02-16T22:06:33Z

@swift-ci benchmark

swift-ci · 2021-02-16T22:27:14Z

Build failed
Swift Test Linux Platform
Git Sha - ba3c403

lorentey · 2021-02-16T22:31:33Z

Oh, nice, we have test coverage! ❤️

[ RUN      ] ArraySliceWithNonZeroStartIndex_MutableRandomAccessCollectionRef.ArraySliceWithNonZeroStartIndex.ArraySlice<LifetimeTracked>.Type.partition/DispatchesThrough_withUnsafeMutableBufferPointerIfSupported
stdout>>> check failed at /home/buildnode/jenkins/workspace/swift-PR-Linux/branch-main/swift/stdlib/private/StdlibCollectionUnittest/CheckMutableCollectionType.swift, line 968
stdout>>> expected: 1 (of type Swift.Int)
stdout>>> actual: 0 (of type Swift.Int)
stdout>>> check failed at /home/buildnode/jenkins/workspace/swift-PR-Linux/branch-main/swift/stdlib/private/StdlibCollectionUnittest/CheckMutableCollectionType.swift, line 970
stdout>>> expected: 1 (of type Swift.Int)
stdout>>> actual: 0 (of type Swift.Int)
[     FAIL ] ArraySliceWithNonZeroStartIndex_MutableRandomAccessCollectionRef.ArraySliceWithNonZeroStartIndex.ArraySlice<LifetimeTracked>.Type.partition/DispatchesThrough_withUnsafeMutableBufferPointerIfSupported

lorentey · 2021-02-16T22:58:39Z

@swift-ci test

lorentey · 2021-02-16T22:58:48Z

@swift-ci benchmark

swift-ci · 2021-02-17T00:37:18Z

Build failed
Swift Test Linux Platform
Git Sha - 1ebbfabe7c48c866262cb6e59e420552501f017c

swift-ci · 2021-02-17T02:05:17Z

Build failed
Swift Test OS X Platform
Git Sha - 1ebbfabe7c48c866262cb6e59e420552501f017c

swift-ci · 2021-02-17T02:13:54Z

Performance: -O

Regression	OLD	NEW	DELTA	RATIO
FlattenListLoop	1453	2185	+50.4%	0.66x (?)
FlattenListFlatMap	4277	5233	+22.4%	0.82x (?)

Code size: -O

Performance: -Osize

Regression	OLD	NEW	DELTA	RATIO
FlattenListFlatMap	5215	5811	+11.4%	0.90x (?)

Code size: -Osize

Performance: -Onone

Regression	OLD	NEW	DELTA	RATIO
DictionaryBridgeToObjC_Access	882	996	+12.9%	0.89x (?)

Code size: -swiftlibs

How to read the data

The 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
regressions before you merge the PR.

Noise: Sometimes the performance results (not code size!) contain false
alarms. Unexpected regressions which are marked with '(?)' are probably noise.
If you see regressions which you cannot explain you can try to run the
benchmarks again. If regressions still show up, please consult with the
performance team (@eeckstein).

Hardware Overview

  Model Name: Mac Pro
  Model Identifier: MacPro6,1
  Processor Name: 8-Core Intel Xeon E5
  Processor Speed: 3 GHz
  Number of Processors: 1
  Total Number of Cores: 8
  L2 Cache (per Core): 256 KB
  L3 Cache: 25 MB
  Memory: 64 GB

lorentey · 2021-02-17T04:38:38Z

16:33:26 
/home/buildnode/jenkins/workspace/swift-PR-Linux/branch-main/swift/stdlib/private/StdlibCollectionUnittest/CheckMutableCollectionType.swift:517:1: 
error: expressions are not allowed at the top level

lorentey · 2021-02-17T04:39:40Z

@swift-ci test

swift-ci · 2021-02-17T04:48:57Z

Build failed
Swift Test Linux Platform
Git Sha - b50db27

swift-ci · 2021-02-17T08:20:48Z

Build failed
Swift Test OS X Platform
Git Sha - b50db27

stdlib/private/StdlibCollectionUnittest/CheckMutableCollectionType.swift

natecook1000

This is great! 👏

…Type.swift Co-authored-by: Nate Cook <[email protected]>

Co-authored-by: Nate Cook <[email protected]>

lorentey · 2021-02-18T02:59:04Z

@swift-ci test

swift-ci · 2021-02-18T03:29:27Z

Build failed
Swift Test Linux Platform
Git Sha - 379a27d

swift-ci · 2021-02-18T05:14:26Z

Build failed
Swift Test OS X Platform
Git Sha - 379a27d

lorentey · 2021-02-24T05:17:05Z

@swift-ci test

swift-ci · 2021-02-24T06:35:55Z

Build failed
Swift Test Linux Platform
Git Sha - aadca9c

lorentey · 2021-02-24T06:53:56Z

'WorkspaceTests.testArtifactDownloadHappyPath' started at 2021-02-24 06:35:06.861
/home/buildnode/jenkins/workspace/swift-PR-Linux/branch-main/swiftpm/Tests/WorkspaceTests/WorkspaceTests.swift:4402: error: WorkspaceTests.testArtifactDownloadHappyPath : XCTAssertEqual failed: ("[<AbsolutePath:"/tmp/ws/.build/artifacts/A">, <AbsolutePath:"/tmp/ws/.build/artifacts/B">]") is not equal to ("[<AbsolutePath:"/tmp/ws/.build/artifacts/A">, <AbsolutePath:"/tmp/ws/.build/artifacts/A">, <AbsolutePath:"/tmp/ws/.build/artifacts/B">]")

Hm.

lorentey · 2021-02-24T20:57:48Z

@swift-ci test linux platform

lorentey · 2021-02-26T08:19:56Z

@swift-ci test

lorentey requested review from natecook1000 and airspeedswift February 16, 2021 22:06

[test] Update tests for stdlib behavior changes

b50db27

lorentey force-pushed the deprecate-underscored-storage-access branch from 1ebbfab to b50db27 Compare February 17, 2021 04:39

natecook1000 reviewed Feb 17, 2021

View reviewed changes

stdlib/private/StdlibCollectionUnittest/CheckMutableCollectionType.swift Outdated Show resolved Hide resolved

natecook1000 reviewed Feb 17, 2021

View reviewed changes

stdlib/private/StdlibCollectionUnittest/CheckMutableCollectionType.swift Outdated Show resolved Hide resolved

natecook1000 reviewed Feb 17, 2021

View reviewed changes

stdlib/private/StdlibCollectionUnittest/CheckMutableCollectionType.swift Outdated Show resolved Hide resolved

natecook1000 reviewed Feb 17, 2021

View reviewed changes

stdlib/private/StdlibCollectionUnittest/CheckMutableCollectionType.swift Outdated Show resolved Hide resolved

natecook1000 approved these changes Feb 17, 2021

View reviewed changes

lorentey and others added 3 commits February 17, 2021 14:42

Update stdlib/private/StdlibCollectionUnittest/CheckMutableCollection…

9aa57cd

…Type.swift Co-authored-by: Nate Cook <[email protected]>

Update stdlib/private/StdlibCollectionUnittest/CheckMutableCollection…

04ce0ab

…Type.swift Co-authored-by: Nate Cook <[email protected]>

Apply suggestions from code review

379a27d

Co-authored-by: Nate Cook <[email protected]>

[test] LoggingMutableCollection: Fix logging targets

aadca9c

[stdlib] Fix warning by restoring original workaround

1b53d26

lorentey merged commit 0836707 into swiftlang:main Feb 26, 2021

[stdlib] Deprecate MutableCollection._withUnsafeMutableBufferPointerIfSupported #36003

[stdlib] Deprecate MutableCollection._withUnsafeMutableBufferPointerIfSupported #36003

Uh oh!

Conversation

lorentey commented Feb 16, 2021

Uh oh!

lorentey commented Feb 16, 2021

Uh oh!

lorentey commented Feb 16, 2021

Uh oh!

swift-ci commented Feb 16, 2021

Uh oh!

lorentey commented Feb 16, 2021

Uh oh!

lorentey commented Feb 16, 2021

Uh oh!

lorentey commented Feb 16, 2021

Uh oh!

swift-ci commented Feb 17, 2021

Uh oh!

swift-ci commented Feb 17, 2021

Uh oh!

swift-ci commented Feb 17, 2021

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

Code size: -swiftlibs

Uh oh!

lorentey commented Feb 17, 2021

Uh oh!

lorentey commented Feb 17, 2021

Uh oh!

swift-ci commented Feb 17, 2021

Uh oh!

swift-ci commented Feb 17, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

natecook1000 left a comment

Choose a reason for hiding this comment

Uh oh!

lorentey commented Feb 18, 2021

Uh oh!

swift-ci commented Feb 18, 2021

Uh oh!

swift-ci commented Feb 18, 2021

Uh oh!

lorentey commented Feb 24, 2021

Uh oh!

swift-ci commented Feb 24, 2021

Uh oh!

lorentey commented Feb 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lorentey commented Feb 24, 2021

Uh oh!

lorentey commented Feb 26, 2021

Uh oh!

Uh oh!

lorentey commented Feb 24, 2021 •

edited

Loading