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-5021] Closure not eliminated #47597
Comments
@swift-ci create |
Today, the callee receiving the closure needed to be marked inline_always for this closure to be eliminated (presumably this is what happened in the “before Dave’s changes” state). The closure specializer could/should handle this: %1 = alloc_stack
%2 = alloc_stack
%closure = partial_apply %closee(%1, %2)
apply %fun(…, %closure)
=>
%1 = alloc_stack
%2 = alloc_stack
apply %fun_with_closee(.., %1, %2)
fun_with_closee () {
bb(%1, %2):
partial_apply closuee(%1, %2)
} But currently it does not seem to do this. We can workaround by marking %fun with always_inline (confirmed that this works) or by expressing the code without the closure. When non-escaping closure become stack allocated the overhead is also going to be much less even if optimization fail. |
@dabrahams Dave, I merged the PR #9987 to support your use-case. Could you check if it solves the performance issue for your. This PR should enable specialization of closures that take @inout address-arguments (i.e. they mutate the captured vars or e.g. capture an existential) and deliver a much better performance for them. But may be it does not catch all of the test-cases you are interested in. |
I checked that closures are specialized now and partial_applies are eliminated. But it does not improve the performance of StringMatch |
That's interesting, because the workaround Arnold gave me, marking the function as |
@dabrahams Which function specifically was marked as @inline(__always)? I see an issue with this function: // specialized String.CharacterView.subscript.getter This function is very big in SIL and is ... self-recursive on the perf-mistery branch. As a result it is not inlined. |
self-recursive, really!? |
OK. I checked again. It is not literally self-recursive. Inside the specialization there is an invocation of a different specialization of the same function but with different generic parameters: |
I checked again and it seems like my measurements yesterday were bogus. My ClosureSpecializer PR fixes the performance issue! |
Additional Detail from JIRA
md5: 608abe7f30675ed1c18101e545e81efa
Issue Description:
The last commit of https://github.com/dabrahams/swift/tree/perf-mystery
produces a big slowdown in the StringMatch benchmark because (we think) closures getting passed to
UTF8.encode(_:into: )
(notUTF8.encode(_: )
) are not getting eliminatedString.CharacterView.subscript
.Here's an excerpt of the generated SIL for the standard library, where the specialization lives.
The text was updated successfully, but these errors were encountered: