Associated type resilience #19266

DougGregor · 2018-09-12T06:11:35Z

Make associated types resilient against reordering, without depending on the "sorting" hack that we previously had in place. This is setting the stage to be able to resiliently add new, defaulted associated types without breaking backward compatibility.

slavapestov · 2018-09-12T06:29:15Z

docs/ABI/Mangling.rst

@@ -138,6 +138,10 @@ types where the metadata itself has unknown layout.)
  global ::= type generic-signature 'TH' // key path equality
  global ::= type generic-signature 'Th' // key path hasher

+  global ::= protocol 'TL'               // protocol requirements base descriptor
+  global ::= assoc-type-name 'Tl'        // associated type descriptor


Why not use the same Tq suffix we use for method descriptors? There won't be a conflict and you'll be able to reuse l for other things

What about conformances? Are we just going to say you can't add new conformance requirements after the fact, and existing conformances have a canonical ordering?

I didn't re-use Tq because IIUC the mangling grammar is usually not resolved this way: the suffix always (?) tells you what grammar to expect before it. Now, I am stealing several of the thunk mangling (for the requirements base, associated type, and possibly associated conformance descriptors), so perhaps I should move out to three-letter discriminators because there aren't that many of these.

Associated conformances are interesting. We need to be able to add new conformances for new defaulted associated types, but we can't add any to existing associated types. Introducing associated conformance descriptors would be the simplest way of ensuring that the addition of new requirements doesn't affect existing clients, so I was planning to investigate that next.

If new conformances are only ever added to new associated types, you might be able to get away with not giving them their own descriptors no?

If we can find a way to consistently put them at the end, we wouldn't need descriptors for associated conformances. But that gets back to the "ABI history database" issue.

slavapestov · 2018-09-12T06:32:03Z

lib/IRGen/IRGenMangler.h

@@ -16,6 +16,7 @@
 #include "IRGenModule.h"
 #include "swift/AST/ASTMangler.h"
 #include "swift/IRGen/ValueWitness.h"
+#include "llvm/Support/SaveAndRestore.h"


Is this needed?

Yes, it's used so we don't drop the protocol name in the mangling.

Hmm. I could mangle this as and let it drop the associated type names from the rest of the mangling.

DougGregor · 2018-09-12T18:46:07Z

@swift-ci please smoke test

slavapestov · 2018-09-13T03:29:22Z

Can you also add a test to validation-test/Evolution/?

DougGregor · 2018-09-13T16:13:25Z

Can you also add a test to validation-test/Evolution/?

It turns out that we're already testing for reordering of associated types in https://github.com/apple/swift/blob/master/validation-test/Evolution/Inputs/protocol_reorder_requirements.swift#L29-L53; it worked before my pull request because we sorted associated types. Now we don't sort them, so it's working "for real".

I have not implemented resiliently adding an associated type (yet). That will need a new test.

DougGregor · 2018-09-13T21:22:48Z

@swift-ci please test

DougGregor · 2018-09-13T21:22:58Z

@swift-ci please benchmark

DougGregor · 2018-09-13T21:23:08Z

@swift-ci please test source compatibility

swift-ci · 2018-09-13T21:24:52Z

Build failed
Swift Test Linux Platform
Git Sha - 2584b287beedf70ff7ca40f60466300f66e35de1

swift-ci · 2018-09-13T21:26:18Z

Build failed
Swift Test OS X Platform
Git Sha - 2584b287beedf70ff7ca40f60466300f66e35de1

DougGregor · 2018-09-13T21:40:06Z

@swift-ci please benchmark

DougGregor · 2018-09-13T21:40:48Z

@swift-ci please test macOS

swift-ci · 2018-09-13T22:15:26Z

Build comment file:

Performance: -O

TEST	OLD	NEW	DELTA	SPEEDUP
Regression
IterateData	1406	1547	+10.0%	0.91x
UTF8Decode_InitFromBytes	1084	1177	+8.6%	0.92x (?)

Code size: -O

TEST	OLD	NEW	DELTA	SPEEDUP
Regression
DictOfArraysToArrayOfDicts.o	35230	35822	+1.7%	0.98x

Performance: -Osize

TEST	OLD	NEW	DELTA	SPEEDUP
Regression
IterateData	1464	1622	+10.8%	0.90x

Code size: -Osize

TEST	OLD	NEW	DELTA	SPEEDUP
Regression
DictOfArraysToArrayOfDicts.o	34854	35446	+1.7%	0.98x
NibbleSort.o	19599	19831	+1.2%	0.99x

Code size: Swift libraries

TEST	OLD	NEW	DELTA	SPEEDUP
Regression
libswiftNetwork.dylib	167936	172032	+2.4%	0.98x
libswiftsimd.dylib	286720	290816	+1.4%	0.99x

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: 16 GB

DougGregor · 2018-09-13T22:52:59Z

Small code size regression, as expected.

swift-ci · 2018-09-13T23:16:53Z

Build failed
Swift Test OS X Platform
Git Sha - 6527d76498f7a0587f4214cba5cef410280b4acb

Emit associated type descriptors (as aliases) to reference each associated type requirement within a resilient protocol.

Introduce an alias that refers one element prior to the start of a protocol descriptor’s protocol requirements. This can be subtracted from an associated type descriptor address to determine the offset of the associated type accessor within a corresponding witness table. The code generation for the latter is not yet implemented.

…able.

