Skip to content

SILGen: Emit literal closures at the abstraction level of their context. #38816

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

Merged

Conversation

jckarter
Copy link
Contributor

@jckarter jckarter commented Aug 9, 2021

Literal closures are only ever directly referenced in the context of the expression they're written in,
so it's wasteful to emit them at their fully-substituted calling convention and then reabstract them if
they're passed directly to a generic function. Avoid this by saving the abstraction pattern of the context
before emitting the closure, and then lowering its main entry point's calling convention at that
level of abstraction. Generalize some of the prolog/epilog code to handle converting arguments and returns
to the correct representation for a different abstraction level.

@jckarter
Copy link
Contributor Author

jckarter commented Aug 9, 2021

@swift-ci Please benchmark

@swift-ci
Copy link
Contributor

swift-ci commented Aug 9, 2021

Performance (x86_64): -O

Regression OLD NEW DELTA RATIO
DictionaryOfAnyHashableStrings_insert 3080 5600 +81.8% 0.55x
UnicodeStringFromCodable 508 727 +43.1% 0.70x (?)
Set.isDisjoint.Box25 355 508 +43.1% 0.70x (?)
LuhnAlgoEager 230 326 +41.7% 0.71x
LuhnAlgoLazy 229 321 +40.2% 0.71x (?)
Set.isDisjoint.Int25 268 343 +28.0% 0.78x (?)
DictionaryKeysContainsNative 22 25 +13.6% 0.88x (?)
DictionaryBridgeToObjC_Access 890 1008 +13.3% 0.88x (?)
StringRemoveDupes 262 293 +11.8% 0.89x (?)
SubstringFromLongString 10 11 +10.0% 0.91x (?)
ArrayInClass 1800 1970 +9.4% 0.91x (?)
FlattenListLoop 3889 4247 +9.2% 0.92x (?)
DistinctClassFieldAccesses 371 405 +9.2% 0.92x (?)
ObjectiveCBridgeToNSArray 12300 13400 +8.9% 0.92x (?)
XorLoop 1747 1890 +8.2% 0.92x (?)
ArrayPlusEqualFiveElementCollection 7807 8436 +8.1% 0.93x (?)
 
Improvement OLD NEW DELTA RATIO
ObjectiveCBridgeStubToNSDate2 600 560 -6.7% 1.07x (?)

Code size: -O

Regression OLD NEW DELTA RATIO
LuhnAlgoLazy.o 12162 13984 +15.0% 0.87x
LuhnAlgoEager.o 12162 13584 +11.7% 0.90x
 
Improvement OLD NEW DELTA RATIO
DictionaryRemove.o 13288 12912 -2.8% 1.03x
DictionarySwap.o 17113 16737 -2.2% 1.02x

Performance (x86_64): -Osize

Regression OLD NEW DELTA RATIO
FlattenListFlatMap 4306 6727 +56.2% 0.64x (?)
UnicodeStringFromCodable 508 714 +40.6% 0.71x (?)
FlattenListLoop 3976 5221 +31.3% 0.76x (?)
LuhnAlgoLazy 818 928 +13.4% 0.88x (?)
LuhnAlgoEager 817 925 +13.2% 0.88x (?)
ArrayInClass 1835 2010 +9.5% 0.91x (?)
Array2D 6928 7520 +8.5% 0.92x (?)
XorLoop 1747 1890 +8.2% 0.92x (?)
ArrayPlusEqualFiveElementCollection 7696 8325 +8.2% 0.92x (?)
ArrayAppendReserved 1370 1480 +8.0% 0.93x (?)
ArrayPlusEqualSingleElementCollection 1786 1927 +7.9% 0.93x (?)
RandomShuffleLCG2 416 448 +7.7% 0.93x (?)
 
Improvement OLD NEW DELTA RATIO
DropFirstAnySeqCRangeIterLazy 240 216 -10.0% 1.11x (?)
DropFirstAnySeqCntRangeLazy 240 216 -10.0% 1.11x (?)
PrefixAnyCollection 180 162 -10.0% 1.11x (?)
DropFirstAnyCollection 180 162 -10.0% 1.11x (?)
DropLastAnyCollection 63 57 -9.5% 1.11x (?)
DropWhileAnyCollection 198 180 -9.1% 1.10x (?)
DropFirstAnySeqCRangeIter 199 181 -9.0% 1.10x (?)
DropFirstAnySeqCntRange 199 181 -9.0% 1.10x (?)
PrefixAnySeqCRangeIterLazy 193 176 -8.8% 1.10x (?)
PrefixAnySeqCntRangeLazy 193 176 -8.8% 1.10x (?)
SuffixAnyCollection 69 63 -8.7% 1.10x (?)
DropWhileAnyCollectionLazy 275 252 -8.4% 1.09x (?)
ObjectiveCBridgeStubFromNSDate 6710 6160 -8.2% 1.09x (?)
DropWhileAnySeqCRangeIterLazy 305 281 -7.9% 1.09x (?)
DropWhileAnySeqCntRangeLazy 305 281 -7.9% 1.09x (?)
PrefixWhileAnyCollection 233 215 -7.7% 1.08x (?)
SuffixAnySequence 2569 2383 -7.2% 1.08x (?)
SuffixAnySequenceLazy 4896 4560 -6.9% 1.07x (?)

Code size: -Osize

Regression OLD NEW DELTA RATIO
LuhnAlgoLazy.o 13079 15999 +22.3% 0.82x
LuhnAlgoEager.o 13079 14779 +13.0% 0.88x
 
Improvement OLD NEW DELTA RATIO
DictionaryRemove.o 10579 10137 -4.2% 1.04x
DictionarySwap.o 15296 14854 -2.9% 1.03x
LazyFilter.o 7280 7177 -1.4% 1.01x
DictionaryCompactMapValues.o 12950 12819 -1.0% 1.01x

Performance (x86_64): -Onone

Regression OLD NEW DELTA RATIO
UnicodeStringFromCodable 534 747 +39.9% 0.71x (?)
DataAppendDataSmallToSmall 4520 5680 +25.7% 0.80x (?)
ArrayAppendOptionals 1360 1510 +11.0% 0.90x (?)
ObjectiveCBridgeStubToNSStringRef 263 289 +9.9% 0.91x (?)
String.replaceSubrange.String 25 27 +8.0% 0.93x (?)
 
Improvement OLD NEW DELTA RATIO
StringToDataSmall 3850 3200 -16.9% 1.20x (?)
StringToDataMedium 7900 6750 -14.6% 1.17x (?)
StringToDataEmpty 3550 3100 -12.7% 1.15x (?)
StringToDataLargeUnicode 8050 7400 -8.1% 1.09x (?)
MapReduceLazyCollectionShort 42191 39095 -7.3% 1.08x (?)
ObjectiveCBridgeFromNSDictionaryAnyObjectForced 12450 11550 -7.2% 1.08x (?)
String.data.Small 89 83 -6.7% 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 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

@jckarter jckarter force-pushed the emit-closure-literal-at-context-abstraction branch from 0538438 to 6542234 Compare August 11, 2021 23:19
@jckarter jckarter changed the title [WIP] Emit closure literals at the abstraction level of their point of use SILGen: Emit literal closures at the abstraction level of their context. Aug 11, 2021
@jckarter
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 6542234f25b15dfb1ca6b20fc84132dd059f1761

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 6542234f25b15dfb1ca6b20fc84132dd059f1761

@jckarter jckarter force-pushed the emit-closure-literal-at-context-abstraction branch from 6542234 to ae1193f Compare August 12, 2021 21:30
@jckarter
Copy link
Contributor Author

@swift-ci Please test

@jckarter jckarter marked this pull request as ready for review August 12, 2021 21:31
@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - ae1193f4f985b14ba826fe1cd1bc09a09aa65ec2

@jckarter jckarter force-pushed the emit-closure-literal-at-context-abstraction branch from ae1193f to e75e2dc Compare August 13, 2021 18:13
@jckarter
Copy link
Contributor Author

@swift-ci Please test

Literal closures are only ever directly referenced in the context of the expression they're written in,
so it's wasteful to emit them at their fully-substituted calling convention and then reabstract them if
they're passed directly to a generic function. Avoid this by saving the abstraction pattern of the context
before emitting the closure, and then lowering its main entry point's calling convention at that
level of abstraction. Generalize some of the prolog/epilog code to handle converting arguments and returns
to the correct representation for a different abstraction level.
@jckarter jckarter force-pushed the emit-closure-literal-at-context-abstraction branch from e75e2dc to 309500d Compare August 16, 2021 16:39
@jckarter
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 309500d

@jckarter
Copy link
Contributor Author

@swift-ci Please test macOS

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 309500d

@jckarter
Copy link
Contributor Author

@swift-ci Please smoke test macOS

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.

2 participants