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

Overhaul swift_dynamicCast #29658

Closed
wants to merge 10 commits into from
Closed

Conversation

tbkka
Copy link
Contributor

@tbkka tbkka commented Feb 5, 2020

Background

swift_dynamicCast() is the runtime support behind the as?, as!, and is
operators. It attempts to convert a source value with a given type into a
destination location with another specified type.

The previous code had evolved over several years into a tangle of different
strategies for different combinations of source and destination types, with a
growing set of inconsistencies between them. The new implementation has a
unified structure that handles common issues with common code to help ensure
consistent behavior.

Design

The new implementation has a main driver called tryCast() that recursively
attempts to find a suitable conversion from source to destination. For
example, if the source is an Any container, tryCast will extract and
recursively try to convert the contents of that container. At each step,
the tryCast will call a function tryCastToXyz that understands specific
requirements for a single target type.

Suppose, for example, you have an Any containing an Optional<Int> and
you want to cast that to a FixedWidthInteger existential:

  • tryCast() will ask tryCastToOpaqueExistential if it can convert the Any
    directly. This will fail because Any does not directly conform to any
    protocols.

  • tryCast() will extract the Optional<Int> from the Any and try again.

  • tryCast() will then try to unwrap the optional. If the optional is nil,
    tryCast will try to initialize the existential with nil and fail.

  • Finally, tryCast() will ask tryCastToOpaqueExistential to convert an Int
    to the desired FixedWidthInteger existential. This will succeed and the
    result is returned.

In addition to unwrapping boxed values, tryCast() is also responsible for
other general conversion strategies: using the dynamic type instead of
the static type; invoking Objective-C bridging machinery; packing values
into a __SwiftValue container.

Perhaps most importantly, this PR adds a lot of new tests, including some
"expected failure" tests for issues that aren't (yet) actually fixed.

Resolves SR-1999
Resolves SR-2289 (rdar://27116100)
Resolves SR-4552 (rdar://59174750)
Resolves SR-6126
Resolves SR-8964 (rdar://45217461)
Resolves rdar://58650899

This implementation also does a better job with existential metatypes
and CoreFoundation types.

Next Steps

This PR only changes the runtime dynamic cast code. The Swift compiler
aggressively optimizes casts in release builds, so these changes will
not impact many problems with casting in release builds. I hope to
work on that very soon. In the meantime, you can work around many
compiler problems by calling the following function, which forces
the cast to be handled by the runtime code:

func runtimeCast<T,U>(_ value: T, to: U.Type) -> U? {
  return value as? U
}

// Might use runtime logic or not, depending on compiler optimizations
let b = a as? Foo

// Always uses runtime cast logic
let c = runtimeCast(a, to: Foo.self)

I've not specifically looked at performance. Hopefully, the
new design will make it easier to speed up cast performance in
a future PR.

Caveats

Many casts now succeed that used to fail. Code that relied on certain
casts failing may be surprised. I found and changed one such place
in the standard library.

Different error messages. When a force-cast (as!) fails, the message
may be different, due to the different way in which the new code
tracks and reports failures. I've also taken this opportunity to detect
and issue a specialized error if the cast logic detects an unexpected
null pointer (this can happen when ill-behaved C or Obj-C code returns an
unexpected null pointer).

@tbkka
Copy link
Contributor Author

tbkka commented Feb 5, 2020

WIP: I still have a couple of failing tests on Linux that I need to resolve.

@tbkka
Copy link
Contributor Author

tbkka commented Feb 5, 2020

@swift-ci Please benchmark

@gottesmm
Copy link
Member

gottesmm commented Feb 5, 2020

@swift-ci test windows platform

@swift-ci
Copy link
Collaborator

swift-ci commented Feb 5, 2020

Performance: -O

Regression OLD NEW DELTA RATIO
ObjectiveCBridgeFromNSSetAnyObjectForced 2900 26440 +811.7% 0.11x
ObjectiveCBridgeFromNSDictionaryAnyObjectForced 4300 28050 +552.3% 0.15x
PrefixAnySeqCRangeIterLazy 14 25 +78.6% 0.56x (?)
ObjectiveCBridgeFromNSDictionaryAnyObject 24400 27300 +11.9% 0.89x
ObjectiveCBridgeFromNSArrayAnyObject 14400 16000 +11.1% 0.90x (?)
ObjectiveCBridgeFromNSStringForced 1705 1890 +10.9% 0.90x (?)
ObjectiveCBridgeFromNSSetAnyObjectToString 42500 47000 +10.6% 0.90x (?)
ObjectiveCBridgeStubFromArrayOfNSString2 2040 2200 +7.8% 0.93x (?)
 
Improvement OLD NEW DELTA RATIO
DictionaryOfAnyHashableStrings_lookup 2688 2448 -8.9% 1.10x (?)
ObjectiveCBridgeFromNSSetAnyObjectToStringForced 53000 48500 -8.5% 1.09x (?)
DictionaryOfAnyHashableStrings_insert 3318 3038 -8.4% 1.09x (?)

Code size: -O

Performance: -Osize

Regression OLD NEW DELTA RATIO
ObjectiveCBridgeFromNSSetAnyObjectForced 2860 25960 +807.7% 0.11x
ObjectiveCBridgeFromNSDictionaryAnyObjectForced 4200 27950 +565.5% 0.15x
ObjectiveCBridgeFromNSDictionaryAnyObject 23600 26500 +12.3% 0.89x (?)
ObjectiveCBridgeFromNSStringForced 1670 1850 +10.8% 0.90x (?)
ObjectiveCBridgeFromNSArrayAnyObject 14100 15600 +10.6% 0.90x (?)
ObjectiveCBridgeFromNSSetAnyObjectToString 42500 46000 +8.2% 0.92x (?)
 
Improvement OLD NEW DELTA RATIO
ObjectiveCBridgeFromNSDictionaryAnyObjectToStringForced 52000 47500 -8.7% 1.09x (?)
ObjectiveCBridgeFromNSSetAnyObjectToStringForced 52000 47500 -8.7% 1.09x (?)
FlattenListFlatMap 3155 2893 -8.3% 1.09x (?)

Code size: -Osize

Performance: -Onone

Regression OLD NEW DELTA RATIO
ObjectiveCBridgeFromNSSetAnyObjectForced 3160 26300 +732.3% 0.12x
ObjectiveCBridgeFromNSDictionaryAnyObjectForced 5250 28300 +439.0% 0.19x
ObjectiveCBridgeFromNSArrayAnyObjectForced 5140 16900 +228.8% 0.30x
ObjectiveCBridgeFromNSStringForced 1745 1980 +13.5% 0.88x (?)
ObjectiveCBridgeFromNSArrayAnyObject 15200 17100 +12.5% 0.89x (?)
ObjectiveCBridgeFromNSString 2035 2270 +11.5% 0.90x (?)
ObjectiveCBridgeFromNSDictionaryAnyObject 26100 28700 +10.0% 0.91x (?)
 
Improvement OLD NEW DELTA RATIO
ObjectiveCBridgeFromNSSetAnyObjectToStringForced 53500 48000 -10.3% 1.11x (?)
ObjectiveCBridgeFromNSDictionaryAnyObjectToStringForced 53000 48500 -8.5% 1.09x (?)
CharacterLiteralsLarge 486 453 -6.8% 1.07x (?)

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 mini
  Model Identifier: Macmini8,1
  Processor Name: Intel Core i7
  Processor Speed: 3.2 GHz
  Number of Processors: 1
  Total Number of Cores: 6
  L2 Cache (per Core): 256 KB
  L3 Cache: 12 MB
  Memory: 64 GB

@tbkka
Copy link
Contributor Author

tbkka commented Feb 5, 2020

Hmmm.... Guess I should take a careful look at casting from Obj-C NSDictionary/NSSet before merging this.

@jckarter
Copy link
Member

jckarter commented Feb 5, 2020

It might also be worth coordinating with @Catfish-Man to see if bridging conversions can be put on a faster path in general. In particular, if they could avoid having to do dynamic conformance lookups, that'd probably be a big win.

@tbkka
Copy link
Contributor Author

tbkka commented Feb 6, 2020

@swift-ci Please benchmark

@swift-ci
Copy link
Collaborator

swift-ci commented Feb 6, 2020

Performance: -O

Regression OLD NEW DELTA RATIO
ObjectiveCBridgeFromNSArrayAnyObject 21900 25800 +17.8% 0.85x (?)
ObjectiveCBridgeFromNSArrayAnyObjectToString 44400 52300 +17.8% 0.85x (?)
ObjectiveCBridgeFromNSDictionaryAnyObjectForced 7300 8500 +16.4% 0.86x (?)
ObjectiveCBridgeFromNSDictionaryAnyObject 40800 46600 +14.2% 0.88x (?)
ObjectiveCBridgeStubFromArrayOfNSString2 3360 3760 +11.9% 0.89x (?)
StringComparison_nonBMPSlowestPrenormal 1380 1540 +11.6% 0.90x
EqualSubstringSubstring 39 43 +10.3% 0.91x
LessSubstringSubstring 39 43 +10.3% 0.91x
EqualSubstringSubstringGenericEquatable 39 43 +10.3% 0.91x
EqualSubstringString 39 43 +10.3% 0.91x
LessSubstringSubstringGenericComparable 39 43 +10.3% 0.91x
StringUTF16Builder 340 370 +8.8% 0.92x
ObjectiveCBridgeFromNSDictionaryAnyObjectToStringForced 91500 99500 +8.7% 0.92x (?)
ObjectiveCBridgeFromNSSetAnyObjectToString 78500 85000 +8.3% 0.92x (?)
StringComparison_emoji 752 812 +8.0% 0.93x
 
Improvement OLD NEW DELTA RATIO
ObjectiveCBridgeStringHash 78 67 -14.1% 1.16x
DictionaryBridge 889 810 -8.9% 1.10x (?)
ObjectiveCBridgeStubToNSDate2 1380 1260 -8.7% 1.10x (?)
DictionaryOfAnyHashableStrings_lookup 3912 3624 -7.4% 1.08x (?)
UTF8Decode_InitFromData_ascii 251 233 -7.2% 1.08x (?)
DictionaryOfAnyHashableStrings_insert 4788 4452 -7.0% 1.08x (?)
StringHashing_fastPrenormal 1000 930 -7.0% 1.08x

Code size: -O

Performance: -Osize

Regression OLD NEW DELTA RATIO
ObjectiveCBridgeFromNSArrayAnyObjectForced 4840 6120 +26.4% 0.79x (?)
ObjectiveCBridgeFromNSSetAnyObjectToString 72000 90000 +25.0% 0.80x (?)
ObjectiveCBridgeFromNSArrayAnyObjectToStringForced 42800 53400 +24.8% 0.80x (?)
ObjectiveCBridgeFromNSDictionaryAnyObjectForced 7350 9150 +24.5% 0.80x (?)
ObjectiveCBridgeFromNSArrayAnyObjectToString 43300 53400 +23.3% 0.81x (?)
ObjectiveCBridgeFromNSArrayAnyObject 22100 26300 +19.0% 0.84x (?)
ObjectiveCBridgeFromNSDictionaryAnyObjectToStringForced 91500 105000 +14.8% 0.87x (?)
ObjectiveCBridgeFromNSDictionaryAnyObject 41100 46500 +13.1% 0.88x (?)
ObjectiveCBridgeFromNSStringForced 2605 2925 +12.3% 0.89x (?)
ObjectiveCBridgeFromNSSetAnyObjectToStringForced 96500 108000 +11.9% 0.89x (?)
EqualStringSubstring 40 44 +10.0% 0.91x (?)
StringComparison_nonBMPSlowestPrenormal 1410 1540 +9.2% 0.92x (?)
SuffixAnySeqCntRangeLazy 38504 41694 +8.3% 0.92x (?)
StringComparison_emoji 756 816 +7.9% 0.93x (?)
SequenceAlgosAnySequence 23200 25000 +7.8% 0.93x (?)
SuffixAnySeqCRangeIterLazy 38662 41646 +7.7% 0.93x (?)
EqualSubstringString 39 42 +7.7% 0.93x (?)
DropLastAnySeqCRangeIterLazy 38490 41394 +7.5% 0.93x (?)
 
Improvement OLD NEW DELTA RATIO
ObjectiveCBridgeStringHash 78 67 -14.1% 1.16x

Code size: -Osize

Performance: -Onone

Regression OLD NEW DELTA RATIO
ObjectiveCBridgeFromNSArrayAnyObject 24900 29900 +20.1% 0.83x (?)
ObjectiveCBridgeFromNSArrayAnyObjectForced 9220 10940 +18.7% 0.84x (?)
ObjectiveCBridgeFromNSSetAnyObjectToString 77000 91000 +18.2% 0.85x (?)
LazilyFilteredRange 975620 1126360 +15.5% 0.87x (?)
ObjectiveCBridgeFromNSDictionaryAnyObject 45600 52500 +15.1% 0.87x (?)
SubstringComparable 2444 2799 +14.5% 0.87x (?)
ArrayAppendRepeatCol 286410 322600 +12.6% 0.89x (?)
Data.append.Sequence.809B.Count.RE 29019 32545 +12.2% 0.89x (?)
ObjectiveCBridgeStubFromArrayOfNSString2 3530 3950 +11.9% 0.89x (?)
Data.append.Sequence.64kB.Count.RE.I 23487 26274 +11.9% 0.89x (?)
DataAppendSequence 2912700 3257800 +11.8% 0.89x (?)
Data.append.Sequence.64kB.Count.RE 23478 26255 +11.8% 0.89x (?)
Data.append.Sequence.809B.Count.RE.I 29080 32505 +11.8% 0.89x (?)
Data.init.Sequence.809B.Count.RE 28946 32341 +11.7% 0.90x (?)
Data.init.Sequence.64kB.Count.RE 23457 26176 +11.6% 0.90x (?)
Data.init.Sequence.64kB.Count.RE.I 23468 26129 +11.3% 0.90x (?)
Data.append.Sequence.64kB.Count0.RE 23722 26240 +10.6% 0.90x (?)
Data.append.Sequence.64kB.Count0.RE.I 23593 26062 +10.5% 0.91x (?)
Data.append.Sequence.809B.Count0.RE 29472 32555 +10.5% 0.91x (?)
Data.append.Sequence.809B.Count0.RE.I 29320 32381 +10.4% 0.91x (?)
Data.init.Sequence.809B.Count.RE.I 29125 32157 +10.4% 0.91x (?)
NormalizedIterator_ascii 454 501 +10.4% 0.91x (?)
Data.init.Sequence.64kB.Count0.RE.I 23643 26089 +10.3% 0.91x (?)
Data.init.Sequence.809B.Count0.RE.I 29433 32411 +10.1% 0.91x (?)
Data.init.Sequence.809B.Count0.RE 29549 32492 +10.0% 0.91x (?)
Data.init.Sequence.64kB.Count0.RE 23748 26038 +9.6% 0.91x (?)
LessSubstringSubstringGenericComparable 46 50 +8.7% 0.92x
FloatingPointPrinting_Float80_description_uniform 57500 62400 +8.5% 0.92x (?)
DataCreateSmallArray 89300 96850 +8.5% 0.92x (?)
ObjectiveCBridgeFromNSSetAnyObject 41600 45000 +8.2% 0.92x (?)
ArrayOfPOD 1038 1122 +8.1% 0.93x (?)
NormalizedIterator_latin1 750 810 +8.0% 0.93x (?)
PrefixAnySeqCntRangeLazy 29124 31425 +7.9% 0.93x (?)
SequenceAlgosRange 2336640 2517660 +7.7% 0.93x (?)
 
Improvement OLD NEW DELTA RATIO
ObjectiveCBridgeStubFromNSDate 5880 5000 -15.0% 1.18x (?)
ObjectiveCBridgeStringHash 78 67 -14.1% 1.16x
FatCompactMap 474260 418900 -11.7% 1.13x (?)

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

@tbkka
Copy link
Contributor Author

tbkka commented Feb 6, 2020

I seem to have fixed the bad perf regression from the initial version. I tinkered some but didn't find any easy fixes for the remaining more minor regressions.

I believe I have fixed the last test failure on macOS, so should be ready for CI there.

@tbkka
Copy link
Contributor Author

tbkka commented Feb 6, 2020

@swift-ci Please test macOS

@swift-ci
Copy link
Collaborator

swift-ci commented Feb 7, 2020

Build failed
Swift Test OS X Platform
Git Sha - 8b8432ae91e77410f293275f087c73e93d5b42c3

@tbkka
Copy link
Contributor Author

tbkka commented Feb 7, 2020

@swift-ci Please test Linux

@swift-ci
Copy link
Collaborator

swift-ci commented Feb 7, 2020

Build failed
Swift Test Linux Platform
Git Sha - 8b8432ae91e77410f293275f087c73e93d5b42c3

@tbkka
Copy link
Contributor Author

tbkka commented Feb 10, 2020

Some of the new tests are failing on Linux when the tests are run in optimized builds. This is expected; this work does not address failures in the cast optimizer. I'll disable those tests in optimized builds for now.

@tbkka
Copy link
Contributor Author

tbkka commented Feb 10, 2020

@swift-ci Please test Linux

@swift-ci
Copy link
Collaborator

Build failed
Swift Test Linux Platform
Git Sha - d36ef248ab58d78d38996880770a0b132ea0ba77

@compnerd
Copy link
Collaborator

@swift-ci please test Windows platform

1 similar comment
@compnerd
Copy link
Collaborator

@swift-ci please test Windows platform

@tbkka
Copy link
Contributor Author

tbkka commented Feb 13, 2020

@swift-ci Please smoke test macOS

@tbkka
Copy link
Contributor Author

tbkka commented Feb 13, 2020

@swift-ci Please smoke test Linux

1 similar comment
@tbkka
Copy link
Contributor Author

tbkka commented Feb 14, 2020

@swift-ci Please smoke test Linux

@tbkka
Copy link
Contributor Author

tbkka commented Feb 17, 2020

@swift-ci Please test macOS

@swift-ci
Copy link
Collaborator

Build failed
Swift Test OS X Platform
Git Sha - 32e0cf01977893258f2874985ed7f174faa04883

@tbkka
Copy link
Contributor Author

tbkka commented Feb 19, 2020

@swift-ci Please test macOS

@swift-ci
Copy link
Collaborator

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

@tbkka
Copy link
Contributor Author

tbkka commented Aug 14, 2020

@swift-ci Please test macOS

@swift-ci
Copy link
Collaborator

Build failed
Swift Test OS X Platform
Git Sha - 42b6fc012bbcc54b15048a691c90f39cd2f59f02

@tbkka
Copy link
Contributor Author

tbkka commented Aug 17, 2020

Building out a separate PR #33473 with a more ambitious set of tests. That PR includes the BoxingCasts test suite from this PR and a new suite based on the Casting spec. That PR runs all the tests with both -O and -Onone as well.

I plan to break out pieces of this PR into separate PRs and merge them while using PR #33473 to track overall progress towards full compliance with the expected behavior.

@tbkka tbkka force-pushed the tbkka-dynamicCastRework branch 7 times, most recently from c3da019 to 70fdc8e Compare August 31, 2020 18:52
This includes the rewritten runtime, several compiler fixes,
and many test changes.

This is (crudely) rebased on top of master to try to minimize
confusion.  TODO:  It would be nice to break this out into
several commits.
This validation test exercises a large matrix of types and invariants for
dynamic casting.  It's formulated as a Python script that emits a number of
Swift test programs, compiles, and executes them.  The programs are compiled in
Swift 4 and 5 mode, with -O and -Onone.

The invariants used by these tests follow the specification presented
in PR apple#33010.  It should be easy to add more as desired.

I've tried to design this in such a way that CI logs can provide enough
information to narrowly identify the problem area:  Separate test cases
are generated for each invariant and each type (in particular, this helps
with compiler crashes that report the full body of the function in question).
The files and test suites are named to identify the optimization mode.

The goal of this test suite is to cover a broad cross-section of casting
capabilities and combinations, and to make it easy to expand the matrix of
combinations.  New invariants can easily be added and applied to many types;
as new types are added to this test, they can exploit the existing invariants
and be exercised across all optimization modes.
Generally, if you can cast something _to_ a box type (Any, AnyObject,
existential, _SwiftValue, AnyHashable, etc), then you should
be able to cast back to the original type.  This test suite
simply exercises a large number of such paths.

This has been the source of many bugs with the previous implementation.
@shahmishal
Copy link
Member

Please update the base branch to main by Oct 5th otherwise the pull request will be closed automatically.

  • How to change the base branch: (Link)
  • More detail about the branch update: (Link)

@tbkka tbkka changed the base branch from master to main October 1, 2020 22:12
@tbkka
Copy link
Contributor Author

tbkka commented Oct 7, 2021

This work was submitted via #33561 and related PRs, so this PR is no longer relevant.

@tbkka tbkka closed this Oct 7, 2021
@tbkka tbkka deleted the tbkka-dynamicCastRework branch October 7, 2021 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants