GlobalOpt: optimize static properties with resilient types. #21614

eeckstein · 2019-01-03T19:23:57Z

For example:

struct ResilientStruct {
  var singleField: Int
  public static let x = ResilientStruct(singleField: 27)
}

The generated (resilient) getter for x should just return a 27 (wrapped in a struct) and not lazily initialize a global variable.

The main SIL infrastructure change for this is to store type lowerings for multiple resilience expansions.
This lets the SIL optimizer reason about types (e.g. if a type is loadable) in specific functions.
For example, a type might be loadable in a resilient function, but not in an inlinable function.

rdar://problem/46712028

eeckstein · 2019-01-03T19:24:29Z

@swift-ci test

eeckstein · 2019-01-03T19:24:36Z

@swift-ci benchmark

slavapestov

How important is this optimization? Can it wait until after opaque values?

slavapestov · 2019-01-03T19:45:23Z

lib/SIL/TypeLowering.cpp

+  }
+
+  // Search for a matching lowering in the linked list of lowerings.
+  while (true) {


This looks O(n), can we do this better?

We could, but currently it's not worth the effort, as n = 1.
In case we add many more resilience expansions in the future we can think of a more complicated lookup mechanism.

slavapestov · 2019-01-03T19:51:07Z

include/swift/SIL/SILBuilder.h

+  bool isLoadableOrOpaque(SILType Ty) {
+    if (!F) {
+      // We are inserting into the static initializer of a SILGlobalVariable.
+      // All types used there are loadable by definition.


What if the emitter for the initializer has a bug? I think you still want to use ResilienceExpansion::Maximal here

Unfortunately this does not work. The conservative assumption would trigger a false assert.

Oh wait, you are saying ResilienceExpansion::Maximal. Yeah, that's a good idea.

slavapestov · 2019-01-03T19:54:45Z

lib/SIL/TypeLowering.cpp

@@ -1139,7 +1139,7 @@ namespace {
  /// Build the appropriate TypeLowering subclass for the given type,
  /// which is assumed to already have been lowered.
  class LowerType
-    : public TypeClassifierBase<LowerType, const TypeLowering *>
+    : public TypeClassifierBase<LowerType, TypeLowering *>


Last time I tried to plumb through something like this I had to change a lot more code. In particular it looks like class LowerType has a number of recursive calls to TC.getTypeLowering(), and we're not passing the expansion down through them.

Look at #5524, where I was trying to do the same thing but with a generic signature.

Hm, as I see it, the Expansion is passed down recursively (with classifyType).
Except in visitTupleType - maybe this is a bug?
Anyway, in case it's not passed down in some cases, it just makes the analysis more conservative.

swift-ci · 2019-01-03T20:10:47Z

Build comment file:

Performance: -O

TEST	OLD	NEW	DELTA	RATIO
Regression
InsertCharacterEndIndexNonASCII	55	61	+10.9%	0.90x (?)
Improvement
StringToDataEmpty	700	650	-7.1%	1.08x (?)

Performance: -Osize

TEST	OLD	NEW	DELTA	RATIO
Improvement
Breadcrumbs.CopyUTF16CodeUnits.Mixed	68	57	-16.2%	1.19x
StringBuilderSmallReservingCapacity	379	338	-10.8%	1.12x
StringBuilder	364	328	-9.9%	1.11x

Performance: -Onone

TEST	OLD	NEW	DELTA	RATIO
Improvement
ArrayOfPOD	858	774	-9.8%	1.11x (?)

Code size: -swiftlibs

TEST	OLD	NEW	DELTA	RATIO
Improvement
libswiftAppKit.dylib	77824	69632	-10.5%	1.12x
libswiftDispatch.dylib	98304	94208	-4.2%	1.04x

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

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

slavapestov · 2019-01-03T22:13:18Z

Now that I think about it wouldn’t it make more sense to change SILGen to use the new SILFunction-specific entry point? Then you won’t need your new peephole at all. This was going to be my approach when I wanted to implement resilience-aware type lowering in SIL.

eeckstein · 2019-01-03T23:38:30Z

I experimented with that a bit, but it didn't work out. The reason is that the initializer of the resilient struct still has to return the struct indirectly. Only after inlining this initializer into the globalinit function the peephole can do its work.

slavapestov · 2019-01-04T02:23:43Z

@eeckstein ah, I see. In any case we should eventually fix SILGen to do the right thing here. Also once we get opaque values, SILGen won't be initializing structs in memory in-place with struct_element_addr anymore; the whole Initialization thing will go away.

So I'm fine with landing this as-is, if you fix the >80 char lines and also fix the TupleType case.

slavapestov · 2019-01-04T02:24:30Z

test/SILOptimizer/globalopt_resilience.swift

+public struct ResilientStruct {
+  var rawValue: Int
+
+  public static let staticVal = ResilientStruct(rawValue: 27)


Can you add some more test case too though? Global variables, as well as maybe statics with tuple type, or something involving enums.

This lets the SIL optimizer reason about types (e.g. if a type is loadable) in specific functions. For example, a type might be loadable in a resilient function, but not in an inlinable function.

…ilience. Allow creating instructions with types which are only loadable in resilient functions (but not in all functions).

For example: struct ResilientStruct { var singleField: Int public static let x = ResilientStruct(singleField: 27) } The generated (resilient) getter for x should just return a 27 (wrapped in a struct) and not lazily initialize a global variable. rdar://problem/46712028

eeckstein · 2019-01-04T19:23:58Z

I will fix the tuple case and add more tests in a follow-up PR. This GlobalOpt peephole is pretty limited and I cannot create test cases for more complex types with it. I'll use the new isLoadable API in other optimizations where I can create more test cases for it.

eeckstein · 2019-01-04T19:24:11Z

@swift-ci smoke test

slavapestov · 2019-01-05T01:27:45Z

@eeckstein Ok, thanks. I'm going to revisit this code soon since I want to implement this optimization at the SILGen level as well.

eeckstein requested review from slavapestov and rjmccall January 3, 2019 19:24

slavapestov reviewed Jan 3, 2019

View reviewed changes

slavapestov reviewed Jan 4, 2019

View reviewed changes

eeckstein added 3 commits January 4, 2019 11:21

SIL: Store type lowerings for multiple resilience expansions

41e2e29

This lets the SIL optimizer reason about types (e.g. if a type is loadable) in specific functions. For example, a type might be loadable in a resilient function, but not in an inlinable function.

SIL: make verification for loadable types more accurate regarding res…

5a91d13

…ilience. Allow creating instructions with types which are only loadable in resilient functions (but not in all functions).

eeckstein force-pushed the resilient-globalopt branch from f8f39ca to aa5a4e1 Compare January 4, 2019 19:21

eeckstein merged commit 3275f56 into swiftlang:master Jan 4, 2019

eeckstein deleted the resilient-globalopt branch January 4, 2019 21:21

GlobalOpt: optimize static properties with resilient types. #21614

GlobalOpt: optimize static properties with resilient types. #21614

Uh oh!

Conversation

eeckstein commented Jan 3, 2019

Uh oh!

eeckstein commented Jan 3, 2019

Uh oh!

eeckstein commented Jan 3, 2019

Uh oh!

slavapestov left a comment

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!

Choose a reason for hiding this comment

Uh oh!

swift-ci commented Jan 3, 2019

Build comment file:

Performance: -O

Performance: -Osize

Performance: -Onone

Code size: -swiftlibs

Uh oh!

slavapestov commented Jan 3, 2019

Uh oh!

eeckstein commented Jan 3, 2019

Uh oh!

slavapestov commented Jan 4, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eeckstein commented Jan 4, 2019

Uh oh!

eeckstein commented Jan 4, 2019

Uh oh!

slavapestov commented Jan 5, 2019

Uh oh!

Uh oh!