When accessing the associated type witness metadata for a resilient protocol, compute the index based on the difference between the associated type’s descriptor and the protocol requirement base descriptor to determine the offset into the witness table.

The signature of this function pointer was incorrect, because it had not yet been updated for the metadata request/response architecture. We were getting lucky (!).

Emit associated type witnesses into resilient conformance tables, so they can be re-ordered within the protocol without breaking clients. This will (eventually) permit adding new, defaulted associated types to protocols resiliently.

…ors. When we’re creating an associated type witness metadata accessor for resilience reasons, but the associated type witness doesn’t involve any type parameters, directly form the type metadata reference (and don’t cache it). While here… update all of the IRGen/SILGen test cases perturbed by the introduction of resilient associated type access patterns.

Generic parameter references, which occur in generic requirement metadata, were hardcoding associated type indices. Instead, use relative references to associated type descriptors and perform the index calculation at runtime. Associated types can now be reordered resiliently (without relying on sorting), which is the first main step toward rdar://problem/44167982.

DougGregor · 2018-09-15T05:30:45Z

@swift-ci please test

DougGregor · 2018-09-15T05:31:08Z

@swift-ci please test

DougGregor · 2018-09-15T05:33:31Z

@swift-ci please test macOS

DougGregor · 2018-09-15T05:35:10Z

@swift-ci please test macOS

DougGregor · 2018-09-15T05:35:30Z

@swift-ci please test macOS

DougGregor · 2018-09-15T06:06:54Z

@swift-ci please test macOS

swift-ci · 2018-09-15T06:25:28Z

Build failed
Swift Test Linux Platform
Git Sha - 0fa1590

swift-ci · 2018-09-15T06:56:19Z

Build failed
Swift Test OS X Platform
Git Sha - 6527d76498f7a0587f4214cba5cef410280b4acb

DougGregor · 2018-09-15T17:18:01Z

@swift-ci please test

swift-ci · 2018-09-15T17:18:26Z

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

swift-ci · 2018-09-15T17:19:44Z

Build failed
Swift Test Linux Platform
Git Sha - 0fa1590

DougGregor changed the title ~~Associated type resilience~~ [WIP] Associated type resilience Sep 12, 2018

slavapestov reviewed Sep 12, 2018

View reviewed changes

DougGregor force-pushed the associated-type-resilience branch from 2584b28 to dca37eb Compare September 12, 2018 21:01

DougGregor changed the title ~~[WIP] Associated type resilience~~ Associated type resilience Sep 13, 2018

DougGregor added 10 commits September 14, 2018 20:59

[ABI] Emit associated type descriptors referencing each requirement.

9873d52

Emit associated type descriptors (as aliases) to reference each associated type requirement within a resilient protocol.

[IRGen] witness_method should always refer to a slot in the witness t…

e77cba7

…able.

[Runtime] Fix _ObjectiveCBridgeableWitnessTable::ObjectiveCType

50d12e0

The signature of this function pointer was incorrect, because it had not yet been updated for the metadata request/response architecture. We were getting lucky (!).

[ABI] Emit associated type witnesses resiliently.

cab6bfa

Emit associated type witnesses into resilient conformance tables, so they can be re-ordered within the protocol without breaking clients. This will (eventually) permit adding new, defaulted associated types to protocols resiliently.

Adjust test for Linux

fcc3ffe

[IRGen] Fix associated type resilience for 32-bit platforms.

0fa1590

DougGregor force-pushed the associated-type-resilience branch from 6527d76 to 0fa1590 Compare September 15, 2018 05:30

Another test fix for Linux

dd77314

DougGregor merged commit 78f3017 into swiftlang:master Sep 16, 2018

DougGregor deleted the associated-type-resilience branch September 16, 2018 04:03

Associated type resilience #19266

Associated type resilience #19266

Uh oh!

Conversation

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

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DougGregor commented Sep 12, 2018

Uh oh!

slavapestov commented Sep 13, 2018

Uh oh!

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

Uh oh!

Uh oh!

DougGregor commented Sep 13, 2018

Uh oh!

DougGregor commented Sep 13, 2018

Uh oh!

DougGregor commented Sep 13, 2018

Uh oh!

swift-ci commented Sep 13, 2018

Uh oh!

swift-ci commented Sep 13, 2018

Uh oh!

DougGregor commented Sep 13, 2018

Uh oh!

DougGregor commented Sep 13, 2018

Uh oh!

swift-ci commented Sep 13, 2018

Build comment file:

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Code size: Swift libraries

Uh oh!

DougGregor commented Sep 13, 2018

Uh oh!

swift-ci commented Sep 13, 2018

Uh oh!

DougGregor commented Sep 15, 2018

Uh oh!

DougGregor commented Sep 15, 2018

Uh oh!

DougGregor commented Sep 15, 2018

Uh oh!

DougGregor commented Sep 15, 2018

Uh oh!

DougGregor commented Sep 15, 2018

Uh oh!

DougGregor commented Sep 15, 2018

Uh oh!

swift-ci commented Sep 15, 2018

Uh oh!

swift-ci commented Sep 15, 2018

Uh oh!

DougGregor commented Sep 15, 2018

Uh oh!

swift-ci commented Sep 15, 2018

Uh oh!

swift-ci commented Sep 15, 2018

Uh oh!

Uh oh!

DougGregor commented Sep 12, 2018 •

edited

Loading

DougGregor commented Sep 13, 2018 •

edited

Loading