[Dynamic Casting] Overhauled Runtime #33561

tbkka · 2020-08-20T17:08:25Z

This is a completely refactored version of the core swift_dynamicCast
runtime method. (Separated out from PR #29658 )

This fixes a number of bugs, especially in the handling of multiply-wrapped
types such as Optional within Any. The result should be much closer to the
behavior specified by docs/DynamicCasting.md.

Most of the type-specific logic is simply copied over from the
earlier implementation, but the overall structure has been changed
to be uniformly recursive. In particular, this provides consistent
handling of Optional, existentials, Any and other common "box"
types along all paths. The consistent structure should also be
easier to update in the future with new general types.

Benchmarking does not show any noticable performance implications.

Temporarily, the old implementation is still available. Setting the
environment variable SWIFT_OLD_DYNAMIC_CAST_RUNTIME before launching a program
will use the old runtime implementation. This is only to facilitate testing;
once the new implementation is stable, I expect to completely remove the old
implementation.

Partially Resolves: SR-1999 SR-2289 SR-4552 SR-6126 SR-8964

This is a completely refactored version of the core swift_dynamicCast runtime method. This fixes a number of bugs, especially in the handling of multiply-wrapped types such as Optional within Any. The result should be much closer to the behavior specified by `docs/DynamicCasting.md`. Most of the type-specific logic is simply copied over from the earlier implementation, but the overall structure has been changed to be uniformly recursive. In particular, this provides uniform handling of Optional, existentials, Any and other common "box" types along all paths. The consistent structure should also be easier to update in the future with new general types. Benchmarking does not show any noticable performance implications. **Temporarily**, the old implementation is still available. Setting the environment variable `SWIFT_OLD_DYNAMIC_CAST_RUNTIME` before launching a program will use the old runtime implementation. This is only to facilitate testing; once the new implementation is stable, I expect to completely remove the old implementation.

The old `tryCastFromNil` could never fail (hence doesn't need to "try") and doesn't need all the arguments we were giving it.

We know this memoization makes a big difference for String, but the wins for Dictionary are less clear. It's easy to add it back later if it proves helpful.

mikeash

I got partway through reviewing this today and thought I'd send the comments I have so far. More to come, presumably.

stdlib/public/runtime/DynamicCast.cpp

mikeash

Read through the rest of it. Didn't see a whole lot!

Overall, this looks fantastic. It's very clear, both in details and in the overall structure. I definitely feel like I'd be able to take a bug report and go straight to the relevant code, rather than stepping through line by line waiting to see where something went wrong like I often did with the old code. Thanks very much for taking this on and seeing it through.

stdlib/public/runtime/DynamicCast.cpp

Clarify the unreachable messaging a bit: * "Unexpected" means I knew about it but thought it could never appear here * "Unknown" means I didn't know about it when I wrote this code

tbkka · 2020-08-26T18:47:08Z

@swift-ci Please test

swift-ci · 2020-08-26T19:32:07Z

Build failed
Swift Test Linux Platform
Git Sha - df82a0f

tbkka · 2020-08-26T19:37:34Z

Looks like I failed to properly mark some code that depends on Obj-C interop.

tbkka · 2020-08-26T19:41:37Z

@swift-ci Please test

swift-ci · 2020-08-26T21:08:21Z

Build failed
Swift Test Linux Platform
Git Sha - 2acaa1a

…n-optimized builds

tbkka · 2020-08-26T22:03:00Z

Linux had an unexpected "PASS" of some of the casting tests because of a mis-match between build optimizations.

The test in question should not be marked as XFAIL for optimized mode, because it ignores the test mode and always builds without optimizations. (It always builds without optimizations because the point of the test is to exercise runtime functions that are not called when the compiler can optimize the cast in question.)

I've removed the XFAIL marker.

tbkka · 2020-08-26T22:03:10Z

@swift-ci Please test

swift-ci · 2020-08-26T22:06:06Z

Build failed
Swift Test Linux Platform
Git Sha - 2acaa1a

tbkka · 2020-08-26T22:06:46Z

Invalid SHA

swift-ci · 2020-08-26T22:07:33Z

Build failed
Swift Test OS X Platform
Git Sha - 2acaa1a

tbkka · 2020-08-26T22:07:59Z

Also invalid SHA

tbkka · 2020-08-26T22:08:32Z

@swift-ci Please test Windows platform

tbkka · 2020-08-27T00:03:11Z

@swift-ci Please test

tbkka · 2020-08-27T00:11:36Z

@swift-ci Please benchmark

tbkka · 2020-08-27T00:11:53Z

@swift-ci Please test Windows platform

swift-ci · 2020-08-27T00:46:18Z

Performance: -O

Regression	OLD	NEW	DELTA	RATIO
ProtocolConformance	34	44	+29.4%	0.77x (?)
ObjectiveCBridgeFromNSDictionaryAnyObjectForced	4550	5500	+20.9%	0.83x (?)
ObjectiveCBridgeFromNSDictionaryAnyObject	26300	29900	+13.7%	0.88x (?)
EqualStringSubstring	22	25	+13.6%	0.88x (?)
ObjectiveCBridgeFromNSSetAnyObjectForced	3100	3520	+13.5%	0.88x (?)

Improvement	OLD	NEW	DELTA	RATIO
ObjectiveCBridgeFromNSArrayAnyObjectToStringForced	26200	23600	-9.9%	1.11x (?)
ObjectiveCBridgeFromNSArrayAnyObjectToString	25000	22800	-8.8%	1.10x (?)
Set.isSubset.Int.Empty	53	49	-7.5%	1.08x (?)
DictionaryOfAnyHashableStrings_lookup	3696	3432	-7.1%	1.08x (?)
ObjectiveCBridgeStubFromArrayOfNSString2	2010	1870	-7.0%	1.07x (?)

Code size: -O

Performance: -Osize

Regression	OLD	NEW	DELTA	RATIO
ProtocolConformance	36	45	+25.0%	0.80x (?)
ObjectiveCBridgeFromNSDictionaryAnyObjectForced	4550	5450	+19.8%	0.83x (?)
ObjectiveCBridgeFromNSSetAnyObjectForced	3100	3520	+13.5%	0.88x (?)
ObjectiveCBridgeFromNSArrayAnyObject	14700	16100	+9.5%	0.91x (?)
EqualSubstringString	22	24	+9.1%	0.92x (?)
ObjectiveCBridgeFromNSDictionaryAnyObject	26100	28400	+8.8%	0.92x (?)
ObjectiveCBridgeFromNSArrayAnyObjectForced	4040	4380	+8.4%	0.92x (?)

Improvement	OLD	NEW	DELTA	RATIO
FlattenListLoop	1527	929	-39.2%	1.64x (?)
ArrayPlusEqualSingleElementCollection	470	423	-10.0%	1.11x (?)
ObjectiveCBridgeFromNSArrayAnyObjectToString	25000	22700	-9.2%	1.10x (?)
ArrayLiteral2	115	105	-8.7%	1.10x (?)
DictionaryOfAnyHashableStrings_lookup	3720	3432	-7.7%	1.08x (?)
DictionaryBridge	452	418	-7.5%	1.08x (?)
Set.isSubset.Seq.Int.Empty	133	123	-7.5%	1.08x (?)

Code size: -Osize

Performance: -Onone

Regression	OLD	NEW	DELTA	RATIO
ObjectiveCBridgeFromNSSetAnyObjectForced	3380	3880	+14.8%	0.87x (?)
ObjectiveCBridgeFromNSDictionaryAnyObject	27000	30800	+14.1%	0.88x (?)
ObjectiveCBridgeFromNSArrayAnyObjectForced	5580	6360	+14.0%	0.88x (?)
ObjectiveCBridgeFromNSDictionaryAnyObjectForced	6600	7500	+13.6%	0.88x (?)
ProtocolConformance	135	149	+10.4%	0.91x (?)
ObjectiveCBridgeFromNSArrayAnyObject	15800	17400	+10.1%	0.91x (?)
ObjectiveCBridgeStubFromNSString	554	599	+8.1%	0.92x (?)

Improvement	OLD	NEW	DELTA	RATIO
ArrayAppendUTF16Substring	48240	43812	-9.2%	1.10x (?)
DictionaryOfAnyHashableStrings_lookup	4032	3768	-6.5%	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: 6-Core 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

swift-ci · 2020-08-27T02:11:55Z

Build failed
Swift Test Linux Platform
Git Sha - 0a413f0

tbkka · 2020-08-27T14:53:37Z

@swift-ci Please test

tbkka mentioned this pull request Aug 20, 2020

DynamicCastRework, Part 2: Test Suite #33473

Closed

tbkka added 3 commits August 24, 2020 14:24

Streamline casting of nil just a bit

fea5309

The old `tryCastFromNil` could never fail (hence doesn't need to "try") and doesn't need all the arguments we were giving it.

Clarify some comments

3b23ced

Don't memoize the bridging witness for Dictionary

2fbb67a

We know this memoization makes a big difference for String, but the wins for Dictionary are less clear. It's easy to add it back later if it proves helpful.

tbkka mentioned this pull request Aug 25, 2020

Internal protocol conformance can be stripped by accident apple/swift-protobuf#1061

Closed

mikeash requested changes Aug 25, 2020

View reviewed changes

tbkka added 6 commits August 25, 2020 14:05

Clarify comment

4a3b48d

Avoid an unnecessary retain when we "take" a CF object reference

f312f6d

Correct the assertion that this memo is only ever used for a single type

7e66535

Clarify that Optionals are handled separately from Enums

6a3c43b

Fix comment typo

b5b5ace

Expand testing of label validation in casting

b09b9b9

mikeash approved these changes Aug 26, 2020

View reviewed changes

stdlib/public/runtime/DynamicCast.cpp Show resolved Hide resolved

stdlib/public/runtime/DynamicCast.cpp Show resolved Hide resolved

Mark types that we should never see as "unreachable"

df82a0f

Clarify the unreachable messaging a bit: * "Unexpected" means I knew about it but thought it could never appear here * "Unknown" means I didn't know about it when I wrote this code

tbkka added 2 commits August 26, 2020 12:38

Avoid unused variables when building without assertions

a2cd863

ObjCBridgeMemo is only used on ObjC-supporting platforms

2acaa1a

tbkka added 2 commits August 26, 2020 14:54

These are not XFAIL for optimized builds because they always force no…

a7364ee

…n-optimized builds

Comment why this test overrides the default test optimization settings

0a413f0

tbkka merged commit 524cfae into swiftlang:master Aug 27, 2020

tbkka mentioned this pull request Sep 9, 2020

Witness tables have special ptrauth requirements #33876

Merged

tbkka mentioned this pull request Oct 7, 2021

Overhaul swift_dynamicCast #29658

Closed

AnthonyLatsis mentioned this pull request Aug 12, 2022

[SR-7432] Can't see through boxed _SwiftValue when casting from @objc Type #49975

Closed

tbkka deleted the tbkka/dynamicCastRework-pt3-Runtime branch August 1, 2024 16:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Dynamic Casting] Overhauled Runtime #33561

[Dynamic Casting] Overhauled Runtime #33561

tbkka commented Aug 20, 2020

mikeash left a comment

mikeash left a comment

tbkka commented Aug 26, 2020

swift-ci commented Aug 26, 2020

tbkka commented Aug 26, 2020

tbkka commented Aug 26, 2020

swift-ci commented Aug 26, 2020

tbkka commented Aug 26, 2020

tbkka commented Aug 26, 2020

swift-ci commented Aug 26, 2020

tbkka commented Aug 26, 2020

swift-ci commented Aug 26, 2020

tbkka commented Aug 26, 2020

tbkka commented Aug 26, 2020

tbkka commented Aug 27, 2020

tbkka commented Aug 27, 2020

tbkka commented Aug 27, 2020

swift-ci commented Aug 27, 2020

swift-ci commented Aug 27, 2020

tbkka commented Aug 27, 2020

[Dynamic Casting] Overhauled Runtime #33561

[Dynamic Casting] Overhauled Runtime #33561

Conversation

tbkka commented Aug 20, 2020

mikeash left a comment

Choose a reason for hiding this comment

mikeash left a comment

Choose a reason for hiding this comment

tbkka commented Aug 26, 2020

swift-ci commented Aug 26, 2020

tbkka commented Aug 26, 2020

tbkka commented Aug 26, 2020

swift-ci commented Aug 26, 2020

tbkka commented Aug 26, 2020

tbkka commented Aug 26, 2020

swift-ci commented Aug 26, 2020

tbkka commented Aug 26, 2020

swift-ci commented Aug 26, 2020

tbkka commented Aug 26, 2020

tbkka commented Aug 26, 2020

tbkka commented Aug 27, 2020

tbkka commented Aug 27, 2020

tbkka commented Aug 27, 2020

swift-ci commented Aug 27, 2020

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

Code size: -swiftlibs

swift-ci commented Aug 27, 2020

tbkka commented Aug 27, 2020