Skip to content
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

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

Closed
dmcyk opened this issue Jan 20, 2019 · 4 comments
Closed

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

dmcyk opened this issue Jan 20, 2019 · 4 comments
Assignees
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself

Comments

@dmcyk
Copy link
Contributor

dmcyk commented Jan 20, 2019

Previous ID SR-9711
Radar None
Original Reporter @dmcyk
Type Bug
Status Resolved
Resolution Done

Attachment: Download

Environment

Swift 5 - swift-5.0-DEVELOPMENT-SNAPSHOT-2019-01-18

Swift 4.2.1 - Xcode 10.1

Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug
Assignee @aschwaighofer
Priority Medium

md5: 975cbca4be4db22be3121cb69a0e583a

cloned to:

  • SR-9712 Swift 5 fails to recognise thin closure (assertion failure)

Issue Description:

Disclaimer: I'm not sure if this should be treated as a bug, as included use case relies on `@inline(__always)`, but I guess it could be affecting stdlib in some cases too.

Problem

There seems to be regression in Swift 5, where the compiler fails to optimise (both thin and thick) closures across module boundaries.

Code

Given module foo:

// foo.swift
import Foundation

public enum Foo<Success, Failure: Error> {

  case success(Success)

  case failure(Failure)

  @inline(__always)
  public func map<NewSuccess>(
    _ transform: (Success) -> NewSuccess
  ) -> Foo<NewSuccess, Failure> {
    switch self {
    case let .success(success):
      return .success(transform(success))
    case let .failure(failure):
      return .failure(failure)
    }
  }

  @inline(__always)
  public func mapContext<T, NewSuccess>(
    _ context: inout T,
    _ transform: (Success, inout T) -> NewSuccess
  ) -> Foo<NewSuccess, Failure> {
    switch self {
    case let .success(success):
      return .success(transform(success, &context))
    case let .failure(failure):
      return .failure(failure)
    }
  }
}

#if swift(>=5.0)
#else
extension Never: Error {}
#endif

and our client module:

// main.swift
import Foundation
import Darwin
import foo

#if swift(>=5.0)
dump("swift-5.0")
#else
dump("swift-4.2")
#endif

func measureMiliseconds(_ work: () -> Void) -> Double {
  var info = mach_timebase_info()
  guard mach_timebase_info(&info) == KERN_SUCCESS else {
     return -1
  }

  let start = mach_absolute_time()
  work()
  let end = mach_absolute_time()

  let elapsed = end - start
  let nano = elapsed * UInt64(info.numer) / UInt64(info.denom)
  return TimeInterval(nano) / TimeInterval(NSEC_PER_MSEC)
}

@discardableResult @inline(never)
func sink<T>(_ val: T) -> T { return val }

@discardableResult @inline(never)
func sink<T>(_ val: inout T) -> T { return val }

func bench(times: Int = 10, _ work: () -> Void) -> TimeInterval {
  guard times > 0 else {
     return 0
  }

  return (0 ..< times).reduce(TimeInterval(0), { result, _ in
    return result + measureMiliseconds(work)
  }) / TimeInterval(times)
}

let n = 1000000

func doHeap() {
  var val = Foo<Int, Never>.success(10)

  for var i in 0 ..< n {
    val = val.map { _ in
      return sink(&i)
    }
  }

  sink(val)
}

func doStack() {
  var val = Foo<Int, Never>.success(10)

  for var i in 0 ..< n {
    val = val.mapContext(&i) {
      return sink(&$1)
    }
  }

  sink(val)
}

print("Bench heap: \(bench(doHeap))")
print("Bench stack: \(bench(doStack))")

Compilation

$ swiftc -force-single-frontend-invocation -whole-module-optimization -emit-module-path foo.swiftmodule -O -emit-library -module-name foo foo.swift
$ swiftc -force-single-frontend-invocation -whole-module-optimization -emit-executable -I . -L . -O -lfoo -module-name repro main.swift

Example Output

Swift 4.2

- "swift-4.2"
Bench heap: 1.1834853
Bench stack: 1.3548947999999998

Swift 5

- "swift-5.0"
Bench heap: 222.2373899
Bench stack: 35.2489776
@dmcyk
Copy link
Contributor Author

dmcyk commented Jan 20, 2019

I guess this could be related to SR-9712

@dmcyk
Copy link
Contributor Author

dmcyk commented Jan 20, 2019

cc aschwaighofer@apple.com (JIRA User) could this be maybe related to #21933 ?

@aschwaighofer
Copy link
Member

With Swift 5 you need to mark the method '@inlinable' even if you have specified to always inline it.

  @inline(__always)
  public func mapContext<T, NewSuccess>(

=>

  @inline(__always)
  @inlinable
  public func mapContext<T, NewSuccess>(

@dmcyk
Copy link
Contributor Author

dmcyk commented Jan 22, 2019

I see, thanks!

I'm still seeing some pretty significant performance differences. With the same code, with higher benchmark sample (100) and `n` equal to 10000000, those are results:

Swift 4.2

- "swift-4.2"
Bench heap: 10.383694049999999
Bench stack: 12.929585360000003

Swift 5

- "swift-5.0"
Bench heap: 12.977278570000001
Bench stack: 12.878154160000003

I looked at Xcode profiling with some more complex case, with essential benchmark:

func doWork() {
  let val = Atomic<Int, MutexLock>(10)

  for i in 0 ..< n {
    val.withWriteLock {
        $0 += 1
    }
  }

  sink(val.read())
}

Do you think its something worth reporting in separate issue then?
Note that the closure#1 is essentially half the time in Swift 4.2

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself
Projects
None yet
Development

No branches or pull requests

2 participants