[ownership] Change init_existential_ref to be forwarding for both guaranteed and owned. #30291
Conversation
|
@swift-ci benchmark |
|
@swift-ci test |
Performance: -O
Code size: -OPerformance: -OsizeCode size: -OsizePerformance: -OnoneCode size: -swiftlibsHow to read the dataThe tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.If you see any unexpected regressions, you should consider fixing the Noise: Sometimes the performance results (not code size!) contain false Hardware Overview |
|
Build failed |
|
Build failed |
…ranteed and owned. Otherwise in call frames like the one in the test in this commit get unneeded ARC traffic. We should never pessimize read only code that doesnt need side-effects with side-effects if we can avoid it. I am seeing this a bunch when I look at SIL from projects that use a lot of protocols. Specifically, one has a sort of trampoline code that wraps a ref counted object in an existential ref container (which from an ARC perspective doesn't imply ownership) and then calls a method on it or passes it off to some other code. Because of this requirement, there is a copy/destroy that can not be eliminated unless we can devirt/inline/eliminate the init_existential_ref box, inline enough that the low level ARC optimizer can hit it. We shouldn't rely on such properties if we do not need to.
gottesmm
force-pushed
the
pr-20d3a11a596c39f79b461a829ff03722b9755ec9
branch
from
530298e to
e9896fb
Compare
|
@swift-ci smoke test and merge |
4 similar comments
|
@swift-ci smoke test and merge |
|
@swift-ci smoke test and merge |
|
@swift-ci smoke test and merge |
|
@swift-ci smoke test and merge |
| // class based code where we are propagating around a plus zero version of a | ||
| // value and need to wrap the class in an existential wrapper in an intermediate | ||
| // frame from usage. In such cases, we have been creating unnecessary ref count | ||
| // traffic in code. |
There was a problem hiding this comment.
@gottesmm it's hard to understand this comment because you're explaining a previous implementation. Just explain the key points of the current implementation:
- it has no net effect on RC
- it consumes an owned value to produce an owned existential
- or it "extends" a borrowed value to produce a borrowed existential inside the lifetime of the original value
I'm saying the a forwarding cast "extends" a borrow, while a guaranteed phi (or tuple/struct) "reborrows" the value, unless you have other terms
There was a problem hiding this comment.
I am fine with that.
Otherwise in call frames like the one in the test in this commit get unneeded
ARC traffic. We should never pessimize read only code that doesnt need
side-effects with side-effects if we can avoid it.
I am seeing this a bunch when I look at SIL from projects that use a lot of
protocols. Specifically, one has a sort of trampoline code that wraps a ref
counted object in an existential ref container (which from an ARC perspective
doesn't imply ownership) and then calls a method on it or passes it off to some
other code.
Because of this requirement, there is a copy/destroy that can not be eliminated
unless we can devirt/inline/eliminate the init_existential_ref box, inline
enough that the low level ARC optimizer can hit it. We shouldn't rely on such
properties if we do not need to.