swiftlang / swift

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

[SR-10682] Swift master, ARC performance regression #53080

Closed weissi closed 4 years ago

weissi commented 5 years ago
Previous ID SR-10682
Radar rdar://50956754
Original Reporter @weissi
Type Bug
Status Resolved
Resolution Done

Attachment: Download

Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug, ARC, Optimizer, Performance | |Assignee | None | |Priority | Medium | md5: 208e526eb5c9a104ccd7933ca893207e

relates to:

Issue Description:

Attached is cp_perf.swift which is basically "SwiftNIO in a box". Not really but it contains an approximate implementation of NIO's ChannelPipeline. Something I'm just about to optimise. Whilst doing that I just noticed, that Swift master makes it a lot slower!

Repro:

$ jw-swift-5.0 swiftc -Ounchecked cp_perf.swift && time ./cp_perf

real    0m2.858s
user    0m2.839s
sys 0m0.009s
johannes:~/devel/pipeline-optimisation (master)
$ jw-swift-5.1 swiftc -Ounchecked cp_perf.swift && time ./cp_perf

real    0m2.758s
user    0m2.748s
sys 0m0.005s
johannes:~/devel/pipeline-optimisation (master)
$ jw-swift-latest swiftc -Ounchecked cp_perf.swift && time ./cp_perf

real    0m5.098s
user    0m5.067s
sys 0m0.007s

As you can see, the attached file runs in 2.8s for 5.0 and 5.1 but takes 5s on a recent snapshot.

The concrete versions are (all OSS toolchains):

Why?

Swift decides that we need more retains/releases instead of fewer 😉

johannes:~/devel/pipeline-optimisation (master)
$ jw-swift-latest swiftc -Ounchecked -emit-assembly cp_perf.swift > /tmp/cp_perf-latest.s
johannes:~/devel/pipeline-optimisation (master)
$ jw-swift-5.1 swiftc -Ounchecked -emit-assembly cp_perf.swift > /tmp/cp_perf-5.1.s
johannes:~/devel/pipeline-optimisation (master)
$ diff -U0 /tmp/cp_perf-* | grep call
+   callq   _swift_unknownObjectRetain
+   callq   _swift_release
+   callq   _swift_unknownObjectRetain
+   callq   _swift_release
+   callq   _swift_unknownObjectRetain
-   callq   *8(%r14)
+   callq   *%r12
+   callq   _swift_unknownObjectRetain
+   callq   _swift_unknownObjectRelease

Specifically in this function:

    @inline(never)
    public func fireEvent() {
        self.next?.invokeEvent()
    }

we now seem to retain both the existential box and I think (??) the PWT too? The lines that are marked red in the screenshot below are the new retain/releases.

weissi commented 5 years ago

added a Swift benchmark in https://github.com/apple/swift/pull/24765

weissi commented 5 years ago

@swift-ci create

gottesmm commented 4 years ago

This is fixed on master. Can you try with one of the snapshots?

weissi commented 4 years ago

thanks very much @gottesmm, confirmed fixed.