[Frontend] Add experimental flag to skip non-inlinable function bodies #20420

harlanhaskins · 2018-11-08T02:03:23Z

This patch adds an experimental flag to skip non-inlinable function
bodies and turns it on for the standard library and overlay
.swiftmodules.

This flag is disabled if the requested action generates IR.

This option causes some small improvements in the time it takes to type check and generate an optimized and unoptimized standard library .swiftmodule

Mode	Optimization	Time	Time (Skipping)	Ratio
`-emit-parseable-module-interface`	N/A	16.254s	14.947s	1.09x
`-emit-module`	`-Onone`	21.685s	19.462s	1.11x
`-emit-module`	`-O`	50.526s	35.234s	1.43x

However, since the standard library is full of inlinable code, it doesn't benefit much from skipping non-inlinable code.

To find a more representative Swift project with little inlinable code, I ran these benchmarks against the current master of SwiftLint, which shows much stronger wins:

Mode	Optimization	Time	Time (Skipping)	Ratio
`-emit-parseable-module-interface`	N/A	13.179s	1.732s	7.70x
`-emit-module`	`-Onone`	16.21s	2.719s	5.88x
`-emit-module`	`-O`	63.408s	12.186s	5.26x

harlanhaskins · 2018-11-08T02:03:32Z

@swift-ci please benchmark

harlanhaskins · 2018-11-08T02:03:38Z

@swift-ci please test

swift-ci · 2018-11-08T02:52:30Z

Build failed
Swift Test Linux Platform
Git Sha - 3225a1bf294752e0e991db412c88ba034423afbb

harlanhaskins · 2018-11-08T03:17:12Z

Okay, looks like I’m missing some specializations or some inlinable code.

harlanhaskins · 2018-11-08T18:48:50Z

The Linux failures might shed some light on what I'm missing:

Test Result (4 failures / +3)
  - Swift(linux-x86_64).SILOptimizer.static_arrays.swift
  - Swift(linux-x86_64).SILOptimizer.optionset.swift
  - Swift(linux-x86_64).SILOptimizer.pointer_conversion.swift
  - Swift(linux-x86_64).SILOptimizer.array_contentof_opt.swift

harlanhaskins · 2018-11-08T22:09:54Z

Oops! Was skipping inlinable deinitializers.

@swift-ci please benchmark

harlanhaskins · 2018-11-09T00:16:39Z

Still missing something...

harlanhaskins · 2018-11-10T01:02:42Z

@swift-ci please smoke test linux platform

harlanhaskins · 2018-11-15T21:39:04Z

Running these again given the @inline(__always) changes.

@swift-ci please benchmark

harlanhaskins · 2018-12-07T21:35:16Z

Just gonna kick off another benchmarking run with @jrose-apple's recent pointer-related changes in #21126.

@swift-ci please benchmark

harlanhaskins · 2018-12-07T21:35:25Z

@swift-ci please smoke test

harlanhaskins · 2018-12-07T22:32:59Z

Seems that wasn't it.

harlanhaskins · 2019-01-04T00:28:44Z

Running again just disabling the optimization for SwiftOnoneSupport.

@swift-ci please benchmark

harlanhaskins · 2019-01-04T01:01:57Z

@swift-ci please benchmark

swift-ci · 2019-01-04T01:33:13Z

Build comment file:

Build failed before running benchmark.

harlanhaskins · 2019-01-04T02:37:07Z

@swift-ci please benchmark

swift-ci · 2019-09-20T22:19:06Z

Performance: -O

Improvement	OLD	NEW	DELTA	RATIO
ObjectiveCBridgeStubFromNSStringRef	93	85	-8.6%	1.09x (?)

Code size: -O

Performance: -Osize

Regression	OLD	NEW	DELTA	RATIO
StaticArray	1	2	+99.9%	0.50x (?)

Code size: -Osize

Performance: -Onone

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

harlanhaskins · 2019-09-23T23:18:51Z

@swift-ci please smoke test

harlanhaskins · 2019-09-25T14:31:06Z

@swift-ci please smoke test

lib/SILGen/SILGen.cpp

lib/Sema/TypeCheckStmt.cpp

harlanhaskins · 2019-09-26T00:48:28Z

@swift-ci please test

This doesn’t need to be a method, since it’s now only going to be used in the Decl checker

This flag, currently staged in as `-experimental-skip-non-inlinable-function-bodies`, will cause the typechecker to skip typechecking bodies of functions that will not be serialized in the resulting `.swiftmodule`. This patch also includes a SIL verifier that ensures that we don’t accidentally include a body that we should have skipped. There is still some work left to make sure the emitted .swiftmodule is exactly the same as what’s emitted without the flag, which is what’s causing the benchmark noise above. I’ll be committing follow-up patches to address those, but for now I’m going to land the implementation behind a flag.

harlanhaskins · 2019-09-26T17:40:32Z

@swift-ci please test

jrose-apple · 2019-10-14T18:48:29Z

include/swift/Option/Options.td

+def experimental_skip_non_inlinable_function_bodies:
+  Flag<["-"], "experimental-skip-non-inlinable-function-bodies">,
+  Flags<[FrontendOption, HelpHidden]>,
+  HelpText<"Skip type-checking and SIL generation for non-inlinable function bodies">;


Nitpick: why is this in the middle of all the stats options?

(Also, why is it a Driver option again?)

jrose-apple · 2019-10-14T18:50:51Z

lib/SILOptimizer/UtilityPasses/NonInlinableFunctionSkippingChecker.cpp

+  // FIXME: We can probably skip property initializers, too.
+  auto func = F.getLocation().getAsASTNode<AbstractFunctionDecl>();
+  if (!func)
+    return false;


Yeah, this scares me. All sorts of synthesized things will slip through here too.

jrose-apple · 2019-10-14T18:51:07Z

lib/SILOptimizer/UtilityPasses/NonInlinableFunctionSkippingChecker.cpp

+
+  // Local function bodies in inlinable code are okay to show up in the module.
+  if (func->getDeclContext()->isLocalContext())
+    return false;


Worth checking that the enclosing context is in fact inlinable.

jrose-apple · 2019-10-14T18:54:13Z

lib/Sema/TypeChecker.h

+
+  bool canSkipNonInlinableBodies() const {
+    return SkipNonInlinableFunctionBodies;
+  }


Nitpick: why does this need a getter and setter?

jrose-apple · 2019-10-14T18:54:57Z

test/Frontend/skip-non-inlinable-function-bodies.swift

+// 2. Emit the SIL for a module and check it against the expected strings in function bodies.
+//    If we're doing the right skipping, then the strings that are CHECK-NOT'd won't have been typechecked or SILGen'd.
+
+// RUN: %target-swift-frontend -emit-sil -O -experimental-skip-non-inlinable-function-bodies %s 2>&1 | %FileCheck %s --check-prefixes CHECK,CHECK-SIL-ONLY


Looks like you didn't end up using CHECK-SIL-ONLY.

jrose-apple · 2019-10-14T18:55:10Z

test/Frontend/skip-non-inlinable-function-bodies.swift

+// 3. Emit the module interface while skipping non-inlinable function bodies. Check it against the same set of strings.
+
+// RUN: %target-swift-frontend -typecheck %s -enable-library-evolution -emit-module-interface-path %t/Module.skipping.swiftinterface -experimental-skip-non-inlinable-function-bodies
+// RUN: %FileCheck %s --check-prefixes CHECK,CHECK-INTERFACE-ONLY < %t/Module.skipping.swiftinterface


…or CHECK-INTERFACE-ONLY