Use stack allocation for Swift closures #21933

aschwaighofer · 2019-01-16T22:26:09Z

Adds an on_stack version of partial_apply
It does not take ownership of its non-trivial arguments, is a trivial
function type and therefore must not be destroyed. The compiler must
make sure to extend the lifetime of non-trivial arguments beyond the
last use of the closure.

Closure lifetime fixup will use partial_apply [on_stack] as appropriate.

  %objc = copy_value %0 : $AnObject
  %closure = partial_apply [stack] [callee_guaranteed] %16(%obj) : $@convention(thin) (@guaranteed AnObject) -> ()
  %closure2 = mark_dependence %closure : $@NoEscape @callee_guaranteed () -> () on %obj : $AnObject
  %user = function_ref @useClosure : $@convention(thin) (@NoEscape @callee_guaranteed () -> ()) -> ()
  apply %user(%closure2) : $@convention(thin) (@NoEscape @callee_guaranteed () -> ()) -> ()
  dealloc_stack %closure : $() ->()
  destroy_value %obj : $AnObject // noescape closure does not take ownership

SR-904
rdar://35590578

@convention

It does not take ownership of its non-trivial arguments, is a trivial function type and therefore must not be destroyed. The compiler must make sure to extend the lifetime of non-trivial arguments beyond the last use of the closure. %objc = copy_value %0 : $AnObject %closure = partial_apply [stack] [callee_guaranteed] %16(%obj) : $@convention(thin) (@guaranteed AnObject) -> () %closure2 = mark_dependence %closure : $@NoEscape @callee_guaranteed () -> () on %obj : $AnObject %user = function_ref @useClosure : $@convention(thin) (@NoEscape @callee_guaranteed () -> ()) -> () apply %user(%closure2) : $@convention(thin) (@NoEscape @callee_guaranteed () -> ()) -> () dealloc_stack %closure : $() ->() destroy_value %obj : $AnObject // noescape closure does not take ownership SR-904 rdar://35590578

…hable block

aschwaighofer · 2019-01-16T22:26:26Z

@swift-ci Please test

aschwaighofer · 2019-01-16T22:26:42Z

@swift-ci Please test source compatibility

aschwaighofer · 2019-01-16T22:27:39Z

@swift-ci Please smoke benchmark

swift-ci · 2019-01-16T23:04:54Z

Build failed
Swift Test Linux Platform
Git Sha - a0b67c6

swift-ci · 2019-01-16T23:15:18Z

Build comment file:

Performance: -O

TEST	OLD	NEW	DELTA	RATIO
Regression
DataReplaceLarge	20500	23200	+13.2%	0.88x (?)
DataSubscriptSmall	28	31	+10.7%	0.90x
NopDeinit	44739	48398	+8.2%	0.92x (?)
Improvement
DataCreateSmall	35000	2000	-94.3%	17.50x
DataMutateBytesSmall	1080	200	-81.5%	5.40x
DataCreateMedium	6400	2900	-54.7%	2.21x
DataMutateBytesMedium	3940	2820	-28.4%	1.40x
CStringShortAscii	2060	1550	-24.8%	1.33x
NSDictionaryCastToSwift	4916	3711	-24.5%	1.32x
CStringLongNonAscii	311	258	-17.0%	1.21x
ObjectiveCBridgeToNSArray	16375	13708	-16.3%	1.19x
DataCountSmall	25	22	-12.0%	1.14x
DataCountMedium	31	28	-9.7%	1.11x
ObjectiveCBridgeStubToArrayOfNSString2	4094	3780	-7.7%	1.08x (?)
CStringLongAscii	414	383	-7.5%	1.08x (?)

Code size: -O

TEST	OLD	NEW	DELTA	RATIO
Regression
StringRemoveDupes.o	7536	7648	+1.5%	0.99x
Improvement
IterateData.o	1893	1757	-7.2%	1.08x
DictionaryCompactMapValues.o	19158	18670	-2.5%	1.03x
DataBenchmarks.o	52194	51066	-2.2%	1.02x
Substring.o	19295	18911	-2.0%	1.02x
CString.o	8498	8338	-1.9%	1.02x
CaptureProp.o	925	909	-1.7%	1.02x
ObjectiveCNoBridgingStubs.o	8159	8023	-1.7%	1.02x
Codable.o	28401	27993	-1.4%	1.01x

Performance: -Osize

TEST	OLD	NEW	DELTA	RATIO
Regression
DictionaryBridgeToObjC_Access	873	975	+11.7%	0.90x (?)
Breadcrumbs.CopyUTF16CodeUnits.Mixed	57	62	+8.8%	0.92x
Improvement
DataCreateSmall	34000	3000	-91.2%	11.33x
DataMutateBytesSmall	1080	220	-79.6%	4.91x
DataCreateMedium	9300	5700	-38.7%	1.63x
DataMutateBytesMedium	3920	2820	-28.1%	1.39x
CStringShortAscii	2050	1540	-24.9%	1.33x
CStringLongNonAscii	316	256	-19.0%	1.23x
ObjectiveCBridgeToNSArray	16794	13627	-18.9%	1.23x
CStringLongAscii	419	381	-9.1%	1.10x (?)
ObjectiveCBridgeStubToArrayOfNSString2	4103	3799	-7.4%	1.08x (?)

Code size: -Osize

TEST	OLD	NEW	DELTA	RATIO
Regression
StringRemoveDupes.o	4777	4873	+2.0%	0.98x
Improvement
IterateData.o	1973	1789	-9.3%	1.10x
SequenceAlgos.o	21916	20996	-4.2%	1.04x
DictionaryCompactMapValues.o	12566	12117	-3.6%	1.04x
CString.o	8394	8170	-2.7%	1.03x
DataBenchmarks.o	39466	38466	-2.5%	1.03x
Substring.o	17130	16762	-2.1%	1.02x
ObjectiveCNoBridgingStubs.o	7959	7791	-2.1%	1.02x
Codable.o	28577	28057	-1.8%	1.02x

Performance: -Onone

TEST	OLD	NEW	DELTA	RATIO
Regression
StrComplexWalk	6700	7340	+9.6%	0.91x (?)
SubstringEquatable	7406	8015	+8.2%	0.92x (?)
AngryPhonebook	5761	6195	+7.5%	0.93x (?)
Improvement
DataMutateBytesSmall	2320	340	-85.3%	6.82x
DataAccessBytesMedium	979	168	-82.8%	5.83x
DataAccessBytesSmall	1028	202	-80.4%	5.09x
DataMutateBytesMedium	5200	2900	-44.2%	1.79x
StackPromo	96986	65039	-32.9%	1.49x
DataCreateSmall	106000	72000	-32.1%	1.47x
CStringShortAscii	6880	5580	-18.9%	1.23x (?)
ObjectiveCBridgeToNSArray	16607	13761	-17.1%	1.21x
NSDictionaryCastToSwift	6237	5195	-16.7%	1.20x (?)
CStringLongNonAscii	326	273	-16.3%	1.19x
DictionaryKeysContainsNative	51	44	-13.7%	1.16x (?)
MapReduceShortString	284	251	-11.6%	1.13x (?)
UTF8Decode_InitDecoding_ascii	755	682	-9.7%	1.11x (?)
DictionaryKeysContainsCocoa	54	49	-9.3%	1.10x (?)
CStringLongAscii	446	408	-8.5%	1.09x (?)
Histogram	8970	8262	-7.9%	1.09x (?)
RangeReplaceableCollectionPlusDefault	9066	8388	-7.5%	1.08x (?)

Code size: -swiftlibs

TEST	OLD	NEW	DELTA	RATIO
Improvement
libswiftSwiftPrivate.dylib	45056	40960	-9.1%	1.10x
libswiftStdlibUnittest.dylib	372736	364544	-2.2%	1.02x

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

--------------

swift-ci · 2019-01-16T23:16:59Z

Build failed
Swift Test OS X Platform
Git Sha - a0b67c6

eeckstein · 2019-01-17T01:17:00Z

nice!

gottesmm · 2019-01-17T03:13:29Z

+1!

milseman · 2019-01-17T18:55:18Z

😍 Thank you!

Are there any guidelines or guarantees as to when this will and will not kick in that we can rely on? I'd like to add a section to stdlib programmer's manual.

aschwaighofer · 2019-01-17T19:32:20Z

@milseman While it is a best effort as this matches patterns I expect any closure directly passed to a swift closure parameter to be allocated on the stack.

useClosure(otherArg, { ... })

Objective-C block arguments remain allocated on the heap for now.

milseman · 2019-01-17T19:50:53Z

What would be an example of a pattern match failure? Is there any way we can write tests to enforce this better? We pretty much have to make all closure-taking functions inlinable currently, and this could help with that. But, the only source-level test I see here calls an @inlinable initializer, which sorta defeats the potential benefit of this change.

… reuses heap object as context

We need to recreate instructions in order as some instructions result type dependends on its operand

aschwaighofer · 2019-01-17T21:48:51Z

@swift-ci Please test

swift-ci · 2019-01-17T21:49:22Z

Build failed
Swift Test OS X Platform
Git Sha - a0b67c6

swift-ci · 2019-01-17T21:51:01Z

Build failed
Swift Test Linux Platform
Git Sha - a0b67c6

… eliminate all dealloc_stack instructions in the block

aschwaighofer · 2019-01-17T23:31:14Z

@swift-ci Please test

aschwaighofer · 2019-01-17T23:31:24Z

@swift-ci Please test source compatibility

swift-ci · 2019-01-17T23:33:30Z

Build failed
Swift Test Linux Platform
Git Sha - 614006e

swift-ci · 2019-01-17T23:35:14Z

Build failed
Swift Test OS X Platform
Git Sha - 614006e

aschwaighofer · 2019-01-18T13:57:53Z

@swift-ci Please test

swift-ci · 2019-01-18T14:00:08Z

Build failed
Swift Test Linux Platform
Git Sha - 0c7b94e

swift-ci · 2019-01-18T14:00:24Z

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

aschwaighofer · 2019-01-18T15:04:14Z

@swift-ci Please test

swift-ci · 2019-01-18T15:06:21Z

Build failed
Swift Test Linux Platform
Git Sha - 830ddef

swift-ci · 2019-01-18T15:08:29Z

Build failed
Swift Test OS X Platform
Git Sha - 830ddef

aschwaighofer · 2019-01-18T16:50:44Z

@milseman I am going to add a more comprehensive set of tests that verify that the closure is stack allocated in a followup PR: aschwaighofer/swift@partial_apply_stack...aschwaighofer:partial_apply_stack_tests

gottesmm · 2019-01-24T01:38:22Z

@aschwaighofer Can you add documentation around triviality/ownership to SIL.rst. I was trying to reason about this today and I had no clue.

gottesmm · 2019-01-24T02:03:19Z

nm I found it.

aschwaighofer added 19 commits January 15, 2019 11:20

IRGen for partial_apply [stack]

6f822a2

Fix value ownership of partial_apply [stack]

745bd00

Fix OperandOwnership for partial_apply [stack]

befff23

Fix StackNesting to work with partial_apply [stack]

df1eb70

Add insertDestroyOfCapturedArguments to Local.h

84c1737

MandatoryInlining fixes for partial_apply [stack]

8bcc66f

Fix exclusivity accesss enforcement for partial_apply [stack]

13498c4

Fix generic specializer for partial_apply [stack]

be31b70

Fix capture promotion for partial_apply [stack]

75dcfa4

Fix SILCombiner for partial_apply [stack]

0d9aced

Fix closure specializer for partial_apply [stack]

7ea69d1

Fix capture propagation for partial_apply [stack]

cac7fda

Update closure lifetime fixup to use partial_apply [stack]

7e6588d

Fix IRGen/partial_apply.sil test case

d102570

Fix test/IRGen/enum_resilience.swift

a3bdc5b

Fix SILGen/reabstract.swift

21d7a25

Fix test/SILOptimizer/inout_deshadow_integration.swift

b8a0893

SimplifyCFG: It is okay to ignore dealloc_stack to simplify an unreac…

a0b67c6

…hable block

BasThomas mentioned this pull request Jan 17, 2019

[126] Issue #126 - January 24, 2019 SwiftWeekly/swiftweekly.github.io#435

Closed

aschwaighofer added 2 commits January 17, 2019 13:45

Fix emitDynamicAlloca of an partial_apply [stack] in a coroutine that…

d0b0de9

… reuses heap object as context

Fix LoadableByAddress pass

614006e

We need to recreate instructions in order as some instructions result type dependends on its operand

aschwaighofer added 2 commits January 17, 2019 15:30

We can only eliminate dealloc_stack in an unreachable block if we can…

b6f9129

… eliminate all dealloc_stack instructions in the block

ClosureLifetimeFixup: Don't insert dealloc_stack at unreachable

0c7b94e

Fix partial_apply.sil test on linux

830ddef

Fix test case linux

ae00311

aschwaighofer merged commit ccbf25b into swiftlang:master Jan 18, 2019

weissi mentioned this pull request Jan 25, 2019

CI: use DEVELOPMENT-SNAPSHOT-2019-01-23-a apple/swift-nio#780

Merged

This was referenced Jan 18, 2019

[SR-7286] non-escaping closures aren't stack allocated #49834

Closed

[SR-904] Allocate closure context for closures passed @noescape on the stack #43516

Closed

This was referenced Jan 20, 2019

[SR-9711] Swift 5 fails to inline closures across module boundaries #52149

Closed

[SR-9712] Swift 5 fails to recognise thin closure (assertion failure) #52150

Open

eeckstein mentioned this pull request Feb 25, 2019

[SR-9980] Eliminate undesired heap allocation of closure box in deinit function of ManagedBuffer subclass #52384

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use stack allocation for Swift closures #21933

Use stack allocation for Swift closures #21933

aschwaighofer commented Jan 16, 2019

aschwaighofer commented Jan 16, 2019

aschwaighofer commented Jan 16, 2019

aschwaighofer commented Jan 16, 2019

swift-ci commented Jan 16, 2019

swift-ci commented Jan 16, 2019

swift-ci commented Jan 16, 2019

eeckstein commented Jan 17, 2019

gottesmm commented Jan 17, 2019

milseman commented Jan 17, 2019

aschwaighofer commented Jan 17, 2019

milseman commented Jan 17, 2019

aschwaighofer commented Jan 17, 2019

swift-ci commented Jan 17, 2019

swift-ci commented Jan 17, 2019

aschwaighofer commented Jan 17, 2019

aschwaighofer commented Jan 17, 2019

swift-ci commented Jan 17, 2019

swift-ci commented Jan 17, 2019

aschwaighofer commented Jan 18, 2019

swift-ci commented Jan 18, 2019

swift-ci commented Jan 18, 2019

aschwaighofer commented Jan 18, 2019

swift-ci commented Jan 18, 2019

swift-ci commented Jan 18, 2019

aschwaighofer commented Jan 18, 2019

gottesmm commented Jan 24, 2019

gottesmm commented Jan 24, 2019

Use stack allocation for Swift closures #21933

Use stack allocation for Swift closures #21933

Conversation

aschwaighofer commented Jan 16, 2019

aschwaighofer commented Jan 16, 2019

aschwaighofer commented Jan 16, 2019

aschwaighofer commented Jan 16, 2019

swift-ci commented Jan 16, 2019

swift-ci commented Jan 16, 2019

Build comment file:

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

Code size: -swiftlibs

swift-ci commented Jan 16, 2019

eeckstein commented Jan 17, 2019

gottesmm commented Jan 17, 2019

milseman commented Jan 17, 2019

aschwaighofer commented Jan 17, 2019

milseman commented Jan 17, 2019

aschwaighofer commented Jan 17, 2019

swift-ci commented Jan 17, 2019

swift-ci commented Jan 17, 2019

aschwaighofer commented Jan 17, 2019

aschwaighofer commented Jan 17, 2019

swift-ci commented Jan 17, 2019

swift-ci commented Jan 17, 2019

aschwaighofer commented Jan 18, 2019

swift-ci commented Jan 18, 2019

swift-ci commented Jan 18, 2019

aschwaighofer commented Jan 18, 2019

swift-ci commented Jan 18, 2019

swift-ci commented Jan 18, 2019

aschwaighofer commented Jan 18, 2019

gottesmm commented Jan 24, 2019

gottesmm commented Jan 24, 2019