[SR-9284] Performance regression in toolchain 2018-11-15

[nsalmoria]

Previous ID	SR-9284
Radar	rdar://46277030
Original Reporter	@nsalmoria
Type	Bug

Attachment: Download

Environment

macOS 10.14.1, Xcode 10.1, Swift Development Snapshot 2018-11-15 (a)

Additional Detail from JIRA

Votes	0
Component/s	Compiler
Labels	Bug, 5.0Regression, Performance
Assignee	None
Priority	Medium

md5: 79e0114211edefa4133861588e7950a1

Issue Description:

This program:

import Foundation

func test() {
    let array = [0]

    let start = Date()

    for _ in 0...10_000_000 {
        var copy = array
        copy[0] = 0
    }

    let end = Date()

    print("\(end.timeIntervalSince(start)) seconds")
}

test()

compiled with -O, runs in 2.6 seconds when compiled with Xcode 10.1 and default toolchain, but only in 7.1 seconds when compiled with toolchain 2018-11-15.

Attached is the source and the output of -emit-sil with both toolchains.

[belkadan]

@atrick, think this is related to exclusivity checking? Something else?

[nsalmoria]

Changing the Build Setting "Exclusive Access to Memory" to "No Enforcement" doesn't seem to make any difference.

[nsalmoria]

I'm also attaching the traces produced by the Time Profiler.

It looks like the faster executable calls "specialized _ArrayBufferProtocol<>.init(copying🙂", while the slower one calls "_ArrayBufferProtocol.init(copying🙂".

[atrick]

@swift-ci create.

[atrick]

@eeckstein,aschwaighofer@apple.com (JIRA User) From glancing at the SIL, it looks to me like the Array subscript modify coroutine is not being inlined.

[aschwaighofer]

I believe the problem is that ``_ArrayBufferProtocol<>.init(copying🙂`` is no longer inlinable and therefore does not get specialized.
The following change happened in April.

diff --git a/stdlib/public/core/ArrayBufferProtocol.swift b/stdlib/public/core/ArrayBufferProtocol.swift
index 8c8cd4d353..9800c89c3e 100644
--- a/stdlib/public/core/ArrayBufferProtocol.swift
+++ b/stdlib/public/core/ArrayBufferProtocol.swift
@@ -134,8 +134,8 @@ extension _ArrayBufferProtocol where Indices == Range<Int>{ 
   }
 
   // Make sure the compiler does not inline _copyBuffer to reduce code size.
-  @inlinable
   @inline(never)
+  @usableFromInline
   internal init(copying buffer: Self) {
     let newBuffer = _ContiguousArrayBuffer<Element>(
       _uninitializedCount: buffer.count, minimumCapacity: buffer.count)

[aschwaighofer]

After changing this back the benchmark goes from 4 to 1.5 seconds on my machine.

CC @milseman

The change was part of the following commit. Can we reintroduce ``@inlineable`` here?

[stdlib] Drop @inlinable if @inline(never).

Aggressively remove all `@inlinable` from any function that's
`@inline(never)` to see the impact.

`@inlinable @inline(never)` is a potential code smell. While it might
expose some optimization and specialization opportunities to the
optimizer, it's most commonly a sign that more thought is needed.

[milseman]

If it's never inlined, then the annotation is allowing the optimizer to partially reason about effects, right? What is the effect that's missing and is there an existing annotation for that?

edit: Or should this not be never-inline? Or, is there something about inlineable that allows specialization without inlining?

[aschwaighofer]

> Or, is there something about inlineable that allows specialization without inlining?
That is what makes the difference here.
Because the function is marked inlinable the SIL is available and the compiler can specialize the function into the client code.

[aschwaighofer]

@inlinable @inline(never) allows specialization but prevents further per use site code bloat of inlining the specialized function.

[milseman]

I see, restore inlinable is ok with me then

[aschwaighofer]

@swift-ci create

[aschwaighofer]

Fixed #20763