> On Dec 29, 2017, at 4:58 PM, Félix Cloutier <felixclout...@icloud.com> wrote:
>
> Thanks Michael! Other people might have different opinions, but knowing that
> a "definitive" fix is on the way is plenty for me and I'm not looking to make
> a fuss.
No worries. Happy to help = ). If you are interested, I would be happy to help
guide you in implementing one of these optimizations.
>
> If you still have some time, I have another question or two. You said that
> only self is passed at +0. Does this mean, for instance, that custom
> operators would still get +1 Expr values?
Yes. But again assuming a non-resilient operator, the optimizer can convert the
+1 in many cases to +0 via the owned->guaranteed optimization.
> It seems to me that if you receive any boxed value at +0, then it's always
> safe to also pass it further at +0 since it is immutable.
> That would have a huge impact on this program. Are there downsides to more
> pervasive +0 parameters?
The main downside is that in some cases you do want to have +1 parameters,
namely places where you are forwarding values into a function for storage. An
example of such a case is a value passed to a setter or an initializer. I would
think of it more as an engineering trade-off.
I am currently experimenting with changing normal function arguments to +0 for
this purpose, leaving initializers and setters as taking values at +1.
Regardless of the default, we can always provide an attribute for the purpose
of allowing the user to specify such behavior.
Michael
>
> Félix
>
>> Le 29 déc. 2017 à 15:42, Michael Gottesman <mgottes...@apple.com> a écrit :
>>
>>
>>
>>> On Dec 28, 2017, at 4:27 PM, Chris Lattner via swift-dev
>>> <swift-dev@swift.org> wrote:
>>>
>>> <moving to swift-dev, where most of the perf optimization people lurk>
>>>
>>> This is a great question, I’m not sure what the answer is: maybe it is a
>>> simple case missed by the arc optimizer?
>>
>> It is a bit more complicated than that. There are codegen aspects and
>> optimization aspects. In both cases there are "structural issues" that I am
>> currently in the process of fixing as part of the "semantic sil" effort.
>> There are some retain/release pairs that we could eliminate by teaching the
>> optimizer about enums/indirect boxes. I go over all of this below.
>>
>> TLDR: There are structural issues here that make this difficult/unsafe in
>> the general case. Semantic SIL eliminates all of the structural issues with
>> ARC and makes this problem (among other problems) "trivial", so we have been
>> focusing our engineering time on implementing that. That being said, we
>> could use the effects analysis to handle this trivial case. But it would
>> have to be very conservative. Chris, feel like doing some ARC Optimization?
>> = p.
>>
>> # Codegen Aspects
>>
>> ## Background Info: Local Retain/Release Pairs
>>
>> If one does not know about local vs non-local retain/release pairs, see the
>> appendix for an explanation.
>>
>> ## +0 Self
>>
>> In Swift today all parameters except for self are passed at +1. Self is
>> passed at +0. Even though this is true, often times SILGen will be
>> conservative around self and will emit a copy/destroy around the call site.
>> This is actually better since it eliminates a non-local retain/release pair
>> in the callee/caller favor of a local retain/release pair in the caller that
>> can be eliminated by the optimizer without any form of inter-procedural
>> optimization. That is why there is a retain/release at the count call. I
>> will go into why we are not optimizing it below in the optimization section.
>>
>> ## Pattern Matching at +1
>>
>> All pattern matching in Swift today is done at +1. That means that in the
>> following Swift:
>>
>> ----
>> enum Expr: CustomStringConvertible {
>> case Int(n: Int)
>> indirect case Var(x: String)
>> indirect case Add(f: Expr, g: Expr)
>> indirect case Mul(f: Expr, g: Expr)
>> indirect case Pow(f: Expr, g: Expr)
>> indirect case Ln(f: Expr)
>>
>> ...
>>
>> var count: Int {
>> switch self {
>> case .Int, .Var: return 1
>> case let .Ln(f): return f.count
>> case let .Add(f, g), let .Mul(f, g), let .Pow(f, g):
>> return f.count + g.count
>> }
>> }
>> }
>> ----
>>
>> even though self is passed at +0 to count, SILGen will emit a retain before
>> the switch. This is unfortunate and adds unnecessary ARC traffic in read
>> only methods on enums. There is no reason not to emit switches that way, so
>> as part of the work to make SILGenPattern able to pass the ownership
>> verifier, I plan on changing all pattern emission to be done at +0. As an
>> added benefit once that work is complete, refactorings in SILGenPattern will
>> be exposed that will enable us to significantly simplify SILGenPattern's
>> handling of cleanups. This complex code has lead to obscure bugs in the past
>> and is IMO overly complex.
>>
>> # Optimizer Aspects
>>
>> Lets start by looking at count.getter. Here is the SIL in CFG form:
>>
>> <count.getter.pdf>
>>
>> Lets analyze bb4 (I think bb5 suffers from similar issues).
>>
>> The retain, release pair on %0 here suffers from the turning off of the
>> guaranteed optimization. The guaranteed optimization says that any
>> retain/release pairs on a guaranteed value can be eliminated if they are
>> "semantic pairings". We had to turn off this optimization since without
>> semantic-sil we were guessing what retain/release pairings were "semantic
>> pairings". In certain cases this would result in mispairings so we had to
>> turn it off in the general case. Specifically, consider the following straw
>> man:
>>
>> ----
>> strong_retain %x
>> ...
>> strong_release obfuscated(%x)
>> ...
>> strong_retain obfuscated'(%x)
>> ...
>> strong_release %x
>> ----
>>
>> in this case we would pair/remove the outer retain, release pair yielding:
>>
>> ----
>> strong_release obfuscated(%x)
>> ...
>> strong_retain obfuscated'(%x)
>> ----
>>
>> While the reference count on %x is still balanced, since the retain count
>> balancing is in inverse order this can result in pre-mature releases and
>> use-after-frees. Once semantic-sil is complete, all pairings are explicit in
>> the IR so such a bug can not happen. To work around that problem, today we
>> are conservative and require any retain/release pair around a guaranteed
>> parameter to be +1 before and have a use afterwards.
>>
>> The second retain, release pair (i.e. %13) is slightly different since this
>> suffers from the optimizer needing to make a connection in between the box
>> in the enum and the value loaded from the box. Today we have enough
>> knowledge in the optimizer to make the first connection without issue via
>> the RCIdentity analysis. We do not make the connection though in between the
>> box being guaranteed and the value loaded from the box being effectively
>> guaranteed as a result. Even if we made that connection today, I am not sure
>> if in the general case it would be safe. With semantic-sil all of those
>> problems go away.
>>
>> For both of these, I think that by using appropriate inter procedural
>> effects analysis (which we have), we could teach the ARC optimizer to be
>> more aggressive around guaranteed. The specific properties would be that:
>>
>> 1. No side-effects (ignoring retain/releases).
>> 2. No retains/releases that are unbalanced.
>> 3. No escapes.
>>
>> I would have to think in more detail in order to be sure though.
>>
>> # Appendix
>>
>> ## Local Retain/Release Pairs
>>
>> There are two types of retain/release pairs in Swift, function local and
>> non-local. An example of a local retain/release is when one creates a new
>> binding, i.e.:
>>
>> ----
>> func f(x: Klass) {
>> let xhat = x
>> ...
>> }
>> ----
>>
>> When the assignment to xhat occurs we retain x and at the end of the
>> function (assuming that xhat is not used), we release xhat. Since the
>> retain/release pair are both in f, we are able to potentially pair and
>> eliminate them without needing to modify the call graph. In contrast, a
>> function non-local pairing is created when a parameter is passed at +1, e.g.:
>>
>> ----
>> func g(x: Klass) { ... x is used but not consumed ... }
>> func f(x: Klass) {
>> g(x)
>> }
>> ----
>>
>> In this case when we call g (ignoring any peepholes) we retain x first in f
>> (the callee) and then release x in g (the caller). This is bad for
>> optimization purposes since it means that we can not eliminate the
>> retain/release pair without modifying the call graph or inlining. For
>> non-resilient functions, we of course can/do inline and modify the callgraph
>> via function signature optimization. In the case of resilient functions this
>> is impossible by the nature of resilience.
>>
>>>
>>> -Chris
>>>
>>>
>>>
>>>> On Dec 27, 2017, at 9:19 PM, Félix Cloutier via swift-evolution
>>>> <swift-evolut...@swift.org> wrote:
>>>>
>>>> Running this on my MBP with 10 as the parameter:
>>>> https://gist.github.com/zneak/ae33bb970a08632cfb2925e2049f9e7a
>>>>
>>>> I get a runtime of about 10 seconds, 45% of which is spent in
>>>> retain/release calls according to Instruments (!!), and at least half of
>>>> that from Expr.count. Looking at the IR, Swift generously sprinkles
>>>> retain/release calls through the outlined copy method:
>>>>
>>>> • `self` is retained at the beginning of `count`
>>>> • The values that you get out of destructuring are retained
>>>> • Of course, when you get `count` on these, they are retained
>>>> again
>>>>
>>>> Of course, Expr.count cannot modify itself or its subexpressions because
>>>> the functions are not mutating; in fact, no function in that program can
>>>> mutate an enum case. Why, then, is Swift retaining/releasing `self` and
>>>> the values obtained from destructured patterns? They can't go away.
>>>> Shouldn't we be getting guaranteed references instead of owning references?
>>>>
>>>> That seems to hit pattern-matching-heavy programs with indirect cases
>>>> pretty hard, and it's pretty frustrating because that seems to be about
>>>> the nicest way to write this program, and there's no workaround from the
>>>> developer's perspective. I don't think that this is a fatal flaw of
>>>> refcounting, but unless I'm missing something, that's sub-par behavior.
>>>>
>>>> Félix
>>>> _______________________________________________
>>>> swift-evolution mailing list
>>>> swift-evolut...@swift.org
>>>> https://lists.swift.org/mailman/listinfo/swift-evolution
>>>
>>> _______________________________________________
>>> swift-dev mailing list
>>> swift-dev@swift.org
>>> https://lists.swift.org/mailman/listinfo/swift-dev
>>
>
_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev
