swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.57k stars 10.36k forks source link

[SR-9772] Closure with context vs closure with context argument performance #52198

Open dmcyk opened 5 years ago

dmcyk commented 5 years ago
Previous ID SR-9772
Radar None
Original Reporter @dmcyk
Type Bug

Attachment: Download

Environment Xcode Version 10.2 beta (10P82s) Apple Swift version 5.0 (swiftlang-1001.0.45.7 clang-1001.0.37.7) Target: x86_64-apple-darwin18.2.0 ABI version: 0.6
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: 4e95807a960d16f759c081feac65e338

Issue Description:

Info

There seems to be a significant performance difference when using closures that implicitly capture variables vs. closures that explicitly take such variable as its argument.
It seems to only happen if within function that closure the closures (non-escaping) there's a call to function from separate module that can't be inlined.

As the closures as non-escaping, I'd assume that there'd be no overhead between either of the options.

In attached example `a.swift` includes functions that are explicitly marked not to be inlined.
Test module in `b.swift` is a benchmark example showing the difference in behaviour.

Difference boils down those two closures:

  @discardableResult
  func write(_ map: (inout T) -> Void) -> T {
    return self.lock.withLock {
      map(&self.value)
      return self.value
    }
  }

  @discardableResult
  func writeCtx(_ map: (inout T) -> Void) -> T {
   return self.lock.withLock(self) {
      map(&$0.value)
      return $0.value
   }
  }

Results

Compilation

$ swiftc -force-single-frontend-invocation -emit-module-path modulea.swiftmodule -emit-library -module-name modulea a.swift -O -whole-module-optimization
$ swiftc -force-single-frontend-invocation -I . -L . -lmodulea b.swift -O -whole-module-optimization

Example output

- "withContext()-0.22116692000000007"
- "withoutContext()-1.6866376200000002"
dmcyk commented 5 years ago

@belkadan do you maybe have any insight onto this case/know who might have?

belkadan commented 5 years ago

This is getting a lot better in master with stack-allocated closure contexts for @noescape closures. aschwaighofer@apple.com (JIRA User), think there's anything else worth tracking here?

dmcyk commented 5 years ago

@belkadan I tested this again with Xcode 11 seems like this haven't changed. I looked at the example in instruments and seems like without passing function argument explicitly there's a lot of ARC overhead.

Is there any work around stack allocated closures still in progress?

belkadan commented 5 years ago

:-/ It might be correct behavior, actually. Since the compiler can't see what sink does (by design), it can't tell if it's going to release something that's captured. @gottesmm, does that sound right?

belkadan commented 5 years ago

Actually, wait, no, that's not right. You're already in a method on self; it can't get released out from under you. But if the compiler doesn't realize that then we do have an optimization opportunity.