[stdlib] Set, Dictionary: Resilient Cocoa indices, vol 2 #19612

lorentey · 2018-09-28T20:02:02Z

Fix effects annotations
Replace typed storage reference with two BridgeObjects. This covers all future variants I can think of.

…ashing

Replace the single typed storage reference with two _BridgeObjects; this should give us plenty of space to play around with alternative representations.

lorentey · 2018-09-28T20:02:51Z

@swift-ci please test

jrose-apple · 2018-09-28T20:50:17Z

stdlib/public/core/SetBridging.swift

@@ -412,17 +412,34 @@ extension _CocoaSet {
  @usableFromInline
  internal struct Index {
    @usableFromInline
-    internal var storage: Storage
+    internal var _object: Builtin.BridgeObject
+    @usableFromInline


I don't think these need to be usableFromInline anymore, do they?

(Though if you do remove it, please do it in a follow-up. We need to get those optimize-test bots back sooner rather than later.)

swift-ci · 2018-09-28T21:04:17Z

Build failed
Swift Test OS X Platform
Git Sha - 15f4b91

lorentey · 2018-09-28T23:10:55Z

[ RUN      ] Dictionary.BridgedFromObjC.Verbatim.SubscriptWithIndex
stderr>>> Fatal error: file /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/swift/stdlib/public/core/DictionaryBridging.swift, line 491
stderr>>> Current stack trace:
stderr>>> 0    libswiftCore.dylib                 0x0000000106a3a4a0 _swift_stdlib_reportFatalErrorInFile + 116
stderr>>> 1    libswiftCore.dylib                 0x00000001069424f0 specialized closure #1 in closure #1 in closure #1 in _fatalErrorMessage(_:_:file:line:flags:) + 306
stderr>>> 2    libswiftCore.dylib                 0x00000001068bc230 specialized closure #1 in closure #1 in _fatalErrorMessage(_:_:file:line:flags:) + 113
stderr>>> 3    libswiftCore.dylib                 0x00000001068bc5f0 specialized closure #1 in _fatalErrorMessage(_:_:file:line:flags:) + 99
stderr>>> 4    libswiftCore.dylib                 0x00000001068bc990 specialized _fatalErrorMessage(_:_:file:line:flags:) + 558
stderr>>> 5    libswiftCore.dylib                 0x00000001067313c0 _CocoaDictionary.formIndex(after:) + 377
stderr>>> 6    libswiftCore.dylib                 0x0000000106713330 Dictionary.index(after:) + 124
stderr>>> 7    libswiftCore.dylib                 0x0000000106720860 protocol witness for Collection.index(after:) in conformance [A : B] + 52
stderr>>> 8    libswiftCore.dylib                 0x00000001067146e0 protocol witness for Collection.index(after:) in conformance [A : B] + 16
stderr>>> 9    libswiftCore.dylib                 0x0000000106645e40 Collection.formIndex(after:) + 81
stderr>>> 10   libswiftCore.dylib                 0x00000001067208b0 protocol witness for Collection.formIndex(after:) in conformance [A : B].Keys + 9
stderr>>> 11   libswiftCore.dylib                 0x00000001069d2b80 protocol witness for Collection.formIndex(after:) in conformance [A : B] + 9
stderr>>> 12   libswiftCore.dylib                 0x0000000106790540 DefaultIndices.formIndex(after:) + 21
stderr>>> 13   libswiftCore.dylib                 0x00000001067906e0 protocol witness for Collection.formIndex(after:) in conformance DefaultIndices<A> + 9
stderr>>> 14   libswiftCore.dylib                 0x0000000106624040 IndexingIterator.next() + 353
stderr>>> 15   Dictionary                         0x00000001065773f0 closure #53 in  + 3014
stderr>>> 16   libswiftStdlibUnittest.dylib       0x00000001075825b0 TestSuite._runTest(name:parameter:) + 443
stderr>>> 17   libswiftStdlibUnittest.dylib       0x0000000107580e50 _childProcess() + 1400
stderr>>> 18   libswiftStdlibUnittest.dylib       0x000000010758c040 runAllTests() + 822
stderr>>> 19   Dictionary                         0x0000000106529a60 main + 42423
stderr>>> 20   libdyld.dylib                      0x00007fff73bd0014 start + 1
stderr>>> CRASHED: SIGILL
the test crashed unexpectedly
[     FAIL ] Dictionary.BridgedFromObjC.Verbatim.SubscriptWithIndex

Good, now it's failing on macOS, too. Let's see if I can find out why.

lorentey · 2018-09-28T23:14:53Z

And this is an unoptimized test, too.

lorentey · 2018-09-29T01:08:21Z

stdlib/public/core/DictionaryBridging.swift

    return _isUnique_native(&storage)
  }

  @usableFromInline
-  internal mutating func copy() -> Storage {
+  internal mutating func copy() -> _CocoaDictionary.Index {


Should not be mutating, nor @usableFromInline

lorentey · 2018-09-29T01:10:06Z

stdlib/public/core/DictionaryBridging.swift

+    guard _isNativePointer(_storage) else {
+      return false
+    }
+    unowned(unsafe) var storage = _bridgeObject(toNative: _storage)
    return _isUnique_native(&storage)


This is quite broken; the result of Index._asCocoa is never unique, so checking uniqueness here will never succeed. This needs to be moved to the top-level index.

lorentey · 2018-09-29T01:12:04Z

stdlib/public/core/SetBridging.swift

@@ -362,7 +362,7 @@ extension _CocoaSet: _SetBuffer {
  internal func formIndex(after index: inout Index) {


formIndex(after:) is not exposed in Dictionary nor Set; only the index(after:) variants ever get called.

And the @_effects attribute obviously doesn't apply on it.

lorentey · 2018-09-29T01:31:55Z

@swift-ci test

lorentey · 2018-09-29T01:32:12Z

@swift-ci please smoke benchmark

lorentey · 2018-09-29T01:33:43Z

Latest commits fix issues uncovered in tests, including the optimized tests regression.

swift-ci · 2018-09-29T02:25:32Z

Build comment file:

Performance: -O

TEST	OLD	NEW	DELTA	RATIO
Regression
IterateData	1562	1708	+9.3%	0.91x
CStringLongAscii	3297	3562	+8.0%	0.93x

Performance: -Osize

TEST	OLD	NEW	DELTA	RATIO
Regression
IterateData	1635	1819	+11.3%	0.90x (?)
CStringLongAscii	3293	3549	+7.8%	0.93x
Improvement
FloatingPointPrinting_Float_description_uniform	5517	5140	-6.8%	1.07x

Performance: -Onone

TEST	OLD	NEW	DELTA	RATIO
Regression
WordCountUniqueUTF16	9516	11629	+22.2%	0.82x
CountAlgoString	6148	7107	+15.6%	0.87x
Improvement
ArrayOfPOD	846	758	-10.4%	1.12x

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: 12-Core Intel Xeon E5
  Processor Speed: 2.7 GHz
  Number of Processors: 1
  Total Number of Cores: 12
  L2 Cache (per Core): 256 KB
  L3 Cache: 30 MB
  Memory: 64 GB

lorentey added 2 commits September 28, 2018 20:15

[stdlib] Set, Dictionary: Remove inappropriate @_effects from Index h…

41bedbe

…ashing

[stdlib] Set, Dictionary: Hide Cocoa index storage classes

15f4b91

Replace the single typed storage reference with two _BridgeObjects; this should give us plenty of space to play around with alternative representations.

lorentey requested review from jrose-apple and milseman September 28, 2018 20:02

jrose-apple reviewed Sep 28, 2018

View reviewed changes

lorentey commented Sep 29, 2018

View reviewed changes

lorentey added 3 commits September 29, 2018 02:29

[stdlib] Set, Dictionary: Fix uniqueness checking for Cocoa indices

526162f

[stdlib] Implement formIndex(after:) in all hashed collections

1529d37

[stdlib] Set, Dictionary: Remove bad @_effects attribute

df73981

lorentey merged commit 261aca3 into swiftlang:master Sep 29, 2018

lorentey deleted the opaque-cocoa-indices branch September 30, 2018 11:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[stdlib] Set, Dictionary: Resilient Cocoa indices, vol 2 #19612

[stdlib] Set, Dictionary: Resilient Cocoa indices, vol 2 #19612

Uh oh!

lorentey commented Sep 28, 2018

Uh oh!

lorentey commented Sep 28, 2018

Uh oh!

jrose-apple Sep 28, 2018

Uh oh!

swift-ci commented Sep 28, 2018

Uh oh!

lorentey commented Sep 28, 2018

Uh oh!

lorentey commented Sep 28, 2018

Uh oh!

lorentey Sep 29, 2018 •

edited

Loading

Uh oh!

lorentey Sep 29, 2018

Uh oh!

lorentey Sep 29, 2018

Uh oh!

lorentey Sep 29, 2018

Uh oh!

lorentey commented Sep 29, 2018

Uh oh!

lorentey commented Sep 29, 2018

Uh oh!

lorentey commented Sep 29, 2018

Uh oh!

swift-ci commented Sep 29, 2018

Uh oh!

Uh oh!

		@@ -362,7 +362,7 @@ extension _CocoaSet: _SetBuffer {
		internal func formIndex(after index: inout Index) {

[stdlib] Set, Dictionary: Resilient Cocoa indices, vol 2 #19612

[stdlib] Set, Dictionary: Resilient Cocoa indices, vol 2 #19612

Uh oh!

Conversation

lorentey commented Sep 28, 2018

Uh oh!

lorentey commented Sep 28, 2018

Uh oh!

jrose-apple Sep 28, 2018

Choose a reason for hiding this comment

Uh oh!

swift-ci commented Sep 28, 2018

Uh oh!

lorentey commented Sep 28, 2018

Uh oh!

lorentey commented Sep 28, 2018

Uh oh!

lorentey Sep 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lorentey Sep 29, 2018

Choose a reason for hiding this comment

Uh oh!

lorentey Sep 29, 2018

Choose a reason for hiding this comment

Uh oh!

lorentey Sep 29, 2018

Choose a reason for hiding this comment

Uh oh!

lorentey commented Sep 29, 2018

Uh oh!

lorentey commented Sep 29, 2018

Uh oh!

lorentey commented Sep 29, 2018

Uh oh!

swift-ci commented Sep 29, 2018

Build comment file:

Performance: -O

Performance: -Osize

Performance: -Onone

Uh oh!

Uh oh!

lorentey Sep 29, 2018 •

edited

Loading