RFC: Proposed rewrite of Unmanaged<T>


(Dave Abrahams) #1

Hi Everybody,

We’ve been working on a rewrite of the Unmanaged<T> component, and are soliciting comments. First, a little background:

Unmanaged <https://developer.apple.com/library/ios/documentation/Swift/Conceptual/BuildingCocoaApps/WorkingWithCocoaDataTypes.html#//apple_ref/doc/uid/TP40014216-CH6-ID79> is primarily used as a return type from imported CoreFoundation functions that haven’t been annotated with reference-counting semantic information
A secondary known use-case is as a vehicle for creating a COpaquePointer containing a reference’s bits, e.g. for when you need to pass a reference through C APIs that use “void*” as a universal “give me some info and I’ll give it back to your callback” mechanism.

We saw several problems with Unmanaged that we wanted to fix:
It was poorly-named (the reference is managed by somebody, we just aren't representing that management in the type system).
Its interface was much broader than it needs to be to cover the use-cases
The purpose of many of its APIs was unclear
Its documentation was vague and hard to understand.
It didn’t establish a maximally-safe usage pattern for handling the results of un-annotated CoreFoundation functions.

The code for the proposed replacement, called UnsafeReference, is here <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift>, and a commit that updates Swift to use it is here <https://github.com/dabrahams/swift/commit/6eb86b48d150342709da3f3be9c738df23382866>.

Maximally Safe Usage

The recommended usage pattern for handling an UnsafeReference<T> returned by a function CFSomething is to always use the T instance produced by one of the forms:

    CFSomething(arguments…).release() // when the result is returned at +1

or

    CFSomething(arguments…).object // when the result is returned at +0

In other words, turn the UnsafeReference<T> into a safe T as quickly as possible, and never store the UnsafeReference<T> in a variable so that it can’t be (mis)used thereafter.

Points of Discussion

We’re interested in any feedback you might have, but there are a few points we’d especially like to address:

The name of the release() method has been contentious.
:+1:: Documentation—or naming conventions such as the “create rule <https://developer.apple.com/library/mac/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/Concepts/Ownership.html>”—normally says something like “you are responsible for releasing the result” in those cases where release() must be called, so there’s a very direct way to know which variant of the recommended usage pattern to employ.
:-1:: Some people who are very familiar with existing manual retain/release programming find the recommended usage pattern really counter-intuitive because they're “using something after calling release on it,” which one never does in Objective-C.
The alternative names we’ve been able to think of so far are verbose, clumsy, and don’t match up with anything in the documentation of the called function, so this seems like a really hard naming problem. Better ideas from the community would be most welcome here.
We’re not sure about the terminology <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift#L27> (Unretained/Retained/Released) used to precisely describe the semantics of UnsafeReference. We’d like to know if these terms make sense to you or whether you have better ideas.
We want to know whether the usage pattern recommended above works for you.
We want to know if the API is sufficiently broad or if there are things you currently get—and need—from Unmanaged that we’ve left out.

Thanks in advance,

-Dave


(Greg Parker) #2

For inspiration, the Objective-C ARC analogues to UnsafeReference.release() are:
    safeReference = (__bridge_transfer id)unsafeReference;
    // foreign retain count is "transferred" to ARC's control

    safeReference = CFBridgingRelease(unsafeReference);
    // conceptually balances CFCreate…() or CFRetain() even though it may or may not perform a release.

The two operations behave identically. Most people find that one or the other matches their mental model better. (Personally I find CFBridgingRelease() to be perfectly natural and I can never remember which way the __bridge_* casts work.)

···

On Dec 17, 2015, at 5:37 PM, Dave Abrahams via swift-evolution <swift-evolution@swift.org> wrote:

  • The name of the release() method has been contentious.
    • :+1:: Documentation—or naming conventions such as the “create rule”—normally says something like “you are responsible for releasing the result” in those cases where release() must be called, so there’s a very direct way to know which variant of the recommended usage pattern to employ.
    • :-1:: Some people who are very familiar with existing manual retain/release programming find the recommended usage pattern really counter-intuitive because they're “using something after calling release on it,” which one never does in Objective-C.
    • The alternative names we’ve been able to think of so far are verbose, clumsy, and don’t match up with anything in the documentation of the called function, so this seems like a really hard naming problem. Better ideas from the community would be most welcome here.

--
Greg Parker gparker@apple.com Runtime Wrangler


(Joe Groff) #3

`UnsafeReference` is a great name, and the proposed API definitely feels cleaner for the CF use case. Just a couple comments:

- The `bitPattern:` constructors should be between UnsafeReference and Unsafe[Mutable]Pointer<Void>, not COpaquePointer. Let COpaquePointer retire gracefully.
- `Unmanaged` has also been promoted as a solution for people who need to do manual reference counting, for performance or other reasons, so I think we might want to keep the 'retain()' method. Conveniently enough, `release()`-ing and dropping the return value would have the net effect of decrementing the refcount by one, though the admonitions about the `UnsafeReference` become invalid after that point wouldn't hold if you're using `release()` purely for that effect, so maybe `manuallyRetain()`/`manuallyRelease()` would be more appropriate for manual refcounting applications.

-Joe

···

On Dec 17, 2015, at 5:37 PM, Dave Abrahams via swift-evolution <swift-evolution@swift.org> wrote:

Hi Everybody,

We’ve been working on a rewrite of the Unmanaged<T> component, and are soliciting comments. First, a little background:

Unmanaged <https://developer.apple.com/library/ios/documentation/Swift/Conceptual/BuildingCocoaApps/WorkingWithCocoaDataTypes.html#//apple_ref/doc/uid/TP40014216-CH6-ID79> is primarily used as a return type from imported CoreFoundation functions that haven’t been annotated with reference-counting semantic information
A secondary known use-case is as a vehicle for creating a COpaquePointer containing a reference’s bits, e.g. for when you need to pass a reference through C APIs that use “void*” as a universal “give me some info and I’ll give it back to your callback” mechanism.

We saw several problems with Unmanaged that we wanted to fix:
It was poorly-named (the reference is managed by somebody, we just aren't representing that management in the type system).
Its interface was much broader than it needs to be to cover the use-cases
The purpose of many of its APIs was unclear
Its documentation was vague and hard to understand.
It didn’t establish a maximally-safe usage pattern for handling the results of un-annotated CoreFoundation functions.

The code for the proposed replacement, called UnsafeReference, is here <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift>, and a commit that updates Swift to use it is here <https://github.com/dabrahams/swift/commit/6eb86b48d150342709da3f3be9c738df23382866>.

Maximally Safe Usage

The recommended usage pattern for handling an UnsafeReference<T> returned by a function CFSomething is to always use the T instance produced by one of the forms:

    CFSomething(arguments…).release() // when the result is returned at +1

or

    CFSomething(arguments…).object // when the result is returned at +0

In other words, turn the UnsafeReference<T> into a safe T as quickly as possible, and never store the UnsafeReference<T> in a variable so that it can’t be (mis)used thereafter.

Points of Discussion

We’re interested in any feedback you might have, but there are a few points we’d especially like to address:

The name of the release() method has been contentious.
:+1:: Documentation—or naming conventions such as the “create rule <https://developer.apple.com/library/mac/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/Concepts/Ownership.html>”—normally says something like “you are responsible for releasing the result” in those cases where release() must be called, so there’s a very direct way to know which variant of the recommended usage pattern to employ.
:-1:: Some people who are very familiar with existing manual retain/release programming find the recommended usage pattern really counter-intuitive because they're “using something after calling release on it,” which one never does in Objective-C.
The alternative names we’ve been able to think of so far are verbose, clumsy, and don’t match up with anything in the documentation of the called function, so this seems like a really hard naming problem. Better ideas from the community would be most welcome here.
We’re not sure about the terminology <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift#L27> (Unretained/Retained/Released) used to precisely describe the semantics of UnsafeReference. We’d like to know if these terms make sense to you or whether you have better ideas.
We want to know whether the usage pattern recommended above works for you.
We want to know if the API is sufficiently broad or if there are things you currently get—and need—from Unmanaged that we’ve left out.


(TJ Usiyan) #4

Hello Dave,

I like this change and think that it will help clarify the purpose of the
type. As I was reading, the only concern that I had was the name. Could you
please provide some of the names that you all have considered so that we
can avoid suggesting the same things? My suggestion is

    CF*Something*(*arguments…*).retainedObject() // when the result is
returned at +1

or

    CF*Something*(*arguments…*).unretainedObject() // when the result is
returned at +0

on the premise that the important bit of information is whether or not the
object is already retained. No matter what names are chosen, that is the
data which determines which method to call. `retainedObject |
unretainedObject`, `takeRetainedObject | takeUnretainedObject`, or
`retained | unretained` all seem like viable options (that you have
probably considered).

TJ

···

On Thu, Dec 17, 2015 at 8:37 PM, Dave Abrahams via swift-evolution < swift-evolution@swift.org> wrote:

Hi Everybody,

We’ve been working on a rewrite of the Unmanaged<T> component, and are
soliciting comments. First, a little background:

   - Unmanaged
   <https://developer.apple.com/library/ios/documentation/Swift/Conceptual/BuildingCocoaApps/WorkingWithCocoaDataTypes.html#//apple_ref/doc/uid/TP40014216-CH6-ID79> is
   primarily used as a return type from imported CoreFoundation functions that
   haven’t been annotated with reference-counting semantic information
   - A secondary known use-case is as a vehicle for creating a
   COpaquePointer containing a reference’s bits, e.g. for when you need to
   pass a reference through C APIs that use “void*” as a universal “give me
   some info and I’ll give it back to your callback” mechanism.

   - We saw several problems with Unmanaged that we wanted to fix:
      - It was poorly-named (the reference is managed by *somebody*, we
      just aren't representing that management in the type system).
      - Its interface was much broader than it needs to be to cover the
      use-cases
      - The purpose of many of its APIs was unclear
      - Its documentation was vague and hard to understand.
      - It didn’t establish a maximally-safe usage pattern for handling
      the results of un-annotated CoreFoundation functions.

The code for the proposed replacement, called UnsafeReference, is here
<https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift>,
and a commit that updates Swift to use it is here
<https://github.com/dabrahams/swift/commit/6eb86b48d150342709da3f3be9c738df23382866>
.

Maximally Safe Usage

The recommended usage pattern for handling an UnsafeReference<T> returned
by a function CF*Something* is to always use the T instance produced by
one of the forms:

    CF*Something*(*arguments…*).release() // when the result is returned
at +1

or

    CF*Something*(*arguments…*).object // when the result is returned
at +0

In other words, turn the UnsafeReference<T> into a safe T as quickly as
possible, and never store the UnsafeReference<T> in a variable so that it
can’t be (mis)used thereafter.

Points of Discussion

We’re interested in any feedback you might have, but there are a few
points we’d especially like to address:

   - The name of the release() method has been contentious.
      - :+1:: Documentation—or naming conventions such as the “create rule
      <https://developer.apple.com/library/mac/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/Concepts/Ownership.html>”—normally
      says something like “you are responsible for releasing the result” in those
      cases where release() must be called, so there’s a very direct way
      to know which variant of the recommended usage pattern to employ.
      - :-1:: Some people who are very familiar with existing manual
      retain/release programming find the recommended usage pattern really
      counter-intuitive because they're “using something after calling release on
      it,” which one never does in Objective-C.
      - The alternative names we’ve been able to think of so far are
      verbose, clumsy, and don’t match up with anything in the documentation of
      the called function, so this seems like a really hard naming problem.
      Better ideas from the community would be most welcome here.
   - We’re not sure about the terminology
   <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift#L27> (Unretained/Retained/Released) used
   to precisely describe the semantics of UnsafeReference. We’d like to
   know if these terms make sense to you or whether you have better ideas.
   - We want to know whether the usage pattern recommended above works
   for you.
   - We want to know if the API is sufficiently broad or if there are
   things you currently get—and need—from Unmanaged that we’ve left out.

Thanks in advance,

-Dave

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Radek Pietruszewski) #5

Regarding naming and clarity, my first impression is “yes, this is far better”.

After 1.5 years of doing Swift, I’m _still_ confused by `takeRetainedValue()` and `takeUnretainedValue()`. Perhaps it’s because I’ve been spared having to learn at a deep level how to do manual refcounting, and so I only occasionally have to interact with APIs that require this. Still, I always have to check the documentation, and re-read the descriptions of both carefully to make sure I’m not mixing up the two.

Although `release()` probably isn’t perfect for reasons mentioned before (a hard naming problem indeed), I somehow find it easier to conceptualize it in my brain than `takeRetainedValue()`.

And the asymmetry between `release()` and `value` definitely seems like a win to me — it really emphasizes that `value` doesn’t really “do” anything, you’re just using the underlying value.

best,
— Radek

···

On 18 Dec 2015, at 02:37, Dave Abrahams via swift-evolution <swift-evolution@swift.org> wrote:

Hi Everybody,

We’ve been working on a rewrite of the Unmanaged<T> component, and are soliciting comments. First, a little background:

Unmanaged <https://developer.apple.com/library/ios/documentation/Swift/Conceptual/BuildingCocoaApps/WorkingWithCocoaDataTypes.html#//apple_ref/doc/uid/TP40014216-CH6-ID79> is primarily used as a return type from imported CoreFoundation functions that haven’t been annotated with reference-counting semantic information
A secondary known use-case is as a vehicle for creating a COpaquePointer containing a reference’s bits, e.g. for when you need to pass a reference through C APIs that use “void*” as a universal “give me some info and I’ll give it back to your callback” mechanism.

We saw several problems with Unmanaged that we wanted to fix:
It was poorly-named (the reference is managed by somebody, we just aren't representing that management in the type system).
Its interface was much broader than it needs to be to cover the use-cases
The purpose of many of its APIs was unclear
Its documentation was vague and hard to understand.
It didn’t establish a maximally-safe usage pattern for handling the results of un-annotated CoreFoundation functions.

The code for the proposed replacement, called UnsafeReference, is here <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift>, and a commit that updates Swift to use it is here <https://github.com/dabrahams/swift/commit/6eb86b48d150342709da3f3be9c738df23382866>.

Maximally Safe Usage

The recommended usage pattern for handling an UnsafeReference<T> returned by a function CFSomething is to always use the T instance produced by one of the forms:

    CFSomething(arguments…).release() // when the result is returned at +1

or

    CFSomething(arguments…).object // when the result is returned at +0

In other words, turn the UnsafeReference<T> into a safe T as quickly as possible, and never store the UnsafeReference<T> in a variable so that it can’t be (mis)used thereafter.

Points of Discussion

We’re interested in any feedback you might have, but there are a few points we’d especially like to address:

The name of the release() method has been contentious.
:+1:: Documentation—or naming conventions such as the “create rule <https://developer.apple.com/library/mac/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/Concepts/Ownership.html>”—normally says something like “you are responsible for releasing the result” in those cases where release() must be called, so there’s a very direct way to know which variant of the recommended usage pattern to employ.
:-1:: Some people who are very familiar with existing manual retain/release programming find the recommended usage pattern really counter-intuitive because they're “using something after calling release on it,” which one never does in Objective-C.
The alternative names we’ve been able to think of so far are verbose, clumsy, and don’t match up with anything in the documentation of the called function, so this seems like a really hard naming problem. Better ideas from the community would be most welcome here.
We’re not sure about the terminology <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift#L27> (Unretained/Retained/Released) used to precisely describe the semantics of UnsafeReference. We’d like to know if these terms make sense to you or whether you have better ideas.
We want to know whether the usage pattern recommended above works for you.
We want to know if the API is sufficiently broad or if there are things you currently get—and need—from Unmanaged that we’ve left out.

Thanks in advance,

-Dave

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Janosch Hildebrand) #6

I like `UnsafeReference` as the new name of the type and I think the basic API is clearer than with `Unmanaged`.
The initializers are much better than the static methods and `take(Un)RetainedValue()` were certainly less than ideal method names.

The name of the release() method has been contentious.

This is a hard one. I don't particularly like `release()` for this but I can't really think of anything better either.
I think it's descriptive which is good but it will take some getting used to as the "consuming" method although the documentation is pretty clear about the semantics which is helpful.

The `.releaseAndReturnObject` proposed by TJ feels a bit clearer but is also a lot more verbose.
Also I don't think it would make anything clearer if I didn't already know about the manual memory management terminology.
And something along these lines would also be bit more awkward to use if the return values were then discarded, although the proposed `manuallyRelease()` would take care of that...

We’re not sure about the terminology <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift#L27> (Unretained/Retained/Released) used to precisely describe the semantics of UnsafeReference. We’d like to know if these terms make sense to you or whether you have better ideas.

Coming from Objective-C I find it makes sense. I also don't think the terminology is a big issue here. If you know the concepts you're probably able to map them to these terms and the documentation seems helpful in that case.

If, on the other hand, you are not familiar with manual reference counting concepts I doubt this will be very helpful.
But I don't think that can be solved with different terminology but instead requires more extensive documentation.
However I don't think the documentation for `UnsafeReference` is the place to try to explain the whole concept and this is more of a topic for `The Swift Programming Language` or a separate (advanced) tutorial or guide.

I like that the documentation tries to lay out a clear path to follow if you just want to get an object out of some un-annotated CF API but I can't really personally judge how well that works for a novice.

One other approach to terminology that comes to mind would be to focus more on the transfer of ownership into ARC as opposed to out of MRC.
Personally I often reason in that direction when thinking about the topic but I can't see a way in which it would be helpful for the documentation or method names...

We want to know whether the usage pattern recommended above works for you.

For interacting with un-annotated CF APIs: Yes.

We want to know if the API is sufficiently broad or if there are things you currently get—and need—from Unmanaged that we’ve left out.

- `Unmanaged` has also been promoted as a solution for people who need to do manual reference counting, for performance or other reasons, so I think we might want to keep the 'retain()' method. Conveniently enough, `release()`-ing and dropping the return value would have the net effect of decrementing the refcount by one, though the admonitions about the `UnsafeReference` become invalid after that point wouldn't hold if you're using `release()` purely for that effect, so maybe `manuallyRetain()`/`manuallyRelease()` would be more appropriate for manual refcounting applications.

As Joe mentioned, `Unmanaged` has a use for manual ref counting beyond immediate transfer from un-annotated APIs.

I have used it for performance reasons myself (~ twice) and while I think it's a pretty small use case there isn't really any alternative.
If it would help I can also describe my use-cases in more detail.

I don't think this use case even needs to be described in the documentation for `UnsafeReference` and it's fine if its use is very much discouraged.

Personally I prefer the proposed `manuallyRetain()`/`manuallyRelease()` over plain `retain()`/`release()` as it clearly separates the returning and more generally applicable `release()` from the MRC methods. `retain()` would probably also have to return the object which would interfere with the max safe usage pattern.

···

On 18 Dec 2015, at 02:37, Dave Abrahams via swift-evolution <swift-evolution@swift.org> wrote:
On 18 Dec 2015, at 03:05, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

On 18 Dec 2015, at 03:05, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

- The `bitPattern:` constructors should be between UnsafeReference and Unsafe[Mutable]Pointer<Void>, not COpaquePointer. Let COpaquePointer retire gracefully.

Very much agreed.

- Janosch


(Dave Abrahams) #7

Thanks very much for the quick feedback TJ,

Hello Dave,

I like this change and think that it will help clarify the purpose of the type. As I was reading, the only concern that I had was the name. Could you please provide some of the names that you all have considered

Honestly, I am sorry to say, we did that exercise almost a month ago and I don’t remember the ones we discussed.

so that we can avoid suggesting the same things? My suggestion is

    CFSomething(arguments…).retainedObject() // when the result is returned at +1

or

    CFSomething(arguments…).unretainedObject() // when the result is returned at +0

on the premise that the important bit of information is whether or not the object is already retained. No matter what names are chosen, that is the data which determines which method to call. `retainedObject | unretainedObject`, `takeRetainedObject | takeUnretainedObject`, or `retained | unretained` all seem like viable options (that you have probably considered).

Some issues with these names:

The “ed/ing” rule <https://swift.org/documentation/api-design-guidelines.html#be-grammatical> makes these names suggest that the accessors are idempotent, but the first one must be called exactly once. That name should really be an active verb since it is state-changing.
“retainedObject” also suggests that it’s returning some underlying object after retaining it, which is almost the opposite of what that API does… and vice-versa for “unretainedObject"
Also, the object “has been retained” in all cases, or it would have been deallocated. The question is whether the object would leak if we fail to call release on it
Nothing in these names connect them to what the documentation says about the functions that return Unmanaged, so it’s hard to know which one to call
The second API is objectively safer than the first one (which causes undefined behavior when overused and only leaks when underused). The API I proposed makes it clear that they are not peers, where yours implies parity—though I am of two minds about the value of representing the lack of parity.

You may legitimately argue that any of these concerns are unimportant, but those are the ones that come up for me.

TJ

Hi Everybody,

We’ve been working on a rewrite of the Unmanaged<T> component, and are soliciting comments. First, a little background:

Unmanaged <https://developer.apple.com/library/ios/documentation/Swift/Conceptual/BuildingCocoaApps/WorkingWithCocoaDataTypes.html#//apple_ref/doc/uid/TP40014216-CH6-ID79> is primarily used as a return type from imported CoreFoundation functions that haven’t been annotated with reference-counting semantic information
A secondary known use-case is as a vehicle for creating a COpaquePointer containing a reference’s bits, e.g. for when you need to pass a reference through C APIs that use “void*” as a universal “give me some info and I’ll give it back to your callback” mechanism.

We saw several problems with Unmanaged that we wanted to fix:
It was poorly-named (the reference is managed by somebody, we just aren't representing that management in the type system).
Its interface was much broader than it needs to be to cover the use-cases
The purpose of many of its APIs was unclear
Its documentation was vague and hard to understand.
It didn’t establish a maximally-safe usage pattern for handling the results of un-annotated CoreFoundation functions.

The code for the proposed replacement, called UnsafeReference, is here <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift>, and a commit that updates Swift to use it is here <https://github.com/dabrahams/swift/commit/6eb86b48d150342709da3f3be9c738df23382866>.

Maximally Safe Usage

The recommended usage pattern for handling an UnsafeReference<T> returned by a function CFSomething is to always use the T instance produced by one of the forms:

    CFSomething(arguments…).release() // when the result is returned at +1

or

    CFSomething(arguments…).object // when the result is returned at +0

In other words, turn the UnsafeReference<T> into a safe T as quickly as possible, and never store the UnsafeReference<T> in a variable so that it can’t be (mis)used thereafter.

Points of Discussion

We’re interested in any feedback you might have, but there are a few points we’d especially like to address:

The name of the release() method has been contentious.
:+1:: Documentation—or naming conventions such as the “create rule <https://developer.apple.com/library/mac/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/Concepts/Ownership.html>”—normally says something like “you are responsible for releasing the result” in those cases where release() must be called, so there’s a very direct way to know which variant of the recommended usage pattern to employ.
:-1:: Some people who are very familiar with existing manual retain/release programming find the recommended usage pattern really counter-intuitive because they're “using something after calling release on it,” which one never does in Objective-C.
The alternative names we’ve been able to think of so far are verbose, clumsy, and don’t match up with anything in the documentation of the called function, so this seems like a really hard naming problem. Better ideas from the community would be most welcome here.
We’re not sure about the terminology <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift#L27> (Unretained/Retained/Released) used to precisely describe the semantics of UnsafeReference. We’d like to know if these terms make sense to you or whether you have better ideas.
We want to know whether the usage pattern recommended above works for you.
We want to know if the API is sufficiently broad or if there are things you currently get—and need—from Unmanaged that we’ve left out.

Thanks in advance,

-Dave

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

-Dave

···

On Dec 17, 2015, at 5:52 PM, T.J. Usiyan <griotspeak@gmail.com> wrote:
On Thu, Dec 17, 2015 at 8:37 PM, Dave Abrahams via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:


(TJ Usiyan) #8

I think I see what you mean about parity. `.object` can be called multiple
times, then? Why not make `release()` slightly more verbose since it should
only be called once anyway? Something along the lines of
`.releaseAndReturnObject` or … something slightly less verbose.

My main point of contention with `.release()` is that it has the *exact*
same name as a method from the MRC strategy. Maybe this is a silly point,
but this overlap could further complicate teaching how ARC works and in
what ways it is based on MRC conventions. I am not of the opinion that ARC
is fundamentally more difficult to understand than MRC, but I do believe
that it takes a very particular kind of faith now that we don't get to
manually write the retains and releases. This is completely worth it, in my
opinion, but I want to avoid making it *more* confusing to explain what
ARC doing at compile time.

TJ

···

On Thu, Dec 17, 2015 at 9:13 PM, Dave Abrahams <dabrahams@apple.com> wrote:

Thanks very much for the quick feedback TJ,

On Dec 17, 2015, at 5:52 PM, T.J. Usiyan <griotspeak@gmail.com> wrote:

Hello Dave,

I like this change and think that it will help clarify the purpose of the
type. As I was reading, the only concern that I had was the name. Could you
please provide some of the names that you all have considered

Honestly, I am sorry to say, we did that exercise almost a month ago and I
don’t remember the ones we discussed.

so that we can avoid suggesting the same things? My suggestion is

    CF*Something*(*arguments…*).retainedObject() // when the result is
returned at +1

or

    CF*Something*(*arguments…*).unretainedObject() // when the result
is returned at +0

on the premise that the important bit of information is whether or not the
object is already retained. No matter what names are chosen, that is the
data which determines which method to call. `retainedObject |
unretainedObject`, `takeRetainedObject | takeUnretainedObject`, or
`retained | unretained` all seem like viable options (that you have
probably considered).

Some issues with these names:

   - The “ed/ing” rule
   <https://swift.org/documentation/api-design-guidelines.html#be-grammatical> makes
   these names suggest that the accessors are idempotent, but the first one
   must be called exactly once. That name should really be an active verb
   since it is state-changing.
   - “retainedObject” also suggests that it’s returning some underlying
   object after retaining it, which is almost the opposite of what that API
   does… and vice-versa for “unretainedObject"
   - Also, the object “has been retained” in all cases, or it would have
   been deallocated. The question is whether the object would leak if we fail
   to call release on it
   - Nothing in these names connect them to what the documentation says
   about the functions that return Unmanaged, so it’s hard to know which one
   to call
   - The second API is objectively safer than the first one (which causes
   undefined behavior when overused and only leaks when underused). The API I
   proposed makes it clear that they are not peers, where yours implies
   parity—though I am of two minds about the value of representing the lack of
   parity.

You may legitimately argue that any of these concerns are unimportant, but
those are the ones that come up for me.

TJ

On Thu, Dec 17, 2015 at 8:37 PM, Dave Abrahams via swift-evolution < > swift-evolution@swift.org> wrote:

Hi Everybody,

We’ve been working on a rewrite of the Unmanaged<T> component, and are
soliciting comments. First, a little background:

   - Unmanaged
   <https://developer.apple.com/library/ios/documentation/Swift/Conceptual/BuildingCocoaApps/WorkingWithCocoaDataTypes.html#//apple_ref/doc/uid/TP40014216-CH6-ID79> is
   primarily used as a return type from imported CoreFoundation functions that
   haven’t been annotated with reference-counting semantic information
   - A secondary known use-case is as a vehicle for creating a
   COpaquePointer containing a reference’s bits, e.g. for when you need to
   pass a reference through C APIs that use “void*” as a universal “give me
   some info and I’ll give it back to your callback” mechanism.

   - We saw several problems with Unmanaged that we wanted to fix:
      - It was poorly-named (the reference is managed by *somebody*, we
      just aren't representing that management in the type system).
      - Its interface was much broader than it needs to be to cover the
      use-cases
      - The purpose of many of its APIs was unclear
      - Its documentation was vague and hard to understand.
      - It didn’t establish a maximally-safe usage pattern for handling
      the results of un-annotated CoreFoundation functions.

The code for the proposed replacement, called UnsafeReference, is here
<https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift>,
and a commit that updates Swift to use it is here
<https://github.com/dabrahams/swift/commit/6eb86b48d150342709da3f3be9c738df23382866>
.

Maximally Safe Usage

The recommended usage pattern for handling an UnsafeReference<T>
returned by a function CF*Something* is to always use the T instance
produced by one of the forms:

    CF*Something*(*arguments…*).release() // when the result is returned
at +1

or

    CF*Something*(*arguments…*).object // when the result is returned
at +0

In other words, turn the UnsafeReference<T> into a safe T as quickly as
possible, and never store the UnsafeReference<T> in a variable so that
it can’t be (mis)used thereafter.

Points of Discussion

We’re interested in any feedback you might have, but there are a few
points we’d especially like to address:

   - The name of the release() method has been contentious.
      - :+1:: Documentation—or naming conventions such as the “create rule
      <https://developer.apple.com/library/mac/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/Concepts/Ownership.html>”—normally
      says something like “you are responsible for releasing the result” in those
      cases where release() must be called, so there’s a very direct way
      to know which variant of the recommended usage pattern to employ.
      - :-1:: Some people who are very familiar with existing manual
      retain/release programming find the recommended usage pattern really
      counter-intuitive because they're “using something after calling release on
      it,” which one never does in Objective-C.
      - The alternative names we’ve been able to think of so far are
      verbose, clumsy, and don’t match up with anything in the documentation of
      the called function, so this seems like a really hard naming problem.
      Better ideas from the community would be most welcome here.
   - We’re not sure about the terminology
   <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift#L27> (Unretained/Retained/Released) used
   to precisely describe the semantics of UnsafeReference. We’d like to
   know if these terms make sense to you or whether you have better ideas.
   - We want to know whether the usage pattern recommended above works
   for you.
   - We want to know if the API is sufficiently broad or if there are
   things you currently get—and need—from Unmanaged that we’ve left out.

Thanks in advance,

-Dave

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

-Dave


(Dave Abrahams) #9

I like `UnsafeReference` as the new name of the type and I think the basic API is clearer than with `Unmanaged`.
The initializers are much better than the static methods and `take(Un)RetainedValue()` were certainly less than ideal method names.

The name of the release() method has been contentious.

This is a hard one. I don't particularly like `release()` for this but I can't really think of anything better either.
I think it's descriptive which is good but it will take some getting used to as the "consuming" method although the documentation is pretty clear about the semantics which is helpful.

The `.releaseAndReturnObject` proposed by TJ feels a bit clearer but is also a lot more verbose.
Also I don't think it would make anything clearer if I didn't already know about the manual memory management terminology.
And something along these lines would also be bit more awkward to use if the return values were then discarded, although the proposed `manuallyRelease()` would take care of that...

I don't see any point in having "manuallyRelease()" if we already have "release()"; they'd do the same thing if you dropped the return value.

We’re not sure about the terminology <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift#L27> (Unretained/Retained/Released) used to precisely describe the semantics of UnsafeReference. We’d like to know if these terms make sense to you or whether you have better ideas.

Coming from Objective-C I find it makes sense. I also don't think the terminology is a big issue here. If you know the concepts you're probably able to map them to these terms and the documentation seems helpful in that case.

If, on the other hand, you are not familiar with manual reference counting concepts I doubt this will be very helpful.
But I don't think that can be solved with different terminology but instead requires more extensive documentation.
However I don't think the documentation for `UnsafeReference` is the place to try to explain the whole concept and this is more of a topic for `The Swift Programming Language` or a separate (advanced) tutorial or guide.

I like that the documentation tries to lay out a clear path to follow if you just want to get an object out of some un-annotated CF API but I can't really personally judge how well that works for a novice.

One other approach to terminology that comes to mind would be to focus more on the transfer of ownership into ARC as opposed to out of MRC.
Personally I often reason in that direction when thinking about the topic but I can't see a way in which it would be helpful for the documentation or method names...

Yeah, doesn't give me any ideas for names, unfortunately.

We want to know whether the usage pattern recommended above works for you.

For interacting with un-annotated CF APIs: Yes.

We want to know if the API is sufficiently broad or if there are things you currently get—and need—from Unmanaged that we’ve left out.

- `Unmanaged` has also been promoted as a solution for people who need to do manual reference counting, for performance or other reasons, so I think we might want to keep the 'retain()' method. Conveniently enough, `release()`-ing and dropping the return value would have the net effect of decrementing the refcount by one, though the admonitions about the `UnsafeReference` become invalid after that point wouldn't hold if you're using `release()` purely for that effect, so maybe `manuallyRetain()`/`manuallyRelease()` would be more appropriate for manual refcounting applications.

As Joe mentioned, `Unmanaged` has a use for manual ref counting beyond immediate transfer from un-annotated APIs.

I have used it for performance reasons myself (~ twice) and while I think it's a pretty small use case there isn't really any alternative.
If it would help I can also describe my use-cases in more detail.

Yes please!

I don't think this use case even needs to be described in the documentation for `UnsafeReference` and it's fine if its use is very much discouraged.

Personally I prefer the proposed `manuallyRetain()`/`manuallyRelease()` over plain `retain()`/`release()` as it clearly separates the returning and more generally applicable `release()` from the MRC methods. `retain()` would probably also have to return the object which would interfere with the max safe usage pattern.

I don't understand your last sentence; care to clarify?

···

On Dec 18, 2015, at 6:18 PM, Janosch Hildebrand via swift-evolution <swift-evolution@swift.org> wrote:

On 18 Dec 2015, at 02:37, Dave Abrahams via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
On 18 Dec 2015, at 03:05, Joe Groff via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On 18 Dec 2015, at 03:05, Joe Groff via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

- The `bitPattern:` constructors should be between UnsafeReference and Unsafe[Mutable]Pointer<Void>, not COpaquePointer. Let COpaquePointer retire gracefully.

Very much agreed.

Please see my comment here <https://github.com/apple/swift-evolution/pull/44#issuecomment-165902471>

Thanks again,

-Dave


(Félix Cloutier) #10

If this can be used as any reference, I wrote some code to help interoperate with Python in C++. The Python API inconsistently returns references with a +0 or +1 count. I use macros called TAKEREF (when the object is returned with a +1 count) and ADDREF (when it's returned with +0) to create smart wrappers. The wrapper will always decrement the reference count during destruction.

I find it unambiguous: you need to add a reference if you're going to take one away at destruction, so ADD; but if the object is returned with a +1 count, you can just TAKE the reference. It coincidentally does not borrow terminology from Objective-C's memory management system.

···

Le 17 déc. 2015 à 21:23:56, T.J. Usiyan via swift-evolution <swift-evolution@swift.org> a écrit :

I think I see what you mean about parity. `.object` can be called multiple times, then? Why not make `release()` slightly more verbose since it should only be called once anyway? Something along the lines of `.releaseAndReturnObject` or … something slightly less verbose.

My main point of contention with `.release()` is that it has the *exact* same name as a method from the MRC strategy. Maybe this is a silly point, but this overlap could further complicate teaching how ARC works and in what ways it is based on MRC conventions. I am not of the opinion that ARC is fundamentally more difficult to understand than MRC, but I do believe that it takes a very particular kind of faith now that we don't get to manually write the retains and releases. This is completely worth it, in my opinion, but I want to avoid making it *more* confusing to explain what ARC doing at compile time.

TJ

On Thu, Dec 17, 2015 at 9:13 PM, Dave Abrahams <dabrahams@apple.com <mailto:dabrahams@apple.com>> wrote:
Thanks very much for the quick feedback TJ,

On Dec 17, 2015, at 5:52 PM, T.J. Usiyan <griotspeak@gmail.com <mailto:griotspeak@gmail.com>> wrote:

Hello Dave,

I like this change and think that it will help clarify the purpose of the type. As I was reading, the only concern that I had was the name. Could you please provide some of the names that you all have considered

Honestly, I am sorry to say, we did that exercise almost a month ago and I don’t remember the ones we discussed.

so that we can avoid suggesting the same things? My suggestion is

    CFSomething(arguments…).retainedObject() // when the result is returned at +1

or

    CFSomething(arguments…).unretainedObject() // when the result is returned at +0

on the premise that the important bit of information is whether or not the object is already retained. No matter what names are chosen, that is the data which determines which method to call. `retainedObject | unretainedObject`, `takeRetainedObject | takeUnretainedObject`, or `retained | unretained` all seem like viable options (that you have probably considered).

Some issues with these names:

The “ed/ing” rule <https://swift.org/documentation/api-design-guidelines.html#be-grammatical> makes these names suggest that the accessors are idempotent, but the first one must be called exactly once. That name should really be an active verb since it is state-changing.
“retainedObject” also suggests that it’s returning some underlying object after retaining it, which is almost the opposite of what that API does… and vice-versa for “unretainedObject"
Also, the object “has been retained” in all cases, or it would have been deallocated. The question is whether the object would leak if we fail to call release on it
Nothing in these names connect them to what the documentation says about the functions that return Unmanaged, so it’s hard to know which one to call
The second API is objectively safer than the first one (which causes undefined behavior when overused and only leaks when underused). The API I proposed makes it clear that they are not peers, where yours implies parity—though I am of two minds about the value of representing the lack of parity.

You may legitimately argue that any of these concerns are unimportant, but those are the ones that come up for me.

TJ

On Thu, Dec 17, 2015 at 8:37 PM, Dave Abrahams via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
Hi Everybody,

We’ve been working on a rewrite of the Unmanaged<T> component, and are soliciting comments. First, a little background:

Unmanaged <https://developer.apple.com/library/ios/documentation/Swift/Conceptual/BuildingCocoaApps/WorkingWithCocoaDataTypes.html#//apple_ref/doc/uid/TP40014216-CH6-ID79> is primarily used as a return type from imported CoreFoundation functions that haven’t been annotated with reference-counting semantic information
A secondary known use-case is as a vehicle for creating a COpaquePointer containing a reference’s bits, e.g. for when you need to pass a reference through C APIs that use “void*” as a universal “give me some info and I’ll give it back to your callback” mechanism.

We saw several problems with Unmanaged that we wanted to fix:
It was poorly-named (the reference is managed by somebody, we just aren't representing that management in the type system).
Its interface was much broader than it needs to be to cover the use-cases
The purpose of many of its APIs was unclear
Its documentation was vague and hard to understand.
It didn’t establish a maximally-safe usage pattern for handling the results of un-annotated CoreFoundation functions.

The code for the proposed replacement, called UnsafeReference, is here <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift>, and a commit that updates Swift to use it is here <https://github.com/dabrahams/swift/commit/6eb86b48d150342709da3f3be9c738df23382866>.

Maximally Safe Usage

The recommended usage pattern for handling an UnsafeReference<T> returned by a function CFSomething is to always use the T instance produced by one of the forms:

    CFSomething(arguments…).release() // when the result is returned at +1

or

    CFSomething(arguments…).object // when the result is returned at +0

In other words, turn the UnsafeReference<T> into a safe T as quickly as possible, and never store the UnsafeReference<T> in a variable so that it can’t be (mis)used thereafter.

Points of Discussion

We’re interested in any feedback you might have, but there are a few points we’d especially like to address:

The name of the release() method has been contentious.
:+1:: Documentation—or naming conventions such as the “create rule <https://developer.apple.com/library/mac/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/Concepts/Ownership.html>”—normally says something like “you are responsible for releasing the result” in those cases where release() must be called, so there’s a very direct way to know which variant of the recommended usage pattern to employ.
:-1:: Some people who are very familiar with existing manual retain/release programming find the recommended usage pattern really counter-intuitive because they're “using something after calling release on it,” which one never does in Objective-C.
The alternative names we’ve been able to think of so far are verbose, clumsy, and don’t match up with anything in the documentation of the called function, so this seems like a really hard naming problem. Better ideas from the community would be most welcome here.
We’re not sure about the terminology <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift#L27> (Unretained/Retained/Released) used to precisely describe the semantics of UnsafeReference. We’d like to know if these terms make sense to you or whether you have better ideas.
We want to know whether the usage pattern recommended above works for you.
We want to know if the API is sufficiently broad or if there are things you currently get—and need—from Unmanaged that we’ve left out.

Thanks in advance,

-Dave

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

-Dave

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution


(Dave Abrahams) #11

I think I see what you mean about parity. `.object` can be called multiple times, then?

Indeed, as noted here <https://github.com/dabrahams/swift/blob/UnsafeReference/stdlib/public/core/UnsafeReference.swift#L28>. I said I was “of two minds” about the importance of highlighting the lack of parity because in the recommended usage pattern, you invoke object or release() exactly once and can’t access the UnsafeReference thereafter. The main reason to emphasize the difference is that some people really seem to resist the recommended usage pattern, and for them, it really matters. The secondary difference is that our guidelines (still partly un-published) for what should be a property dictate that one has to be a method and the other a property; I need to try to get those updates out soon.

Why not make `release()` slightly more verbose since it should only be called once anyway? Something along the lines of `.releaseAndReturnObject` or … something slightly less verbose.

We could do that. I find “Object” a pretty weak term here, since the original UnsafeReference represents the same “object” in some abstract sense. It’s really something more like “Ownership” that’s being returned, but you could also argue that ownership was returned by the original call, too. If we add words here I’d like to understand what value they’re adding in terms of comprehensibility.

My main point of contention with `.release()` is that it has the *exact* same name as a method from the MRC strategy. Maybe this is a silly point, but this overlap could further complicate teaching how ARC works and in what ways it is based on MRC conventions.

I am not of the opinion that ARC is fundamentally more difficult to understand than MRC, but I do believe that it takes a very particular kind of faith now that we don't get to manually write the retains and releases. This is completely worth it, in my opinion, but I want to avoid making it *more* confusing to explain what ARC doing at compile time.

Well, unsafeRef.release() is equivalent to

  {
    let x = $0.object
    CFRelease($0) // if CFRelease() wasn't @unavailable
    return x
}(unsafeRef)

If you let the return value drop on the floor, it ends up being exactly equivalent to the method with the exact same name from the MRC strategy. So the correspondence is strong and shouldn’t be a problem. That’s just my opinion, though, and part of the reason we’re asking for feedback here is so people steeped in MRC like you can argue with me about that :-), so if you find this unconvincing please explain why.

Thanks again,
Dave

TJ

Thanks very much for the quick feedback TJ,

Hello Dave,

I like this change and think that it will help clarify the purpose of the type. As I was reading, the only concern that I had was the name. Could you please provide some of the names that you all have considered

Honestly, I am sorry to say, we did that exercise almost a month ago and I don’t remember the ones we discussed.

so that we can avoid suggesting the same things? My suggestion is

    CFSomething(arguments…).retainedObject() // when the result is returned at +1

or

    CFSomething(arguments…).unretainedObject() // when the result is returned at +0

on the premise that the important bit of information is whether or not the object is already retained. No matter what names are chosen, that is the data which determines which method to call. `retainedObject | unretainedObject`, `takeRetainedObject | takeUnretainedObject`, or `retained | unretained` all seem like viable options (that you have probably considered).

Some issues with these names:

The “ed/ing” rule <https://swift.org/documentation/api-design-guidelines.html#be-grammatical> makes these names suggest that the accessors are idempotent, but the first one must be called exactly once. That name should really be an active verb since it is state-changing.
“retainedObject” also suggests that it’s returning some underlying object after retaining it, which is almost the opposite of what that API does… and vice-versa for “unretainedObject"
Also, the object “has been retained” in all cases, or it would have been deallocated. The question is whether the object would leak if we fail to call release on it
Nothing in these names connect them to what the documentation says about the functions that return Unmanaged, so it’s hard to know which one to call
The second API is objectively safer than the first one (which causes undefined behavior when overused and only leaks when underused). The API I proposed makes it clear that they are not peers, where yours implies parity—though I am of two minds about the value of representing the lack of parity.

You may legitimately argue that any of these concerns are unimportant, but those are the ones that come up for me.

TJ

Hi Everybody,

We’ve been working on a rewrite of the Unmanaged<T> component, and are soliciting comments. First, a little background:

Unmanaged <https://developer.apple.com/library/ios/documentation/Swift/Conceptual/BuildingCocoaApps/WorkingWithCocoaDataTypes.html#//apple_ref/doc/uid/TP40014216-CH6-ID79> is primarily used as a return type from imported CoreFoundation functions that haven’t been annotated with reference-counting semantic information
A secondary known use-case is as a vehicle for creating a COpaquePointer containing a reference’s bits, e.g. for when you need to pass a reference through C APIs that use “void*” as a universal “give me some info and I’ll give it back to your callback” mechanism.

We saw several problems with Unmanaged that we wanted to fix:
It was poorly-named (the reference is managed by somebody, we just aren't representing that management in the type system).
Its interface was much broader than it needs to be to cover the use-cases
The purpose of many of its APIs was unclear
Its documentation was vague and hard to understand.
It didn’t establish a maximally-safe usage pattern for handling the results of un-annotated CoreFoundation functions.

The code for the proposed replacement, called UnsafeReference, is here <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift>, and a commit that updates Swift to use it is here <https://github.com/dabrahams/swift/commit/6eb86b48d150342709da3f3be9c738df23382866>.

Maximally Safe Usage

The recommended usage pattern for handling an UnsafeReference<T> returned by a function CFSomething is to always use the T instance produced by one of the forms:

    CFSomething(arguments…).release() // when the result is returned at +1

or

    CFSomething(arguments…).object // when the result is returned at +0

In other words, turn the UnsafeReference<T> into a safe T as quickly as possible, and never store the UnsafeReference<T> in a variable so that it can’t be (mis)used thereafter.

Points of Discussion

We’re interested in any feedback you might have, but there are a few points we’d especially like to address:

The name of the release() method has been contentious.
:+1:: Documentation—or naming conventions such as the “create rule <https://developer.apple.com/library/mac/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/Concepts/Ownership.html>”—normally says something like “you are responsible for releasing the result” in those cases where release() must be called, so there’s a very direct way to know which variant of the recommended usage pattern to employ.
:-1:: Some people who are very familiar with existing manual retain/release programming find the recommended usage pattern really counter-intuitive because they're “using something after calling release on it,” which one never does in Objective-C.
The alternative names we’ve been able to think of so far are verbose, clumsy, and don’t match up with anything in the documentation of the called function, so this seems like a really hard naming problem. Better ideas from the community would be most welcome here.
We’re not sure about the terminology <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift#L27> (Unretained/Retained/Released) used to precisely describe the semantics of UnsafeReference. We’d like to know if these terms make sense to you or whether you have better ideas.
We want to know whether the usage pattern recommended above works for you.
We want to know if the API is sufficiently broad or if there are things you currently get—and need—from Unmanaged that we’ve left out.

Thanks in advance,

-Dave

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

-Dave

-Dave

···

On Dec 17, 2015, at 6:23 PM, T.J. Usiyan <griotspeak@gmail.com> wrote:
On Thu, Dec 17, 2015 at 9:13 PM, Dave Abrahams <dabrahams@apple.com <mailto:dabrahams@apple.com>> wrote:

On Dec 17, 2015, at 5:52 PM, T.J. Usiyan <griotspeak@gmail.com <mailto:griotspeak@gmail.com>> wrote:
On Thu, Dec 17, 2015 at 8:37 PM, Dave Abrahams via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:


(Joe Groff) #12

I'm open to changing how we import void* from C. I think it's pretty important that UnsafeReference's conversion APIs match the C importer's behavior in any case.

-Joe

···

On Dec 19, 2015, at 1:09 PM, Dave Abrahams via swift-evolution <swift-evolution@swift.org> wrote:

Please see my comment here <https://github.com/apple/swift-evolution/pull/44#issuecomment-165902471>


(Janosch Hildebrand) #13

...

I don't see any point in having "manuallyRelease()" if we already have "release()"; they'd do the same thing if you dropped the return value.

I like the proposed idea of having two separate types for handling unannotated CF APIs and MRC which would also nicely resolve this issue.

Would a separate type for MRC also fall under this proposal or would that require a separate proposal?

And speaking of a separate type for MRC, how about `ManagedReference` as a name? Seems much better than `Unmanaged`, nicely contrasts with `UnsafeReference` and `ManuallyManagedReference` is a bit of a mouthful...

I don't think this use case even needs to be described in the documentation for `UnsafeReference` and it's fine if its use is very much discouraged.

Personally I prefer the proposed `manuallyRetain()`/`manuallyRelease()` over plain `retain()`/`release()` as it clearly separates the returning and more generally applicable `release()` from the MRC methods. `retain()` would probably also have to return the object which would interfere with the max safe usage pattern.

I don't understand your last sentence; care to clarify?

My main reason for preferring `manuallyRetain()`/`manuallyRelease()` over `retain()`/`release()` would be that the former would *not* return the object, thus more cleanly separating them from the current `release()` which returns the object to be used from now on, with the `UnsafeReference` to be discarded at that point.

I just think it might be more confusing to also use `release()` for MRC and also introducing `retain()` would only exacerbate the issue. For symmetry reasons `retain()` would likely also return the object. That would make it very similar to `release()` and `.object` which it really shouldn't be as it shouldn't ever be used for handling object from unannotated CF APIs.

I think having a third method/property with a very similar signature would likely confusion regarding the "Maximally Safe Usage" pattern you described.

But as mentioned above I would actually prefer having two separate types which would also make this a non-issue.

As Joe mentioned, `Unmanaged` has a use for manual ref counting beyond immediate transfer from un-annotated APIs.

I have used it for performance reasons myself (~ twice) and while I think it's a pretty small use case there isn't really any alternative.
If it would help I can also describe my use-cases in more detail.

Yes please!

One place I used Unmanaged is in a small project where I experiment with binary heaps in Swift. I've put the project on Github --(https://github.com/Jnosh/SwiftBinaryHeapExperiments) but basically I'm using `Unmanaged` in two places here:

1) Testing the 'overhead' of (A)RC.
Basically comparing the performance of using ARC-managed objects in the heaps vs. using 'unmanaged' objects. In Swift 1.2 the difference was still ~2x but with Swift 2+ it's likely approaching the cost of the retain/release when entering and exiting the collection.

Now this could also be accomplished using `unowned(unsafe)` but `Unmanaged` has some minor advantages:
  a) I can keep the objects alive without keeping them in a separate collection. Not a big issue here since I'm doing that anyway but I also find that `Unmanaged` makes it clearer that & how the objects are (partly) manually managed.
  b) I had previously experimented with using `unowned(unsafe)` for this purpose but found that `Unmanaged` performed better. However, that was in a more complex example and in the Swift 1.2 era. A quick test indicates that in this case and with Swift 2.1 `unowned(unsafe)` and `Unmanaged` perform about equally.

2) A (object only) binary heap that uses `Unmanaged` internally
Not much practical use either in this case since the compiler seems to do quite well by itself but still a somewhat interesting exercise.
`Unmanaged` is pretty much required here to make CoW work by manually retaining the objects.

The other project was a simple 2D sprite engine (think a simplified version of SpriteKit) I experimented with about a year ago.
Textures and Shaders were abstracted as value types privately backed by reference types that managed the underlying OpenGL objects, i.e. destroy the OpenGL texture object on deinit, etc...

I found this to be quite nice to use but ARC overhead during batching & rendering amounted to something like 20-30% of CPU time IIRC. (This was under Swift 1.2 and with WMO). Using `Unmanaged` was one of the things I played around with to get around this and it worked very well.
The `Unmanaged` instances were created when draw commands are submitted to the renderer so they were only used inside the rendering pipeline.
I eventually switched to using the OpenGL names (i.e. UInts) directly inside the renderer since they are already available anyway but that also requires extra logic to ensure the resources are not destroyed prematurely (e.g. retaining the object until the end of the frame or delaying the cleanup of the OpenGL resources until the end of the frame, ...). In many ways it's quite a bit messier than just using `Unmanaged`.

I don't think these are particularly great examples and I could certainly live without 'native' MRC but ultimately I think it's an interesting capability so I'd like to keep it around. Although I'd be in favor of keeping it out of the stdlib but I don't think that's really an option just yet...

It would also be interesting to be able to do the same with indirect enum instances and closures but it's not like I have a particular use case for that :wink:

- Janosch

···

On 19 Dec 2015, at 22:09, Dave Abrahams <dabrahams@apple.com> wrote:

On Dec 18, 2015, at 6:18 PM, Janosch Hildebrand via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:


(Dave Abrahams) #14

If this can be used as any reference, I wrote some code to help interoperate with Python in C++.

BTDT; see http://boost.org/libs/python :slight_smile:

The Python API inconsistently returns references with a +0 or +1 count. I use macros called TAKEREF (when the object is returned with a +1 count) and ADDREF (when it's returned with +0) to create smart wrappers. The wrapper will always decrement the reference count during destruction.

I find it unambiguous: you need to add a reference if you're going to take one away at destruction, so ADD; but if the object is returned with a +1 count, you can just TAKE the reference. It coincidentally does not borrow terminology from Objective-C's memory management system.

But it does tread on an area we’re trying very hard to clear up in Swift’s use of terminology: what does the word “take” mean? In fact the “take” problem was partly caused by the existing design of Unmanaged. Also, while unambiguous, it’s a very low-level description of what’s happening, that doesn’t much help the user reading the documentation of a CF function to answer the “which one do I use?” question unless s/he’s already experienced with how to map what it says in that docs onto the use of ADDREF/TAKEREF.

···

On Dec 17, 2015, at 7:32 PM, Félix Cloutier <felixcca@yahoo.ca> wrote:

Le 17 déc. 2015 à 21:23:56, T.J. Usiyan via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> a écrit :

I think I see what you mean about parity. `.object` can be called multiple times, then? Why not make `release()` slightly more verbose since it should only be called once anyway? Something along the lines of `.releaseAndReturnObject` or … something slightly less verbose.

My main point of contention with `.release()` is that it has the *exact* same name as a method from the MRC strategy. Maybe this is a silly point, but this overlap could further complicate teaching how ARC works and in what ways it is based on MRC conventions. I am not of the opinion that ARC is fundamentally more difficult to understand than MRC, but I do believe that it takes a very particular kind of faith now that we don't get to manually write the retains and releases. This is completely worth it, in my opinion, but I want to avoid making it *more* confusing to explain what ARC doing at compile time.

TJ

On Thu, Dec 17, 2015 at 9:13 PM, Dave Abrahams <dabrahams@apple.com <mailto:dabrahams@apple.com>> wrote:
Thanks very much for the quick feedback TJ,

On Dec 17, 2015, at 5:52 PM, T.J. Usiyan <griotspeak@gmail.com <mailto:griotspeak@gmail.com>> wrote:

Hello Dave,

I like this change and think that it will help clarify the purpose of the type. As I was reading, the only concern that I had was the name. Could you please provide some of the names that you all have considered

Honestly, I am sorry to say, we did that exercise almost a month ago and I don’t remember the ones we discussed.

so that we can avoid suggesting the same things? My suggestion is

    CFSomething(arguments…).retainedObject() // when the result is returned at +1

or

    CFSomething(arguments…).unretainedObject() // when the result is returned at +0

on the premise that the important bit of information is whether or not the object is already retained. No matter what names are chosen, that is the data which determines which method to call. `retainedObject | unretainedObject`, `takeRetainedObject | takeUnretainedObject`, or `retained | unretained` all seem like viable options (that you have probably considered).

Some issues with these names:

The “ed/ing” rule <https://swift.org/documentation/api-design-guidelines.html#be-grammatical> makes these names suggest that the accessors are idempotent, but the first one must be called exactly once. That name should really be an active verb since it is state-changing.
“retainedObject” also suggests that it’s returning some underlying object after retaining it, which is almost the opposite of what that API does… and vice-versa for “unretainedObject"
Also, the object “has been retained” in all cases, or it would have been deallocated. The question is whether the object would leak if we fail to call release on it
Nothing in these names connect them to what the documentation says about the functions that return Unmanaged, so it’s hard to know which one to call
The second API is objectively safer than the first one (which causes undefined behavior when overused and only leaks when underused). The API I proposed makes it clear that they are not peers, where yours implies parity—though I am of two minds about the value of representing the lack of parity.

You may legitimately argue that any of these concerns are unimportant, but those are the ones that come up for me.

TJ

On Thu, Dec 17, 2015 at 8:37 PM, Dave Abrahams via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
Hi Everybody,

We’ve been working on a rewrite of the Unmanaged<T> component, and are soliciting comments. First, a little background:

Unmanaged <https://developer.apple.com/library/ios/documentation/Swift/Conceptual/BuildingCocoaApps/WorkingWithCocoaDataTypes.html#//apple_ref/doc/uid/TP40014216-CH6-ID79> is primarily used as a return type from imported CoreFoundation functions that haven’t been annotated with reference-counting semantic information
A secondary known use-case is as a vehicle for creating a COpaquePointer containing a reference’s bits, e.g. for when you need to pass a reference through C APIs that use “void*” as a universal “give me some info and I’ll give it back to your callback” mechanism.

We saw several problems with Unmanaged that we wanted to fix:
It was poorly-named (the reference is managed by somebody, we just aren't representing that management in the type system).
Its interface was much broader than it needs to be to cover the use-cases
The purpose of many of its APIs was unclear
Its documentation was vague and hard to understand.
It didn’t establish a maximally-safe usage pattern for handling the results of un-annotated CoreFoundation functions.

The code for the proposed replacement, called UnsafeReference, is here <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift>, and a commit that updates Swift to use it is here <https://github.com/dabrahams/swift/commit/6eb86b48d150342709da3f3be9c738df23382866>.

Maximally Safe Usage

The recommended usage pattern for handling an UnsafeReference<T> returned by a function CFSomething is to always use the T instance produced by one of the forms:

    CFSomething(arguments…).release() // when the result is returned at +1

or

    CFSomething(arguments…).object // when the result is returned at +0

In other words, turn the UnsafeReference<T> into a safe T as quickly as possible, and never store the UnsafeReference<T> in a variable so that it can’t be (mis)used thereafter.

Points of Discussion

We’re interested in any feedback you might have, but there are a few points we’d especially like to address:

The name of the release() method has been contentious.
:+1:: Documentation—or naming conventions such as the “create rule <https://developer.apple.com/library/mac/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/Concepts/Ownership.html>”—normally says something like “you are responsible for releasing the result” in those cases where release() must be called, so there’s a very direct way to know which variant of the recommended usage pattern to employ.
:-1:: Some people who are very familiar with existing manual retain/release programming find the recommended usage pattern really counter-intuitive because they're “using something after calling release on it,” which one never does in Objective-C.
The alternative names we’ve been able to think of so far are verbose, clumsy, and don’t match up with anything in the documentation of the called function, so this seems like a really hard naming problem. Better ideas from the community would be most welcome here.
We’re not sure about the terminology <https://github.com/dabrahams/swift/blob/6eb86b48d150342709da3f3be9c738df23382866/stdlib/public/core/UnsafeReference.swift#L27> (Unretained/Retained/Released) used to precisely describe the semantics of UnsafeReference. We’d like to know if these terms make sense to you or whether you have better ideas.
We want to know whether the usage pattern recommended above works for you.
We want to know if the API is sufficiently broad or if there are things you currently get—and need—from Unmanaged that we’ve left out.

Thanks in advance,

-Dave

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

-Dave

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

-Dave


(Dave Abrahams) #15

Back to this after a long hiatus, sorry.

...

I don't see any point in having "manuallyRelease()" if we already
have "release()"; they'd do the same thing if you dropped the return
value.

I like the proposed idea of having two separate types for handling
unannotated CF APIs and MRC which would also nicely resolve this
issue.

Would a separate type for MRC also fall under this proposal or would
that require a separate proposal?

Considering that we're just RFC'ing here, we can certainly talk about
that.

And speaking of a separate type for MRC, how about `ManagedReference`
as a name? Seems much better than `Unmanaged`, nicely contrasts with
`UnsafeReference` and `ManuallyManagedReference` is a bit of a
mouthful...

I think we want “managed” to mean “managed for you,” not “managed by
you.” It's also quite unsafe because you can overrelease it, etc., so
it would have to have “unsafe” in the name somewhere I think.

I don't think this use case even needs to be described in the
documentation for `UnsafeReference` and it's fine if its use is
very much discouraged.

Personally I prefer the proposed
`manuallyRetain()`/`manuallyRelease()` over plain
`retain()`/`release()` as it clearly separates the returning and
more generally applicable `release()` from the MRC
methods. `retain()` would probably also have to return the object
which would interfere with the max safe usage pattern.

I don't understand your last sentence; care to clarify?

My main reason for preferring `manuallyRetain()`/`manuallyRelease()`
over `retain()`/`release()` would be that the former would *not*
return the object, thus more cleanly separating them from the current
`release()` which returns the object to be used from now on, with the
`UnsafeReference` to be discarded at that point.

I just think it might be more confusing to also use `release()` for
MRC and also introducing `retain()` would only exacerbate the
issue. For symmetry reasons `retain()` would likely also return the
object.

There might be other reasons to do it, but I don't think symmetry is
necessarily a design goal here.

That would make it very similar to `release()` and `.object` which it
really shouldn't be as it shouldn't ever be used for handling object
from unannotated CF APIs.

I think having a third method/property with a very similar signature
would likely confusion regarding the "Maximally Safe Usage" pattern
you described.

But as mentioned above I would actually prefer having two separate
types which would also make this a non-issue.

Questions:

1. How would these types interact? Does one need to be able to convert
   between them liberally, or is it sufficient to use strong references
   as the common currency?

2. Do you really want a type at all? Why not just retain() and
   release() as free functions?

As Joe mentioned, `Unmanaged` has a use for manual ref counting
beyond immediate transfer from un-annotated APIs.

I have used it for performance reasons myself (~ twice) and while I
think it's a pretty small use case there isn't really any
alternative.
If it would help I can also describe my use-cases in more detail.

Yes please!

One place I used Unmanaged is in a small project where I experiment
with binary heaps in Swift. I've put the project on Github
--(https://github.com/Jnosh/SwiftBinaryHeapExperiments) but basically
I'm using `Unmanaged` in two places here:

1) Testing the 'overhead' of (A)RC.
Basically comparing the performance of using ARC-managed objects in
the heaps vs. using 'unmanaged' objects. In Swift 1.2 the difference
was still ~2x but with Swift 2+ it's likely approaching the cost of
the retain/release when entering and exiting the collection.

Now this could also be accomplished using `unowned(unsafe)` but
`Unmanaged` has some minor advantages:
  a) I can keep the objects alive without keeping them in a
separate collection. Not a big issue here since I'm doing that anyway
but I also find that `Unmanaged` makes it clearer that & how the
objects are (partly) manually managed.
  b) I had previously experimented with using `unowned(unsafe)`
for this purpose but found that `Unmanaged` performed better. However,
that was in a more complex example and in the Swift 1.2 era. A quick
test indicates that in this case and with Swift 2.1 `unowned(unsafe)`
and `Unmanaged` perform about equally.

They should. unowned(unsafe) var T is essentially just an
UnsafePointer. unowned/unowned(safe) do incur reference-counting cost
in exchange for their safety.

2) A (object only) binary heap that uses `Unmanaged` internally
Not much practical use either in this case since the compiler seems to
do quite well by itself but still a somewhat interesting exercise.
`Unmanaged` is pretty much required here to make CoW work by manually
retaining the objects.

It's hard for me to imagine why that would be the case. Would I have
needed to use Unmanaged in implementing Arrays of objects, if it were?

The other project was a simple 2D sprite engine (think a simplified
version of SpriteKit) I experimented with about a year ago.
Textures and Shaders were abstracted as value types privately backed
by reference types that managed the underlying OpenGL objects,
i.e. destroy the OpenGL texture object on deinit, etc...

I found this to be quite nice to use but ARC overhead during batching
& rendering amounted to something like 20-30% of CPU time IIRC. (This
was under Swift 1.2 and with WMO). Using `Unmanaged` was one of the
things I played around with to get around this and it worked very
well.

Another case where you can use unowned(unsafe), is it not?

The `Unmanaged` instances were created when draw commands are
submitted to the renderer so they were only used inside the rendering
pipeline.
I eventually switched to using the OpenGL names (i.e. UInts) directly
inside the renderer since they are already available anyway but that
also requires extra logic to ensure the resources are not destroyed
prematurely (e.g. retaining the object until the end of the frame or
delaying the cleanup of the OpenGL resources until the end of the
frame, ...). In many ways it's quite a bit messier than just using
`Unmanaged`.

I don't see how Unmanaged could have been less messy; don't you still
need a strong reference somewhere to ensure the lifetime?

I don't think these are particularly great examples and I could
certainly live without 'native' MRC but ultimately I think it's an
interesting capability so I'd like to keep it around.
Although I'd be in favor of keeping it out of the stdlib but I don't
think that's really an option just yet...

It would also be interesting to be able to do the same with indirect
enum instances and closures but it's not like I have a particular use
case for that :wink:

I don't understand what you might be hinting at here.

···

on Tue Dec 29 2015, Janosch Hildebrand <jnosh-AT-jnosh.com> wrote:

On 19 Dec 2015, at 22:09, Dave Abrahams <dabrahams@apple.com> wrote:

On Dec 18, 2015, at 6:18 PM, Janosch Hildebrand via swift-evolution >>> <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> >>> wrote:

--
-Dave


(Brent Royal-Gordon) #16

Also, while unambiguous, it’s a very low-level description of what’s happening, that doesn’t much help the user reading the documentation of a CF function to answer the “which one do I use?” question unless s/he’s already experienced with how to map what it says in that docs onto the use of ADDREF/TAKEREF.

I wonder if, instead of talking about retained/unretained, we should talk about whether the call “created" or “retrieved" the object it’s returning. If it created it, the object is +1; if it retrieved it, the object is +0.

···

--
Brent Royal-Gordon
Architechies


(Janosch Hildebrand) #17

Back to this after a long hiatus, sorry.

No problem, you've got bigger fish to fry. :slight_smile:

And thanks again for your time and questions. Having someone ask
the hard questions really helps me reevaluate and better understand
my own thoughts and opinions.

...

I don't see any point in having "manuallyRelease()" if we already
have "release()"; they'd do the same thing if you dropped the return
value.

I like the proposed idea of having two separate types for handling
unannotated CF APIs and MRC which would also nicely resolve this
issue.

Would a separate type for MRC also fall under this proposal or would
that require a separate proposal?

Considering that we're just RFC'ing here, we can certainly talk about
that.

And speaking of a separate type for MRC, how about `ManagedReference`
as a name? Seems much better than `Unmanaged`, nicely contrasts with
`UnsafeReference` and `ManuallyManagedReference` is a bit of a
mouthful...

I think we want “managed” to mean “managed for you,” not “managed by
you.” It's also quite unsafe because you can overrelease it, etc., so
it would have to have “unsafe” in the name somewhere I think.

That makes sense. Something like `UnsafeReferenceCountedPointer` but
shorter?

I don't think this use case even needs to be described in the
documentation for `UnsafeReference` and it's fine if its use is
very much discouraged.

Personally I prefer the proposed
`manuallyRetain()`/`manuallyRelease()` over plain
`retain()`/`release()` as it clearly separates the returning and
more generally applicable `release()` from the MRC
methods. `retain()` would probably also have to return the object
which would interfere with the max safe usage pattern.

I don't understand your last sentence; care to clarify?

My main reason for preferring `manuallyRetain()`/`manuallyRelease()`
over `retain()`/`release()` would be that the former would *not*
return the object, thus more cleanly separating them from the current
`release()` which returns the object to be used from now on, with the
`UnsafeReference` to be discarded at that point.

I just think it might be more confusing to also use `release()` for
MRC and also introducing `retain()` would only exacerbate the
issue. For symmetry reasons `retain()` would likely also return the
object.

There might be other reasons to do it, but I don't think symmetry is
necessarily a design goal here.

That would make it very similar to `release()` and `.object` which it
really shouldn't be as it shouldn't ever be used for handling object
from unannotated CF APIs.

I think having a third method/property with a very similar signature
would likely confusion regarding the "Maximally Safe Usage" pattern
you described.

But as mentioned above I would actually prefer having two separate
types which would also make this a non-issue.

Questions:

1. How would these types interact? Does one need to be able to convert
  between them liberally, or is it sufficient to use strong references
  as the common currency?

If we were to have two separate types I think it would be more than fine to
use strong references as a go-between. The use cases for the two types are
so different that I doubt it would be an issue.

2. Do you really want a type at all? Why not just retain() and
  release() as free functions?

I assume these would be unsafeRetain() and unsafeRelease() :wink:

But yeah, that would work as well and might be a nicer solution overall.
(And you could easily create your own type from these + unowned(unsafe))

The downside I see is that being free functions and working with
AnyObject makes them much more discoverable than being hidden
inside some other type. But given the other unsafe* free functions
that's already exists that might be fine.

Also having a predefined wrapper type has some use, e.g. when you
want to store your unowned(unsafe) objects into some collection.
But I'd guess you'll end up with a dedicated wrapper type anyway
in most circumstances where this is an issue so it's probably OK.
And this gets rid of having a separate type that also plays the role
of unowned(unsafe) which seems like a plus.

I have some more answers below but I'll summarize my opinion here.
Preferences (in descending order):

1) unsafeRetain() + unsafeRelease() + unowned(unsafe)
2) "UnsafeReferenceCountedPointer"
3) No dedicated functionality so just abuse UnsafeReference instead of
    Unmanaged

I think ultimately it's a question of whether we want to expose the
reference counting implementation to manual use...

Is it something I can live without? Absolutely.
Is it something I would use often? Absolutely not.
But then again, it is going to be exposed through UnsafeReference anyway.

And having access to a solid manual reference counting solution
that integrates very well with the rest of the language is kinda neat in
my opinion. And the integration with ARC is something an external
library cannot provide without becoming even more "hacky".
And I don't think it's any more dangerous than any other manual
memory management we have access to.

I also wonder if it has some minor use for learning/teaching.
Yes, ARC is great because most of the time you don't need to think
about this but if you're trying to understand reference counting it's
kinda nice to be able to actually interact and play around with it.
Then again I'm the kind of person that likes doing that but YMMV...

As Joe mentioned, `Unmanaged` has a use for manual ref counting
beyond immediate transfer from un-annotated APIs.

I have used it for performance reasons myself (~ twice) and while I
think it's a pretty small use case there isn't really any
alternative.
If it would help I can also describe my use-cases in more detail.

Yes please!

One place I used Unmanaged is in a small project where I experiment
with binary heaps in Swift. I've put the project on Github
--(https://github.com/Jnosh/SwiftBinaryHeapExperiments) but basically
I'm using `Unmanaged` in two places here:

1) Testing the 'overhead' of (A)RC.
Basically comparing the performance of using ARC-managed objects in
the heaps vs. using 'unmanaged' objects. In Swift 1.2 the difference
was still ~2x but with Swift 2+ it's likely approaching the cost of
the retain/release when entering and exiting the collection.

Now this could also be accomplished using `unowned(unsafe)` but
`Unmanaged` has some minor advantages:
  a) I can keep the objects alive without keeping them in a
separate collection. Not a big issue here since I'm doing that anyway
but I also find that `Unmanaged` makes it clearer that & how the
objects are (partly) manually managed.
  b) I had previously experimented with using `unowned(unsafe)`
for this purpose but found that `Unmanaged` performed better. However,
that was in a more complex example and in the Swift 1.2 era. A quick
test indicates that in this case and with Swift 2.1 `unowned(unsafe)`
and `Unmanaged` perform about equally.

They should. unowned(unsafe) var T is essentially just an
UnsafePointer. unowned/unowned(safe) do incur reference-counting cost
in exchange for their safety.

I'll come back to this further down.

2) A (object only) binary heap that uses `Unmanaged` internally
Not much practical use either in this case since the compiler seems to
do quite well by itself but still a somewhat interesting exercise.
`Unmanaged` is pretty much required here to make CoW work by manually
retaining the objects.

It's hard for me to imagine why that would be the case. Would I have
needed to use Unmanaged in implementing Arrays of objects, if it were?

Sorry, I wasn't clear enough. I (ab)use Unmanaged for two different reasons here.

1) To have a performance baseline where the ARC overhead inside the collection
is essentially zero beyond the mandatory retain on insert, i.e. as if the compiler was
able to eliminate all (redundant) retains and releases.

One part of this is exempting the objects from ARC which is is done by storing the
elements in Unmanaged instances but a wrapper type using unowned(unsafe)
would work just as well.

However, I still need a strong reference to the objects to keep them alive. Using a
separate data structure would work but that has a space, time and code complexity
cost.
Instead I use Unmanaged to manually retain the objects on insert and release on
removal. unowned cannot do that on its own hence the need for something like
unsafeRetain() & unsafeRelease().

2) I then abuse Unmanaged's capabilities a second time to retain the elements
when the collection is copied (which would happen 'automatically' with ARC).

Btw, with Swift 2.2 under WMO the performance of a normal ManagedBuffer
is on par with this "hack". Go Swift team!

The other project was a simple 2D sprite engine (think a simplified
version of SpriteKit) I experimented with about a year ago.
Textures and Shaders were abstracted as value types privately backed
by reference types that managed the underlying OpenGL objects,
i.e. destroy the OpenGL texture object on deinit, etc...

I found this to be quite nice to use but ARC overhead during batching
& rendering amounted to something like 20-30% of CPU time IIRC. (This
was under Swift 1.2 and with WMO). Using `Unmanaged` was one of the
things I played around with to get around this and it worked very
well.

Another case where you can use unowned(unsafe), is it not?

Indeed, and that was what I originally tried to use.
Ultimately i settled on Unmanaged however. Now it's been a long time and
I don't recall the exact details so take this with a grain of salt:

One reason certainly was that I ended up needing Unmanaged anyway
to perform manual retain & releases at which point why not also use it
for 'storage'...

But I also vaguely recall that Unmanaged had more of a performance impact.
Now one possibility is that there was some issue with unowned(unsafe) (this
was with 1.2β1) but much more likely is that Unmanaged was easier to apply
consistently and correctly.
e.g. assume you have some struct that contains an unowned(unsafe) variable.
Now if you extract that into a local variable you add a perhaps unwanted
retain/release so you might need to mark the local variable as unowned(unsafe)
as well, etc...
What I'm trying to say is that with unowned you need to be careful and considerate
with how you use it at all times since the 'obvious' thing generally leads to
retain/release.
Unmanaged is fine to pass around, store in a local variable, etc... and any ARC
related interactions are obvious because they manifest as method calls on the
Unmanaged instance.

For their main application, breaking retain cycles, weak and unowned work fine
because you want to retain the objects when they are not 'at rest'.
But if you want to avoid retains even when working with the objects, a type is
just a much more comfortable way to handle this.
Like I mentioned before, I imagine that in many cases you'll end up making a
custom wrapper anyway but it's something I'm a bit apprehensive about.

Still, I think unowned(unsafe) together with unsafeRetain() and unsafeRelease()
free functions makes for a nicer API and I don't think I can adequately judge it
beyond that. I've barely used this in it's current form have no real experience
with a potential future form and I hopefully won't use it (often) anyway.
So I think it's more than appropriate to prioritize the general API over making
this esoteric use case more comfortable to use.

The `Unmanaged` instances were created when draw commands are
submitted to the renderer so they were only used inside the rendering
pipeline.
I eventually switched to using the OpenGL names (i.e. UInts) directly
inside the renderer since they are already available anyway but that
also requires extra logic to ensure the resources are not destroyed
prematurely (e.g. retaining the object until the end of the frame or
delaying the cleanup of the OpenGL resources until the end of the
frame, ...). In many ways it's quite a bit messier than just using
`Unmanaged`.

I don't see how Unmanaged could have been less messy; don't you still
need a strong reference somewhere to ensure the lifetime?

Absolutely. You can retain the object directly through Unmanaged
(via passRetained() or retain()) and make the Unmanaged instance
a strong reference in effect.

Not a big difference to retaining by putting the objects in some container.
Just a different set of tradeoffs.

I don't think it's the best solution for this case but it's pretty simple. Retain
when creating the draw command, release when discarding the draw
command - nothing different than malloc/free.

Collecting the objects in some collection is likely to be a cleaner solution
and more efficient too, since you don't retain objects multiple times if they are
used multiple times in the same frame (which is likely for shaders, textures).
But then someone somewhere needs to manage this, and you need to
access that state when creating or submitting the draw command.

Or perhaps make sure the referenced resources stay valid until the frame
is drawn so you don't need to retain here at all but now you need to track
all the scene contents, etc...

Hopefully I don't come across as too petulant. :slight_smile:
I don't really want to argue in favor of or defend these approaches.
I'm merely trying to give some examples of when and what for I actually
used this stuff not to prove the merits of these cases but instead to argue
for the existence of better justified uses based on the same ideas.
Not sure if that makes any sense but there you go :slight_smile:

I don't think these are particularly great examples and I could
certainly live without 'native' MRC but ultimately I think it's an
interesting capability so I'd like to keep it around.
Although I'd be in favor of keeping it out of the stdlib but I don't
think that's really an option just yet...

It would also be interesting to be able to do the same with indirect
enum instances and closures but it's not like I have a particular use
case for that :wink:

I don't understand what you might be hinting at here.

Just that AFAIK closures and indirect enum instances also use ARCed
references under the hood. So in theory the could potentially also be
stored unowned and manually retained/released.

I just find it slightly interesting that with (Any)Objects certain things are
exposed (unsafeAddressOf, retain/release, ...) whereas with indirect enums
and closures they are not.

I don't want to imply that that would be a good idea and it would certainly
be hard, complicated, and annoying to implement with essentially n
benefit so I don't want to go anywhere with this other than the partial similarity.

Basically it's just my brain going:
"Oh look, some pyramids. Hmm, you could store these much more efficiently
if you stacked them up against each other" :wink:

- Janosch

···

On 21 Feb 2016, at 12:28, Dave Abrahams <dabrahams@apple.com> wrote:
on Tue Dec 29 2015, Janosch Hildebrand <jnosh-AT-jnosh.com> wrote:

On 19 Dec 2015, at 22:09, Dave Abrahams <dabrahams@apple.com> wrote:

On Dec 18, 2015, at 6:18 PM, Janosch Hildebrand via swift-evolution >>>> <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> >>>> wrote:

--
-Dave


(Dave Abrahams) #18

I think I’d need more detail in order to evaluate the idea. Are you suggesting changing the way some CF functions are documented? Some concrete examples would certainly help.

-Dave

···

On Dec 18, 2015, at 1:18 PM, Brent Royal-Gordon <brent@architechies.com> wrote:

Also, while unambiguous, it’s a very low-level description of what’s happening, that doesn’t much help the user reading the documentation of a CF function to answer the “which one do I use?” question unless s/he’s already experienced with how to map what it says in that docs onto the use of ADDREF/TAKEREF.

I wonder if, instead of talking about retained/unretained, we should talk about whether the call “created" or “retrieved" the object it’s returning. If it created it, the object is +1; if it retrieved it, the object is +0.


(Dave Abrahams) #19

And speaking of a separate type for MRC, how about `ManagedReference`
as a name? Seems much better than `Unmanaged`, nicely contrasts with
`UnsafeReference` and `ManuallyManagedReference` is a bit of a
mouthful...

I think we want “managed” to mean “managed for you,” not “managed by
you.” It's also quite unsafe because you can overrelease it, etc., so
it would have to have “unsafe” in the name somewhere I think.

That makes sense. Something like `UnsafeReferenceCountedPointer` but
shorter?

Something like that. I'm not attached to it being shorter.

I don't think this use case even needs to be described in the
documentation for `UnsafeReference` and it's fine if its use is
very much discouraged.

Personally I prefer the proposed
`manuallyRetain()`/`manuallyRelease()` over plain
`retain()`/`release()` as it clearly separates the returning and
more generally applicable `release()` from the MRC
methods. `retain()` would probably also have to return the object
which would interfere with the max safe usage pattern.

I don't understand your last sentence; care to clarify?

My main reason for preferring `manuallyRetain()`/`manuallyRelease()`
over `retain()`/`release()` would be that the former would *not*
return the object, thus more cleanly separating them from the current
`release()` which returns the object to be used from now on, with the
`UnsafeReference` to be discarded at that point.

I just think it might be more confusing to also use `release()` for
MRC and also introducing `retain()` would only exacerbate the
issue. For symmetry reasons `retain()` would likely also return the
object.

There might be other reasons to do it, but I don't think symmetry is
necessarily a design goal here.

That would make it very similar to `release()` and `.object` which it
really shouldn't be as it shouldn't ever be used for handling object
from unannotated CF APIs.

I think having a third method/property with a very similar signature
would likely confusion regarding the "Maximally Safe Usage" pattern
you described.

But as mentioned above I would actually prefer having two separate
types which would also make this a non-issue.

Questions:

1. How would these types interact? Does one need to be able to convert
  between them liberally, or is it sufficient to use strong references
  as the common currency?

If we were to have two separate types I think it would be more than fine to
use strong references as a go-between. The use cases for the two types are
so different that I doubt it would be an issue.

2. Do you really want a type at all? Why not just retain() and
  release() as free functions?

I assume these would be unsafeRetain() and unsafeRelease() :wink:

Yes of course.

But yeah, that would work as well and might be a nicer solution overall.
(And you could easily create your own type from these + unowned(unsafe))

The downside I see is that being free functions and working with
AnyObject makes them much more discoverable than being hidden
inside some other type.

How discoverable *are* free functions, really, after all? If you want
we could nest them:

enum UnsafeManualReferenceCounting {
  static func retain(AnyObject) {...}
  static func release(AnyObject) {...}
}

But given the other unsafe* free functions that's already exists that
might be fine.

Yeah, IMO that would be reasonable.

Also having a predefined wrapper type has some use, e.g. when you
want to store your unowned(unsafe) objects into some collection.

Yeah, though it seems to me that wrapper should be UnsafeReference<T>.

But I'd guess you'll end up with a dedicated wrapper type anyway in
most circumstances where this is an issue so it's probably OK. And
this gets rid of having a separate type that also plays the role of
unowned(unsafe) which seems like a plus.

I have some more answers below but I'll summarize my opinion here.
Preferences (in descending order):

1) unsafeRetain() + unsafeRelease() + unowned(unsafe)
2) "UnsafeReferenceCountedPointer"
3) No dedicated functionality so just abuse UnsafeReference instead of
    Unmanaged

I think ultimately it's a question of whether we want to expose the
reference counting implementation to manual use...

Is it something I can live without? Absolutely.
Is it something I would use often? Absolutely not.
But then again, it is going to be exposed through UnsafeReference anyway.

And having access to a solid manual reference counting solution
that integrates very well with the rest of the language is kinda neat in
my opinion. And the integration with ARC is something an external
library cannot provide without becoming even more "hacky".
And I don't think it's any more dangerous than any other manual
memory management we have access to.

Okay. My current feeling about all this is that UnsafeReference should
have retain and release directly on it, for MRC.

The thing I'm really unclear about at this point is whether it's
reasonable to ask users to use “release()” for the maximally-safe usage
pattern, or if that's just too weird. My sense all along has been that
asking them to get used to it is a much better design choice than
expanding the API, especially when we don't really have a better name for
it than “release.” If a better name presented itself, that might change
the picture.

I also wonder if it has some minor use for learning/teaching.
Yes, ARC is great because most of the time you don't need to think
about this but if you're trying to understand reference counting it's
kinda nice to be able to actually interact and play around with it.
Then again I'm the kind of person that likes doing that but YMMV...

As long as you don't need to share anything across threads, it's easy
enough to build MRC using malloc, free, and a counter in each allocated
block, so this doesn't sound like a strong argument to me.

As Joe mentioned, `Unmanaged` has a use for manual ref counting
beyond immediate transfer from un-annotated APIs.

I have used it for performance reasons myself (~ twice) and while I
think it's a pretty small use case there isn't really any
alternative.
If it would help I can also describe my use-cases in more detail.

Yes please!

One place I used Unmanaged is in a small project where I experiment
with binary heaps in Swift. I've put the project on Github
--(https://github.com/Jnosh/SwiftBinaryHeapExperiments) but basically
I'm using `Unmanaged` in two places here:

1) Testing the 'overhead' of (A)RC.
Basically comparing the performance of using ARC-managed objects in
the heaps vs. using 'unmanaged' objects. In Swift 1.2 the difference
was still ~2x but with Swift 2+ it's likely approaching the cost of
the retain/release when entering and exiting the collection.

Now this could also be accomplished using `unowned(unsafe)` but
`Unmanaged` has some minor advantages:
  a) I can keep the objects alive without keeping them in a
separate collection. Not a big issue here since I'm doing that anyway
but I also find that `Unmanaged` makes it clearer that & how the
objects are (partly) manually managed.
  b) I had previously experimented with using `unowned(unsafe)`
for this purpose but found that `Unmanaged` performed better. However,
that was in a more complex example and in the Swift 1.2 era. A quick
test indicates that in this case and with Swift 2.1 `unowned(unsafe)`
and `Unmanaged` perform about equally.

They should. unowned(unsafe) var T is essentially just an
UnsafePointer. unowned/unowned(safe) do incur reference-counting cost
in exchange for their safety.

I'll come back to this further down.

2) A (object only) binary heap that uses `Unmanaged` internally
Not much practical use either in this case since the compiler seems to
do quite well by itself but still a somewhat interesting exercise.
`Unmanaged` is pretty much required here to make CoW work by manually
retaining the objects.

It's hard for me to imagine why that would be the case. Would I have
needed to use Unmanaged in implementing Arrays of objects, if it were?

Sorry, I wasn't clear enough. I (ab)use Unmanaged for two different reasons here.

1) To have a performance baseline where the ARC overhead inside the collection
is essentially zero beyond the mandatory retain on insert, i.e. as if the compiler was
able to eliminate all (redundant) retains and releases.

One part of this is exempting the objects from ARC which is is done by storing the
elements in Unmanaged instances but a wrapper type using unowned(unsafe)
would work just as well.

However, I still need a strong reference to the objects to keep them alive. Using a
separate data structure would work but that has a space, time and code complexity
cost.
Instead I use Unmanaged to manually retain the objects on insert and release on
removal. unowned cannot do that on its own hence the need for something like
unsafeRetain() & unsafeRelease().

OK.

2) I then abuse Unmanaged's capabilities a second time to retain the elements
when the collection is copied (which would happen 'automatically' with
ARC).

Btw, with Swift 2.2 under WMO the performance of a normal ManagedBuffer
is on par with this "hack". Go Swift team!

:slight_smile:

The other project was a simple 2D sprite engine (think a simplified
version of SpriteKit) I experimented with about a year ago.
Textures and Shaders were abstracted as value types privately backed
by reference types that managed the underlying OpenGL objects,
i.e. destroy the OpenGL texture object on deinit, etc...

I found this to be quite nice to use but ARC overhead during batching
& rendering amounted to something like 20-30% of CPU time IIRC. (This
was under Swift 1.2 and with WMO). Using `Unmanaged` was one of the
things I played around with to get around this and it worked very
well.

Another case where you can use unowned(unsafe), is it not?

Indeed, and that was what I originally tried to use.
Ultimately i settled on Unmanaged however. Now it's been a long time and
I don't recall the exact details so take this with a grain of salt:

One reason certainly was that I ended up needing Unmanaged anyway
to perform manual retain & releases at which point why not also use it
for 'storage'...

But I also vaguely recall that Unmanaged had more of a performance impact.
Now one possibility is that there was some issue with unowned(unsafe) (this
was with 1.2.β1) but much more likely is that Unmanaged was easier to apply
consistently and correctly.
e.g. assume you have some struct that contains an unowned(unsafe) variable.
Now if you extract that into a local variable you add a perhaps unwanted
retain/release so you might need to mark the local variable as unowned(unsafe)
as well, etc...

Right; makes sense.

What I'm trying to say is that with unowned you need to be careful and
considerate with how you use it at all times since the 'obvious' thing
generally leads to retain/release.

That's safer, though, but in this case you're more concerned about performance.

Unmanaged is fine to pass around, store in a local variable,
etc... and any ARC related interactions are obvious because they
manifest as method calls on the Unmanaged instance.

For their main application, breaking retain cycles, weak and unowned work fine
because you want to retain the objects when they are not 'at rest'.
But if you want to avoid retains even when working with the objects, a type is
just a much more comfortable way to handle this.
Like I mentioned before, I imagine that in many cases you'll end up making a
custom wrapper anyway but it's something I'm a bit apprehensive about.

Still, I think unowned(unsafe) together with unsafeRetain() and unsafeRelease()
free functions makes for a nicer API and I don't think I can adequately judge it
beyond that. I've barely used this in it's current form have no real experience
with a potential future form and I hopefully won't use it (often) anyway.
So I think it's more than appropriate to prioritize the general API over making
this esoteric use case more comfortable to use.

I think you've already talked me into the idea that we need a type for
transporting manually reference-counted references.

The `Unmanaged` instances were created when draw commands are
submitted to the renderer so they were only used inside the rendering
pipeline.
I eventually switched to using the OpenGL names (i.e. UInts) directly
inside the renderer since they are already available anyway but that
also requires extra logic to ensure the resources are not destroyed
prematurely (e.g. retaining the object until the end of the frame or
delaying the cleanup of the OpenGL resources until the end of the
frame, ...). In many ways it's quite a bit messier than just using
`Unmanaged`.

I don't see how Unmanaged could have been less messy; don't you still
need a strong reference somewhere to ensure the lifetime?

Absolutely. You can retain the object directly through Unmanaged
(via passRetained() or retain()) and make the Unmanaged instance
a strong reference in effect.

Right. Thanks for refreshing my ability to think about these issues
again :slight_smile:

Not a big difference to retaining by putting the objects in some container.
Just a different set of tradeoffs.

I don't think it's the best solution for this case but it's pretty simple. Retain
when creating the draw command, release when discarding the draw
command - nothing different than malloc/free.

Collecting the objects in some collection is likely to be a cleaner solution
and more efficient too, since you don't retain objects multiple times if they are
used multiple times in the same frame (which is likely for shaders, textures).
But then someone somewhere needs to manage this, and you need to
access that state when creating or submitting the draw command.

Or perhaps make sure the referenced resources stay valid until the frame
is drawn so you don't need to retain here at all but now you need to track
all the scene contents, etc...

Hopefully I don't come across as too petulant. :slight_smile:

Not a bit.

I don't really want to argue in favor of or defend these approaches.
I'm merely trying to give some examples of when and what for I actually
used this stuff not to prove the merits of these cases but instead to argue
for the existence of better justified uses based on the same ideas.
Not sure if that makes any sense but there you go :slight_smile:

Thanks, it's been very helpful.

I don't think these are particularly great examples and I could
certainly live without 'native' MRC but ultimately I think it's an
interesting capability so I'd like to keep it around.
Although I'd be in favor of keeping it out of the stdlib but I don't
think that's really an option just yet...

It would also be interesting to be able to do the same with indirect
enum instances and closures but it's not like I have a particular use
case for that :wink:

I don't understand what you might be hinting at here.

Just that AFAIK closures and indirect enum instances also use ARCed
references under the hood. So in theory the could potentially also be
stored unowned and manually retained/released.

I just find it slightly interesting that with (Any)Objects certain things are
exposed (unsafeAddressOf, retain/release, ...) whereas with indirect enums
and closures they are not.

I don't want to imply that that would be a good idea and it would certainly
be hard, complicated, and annoying to implement with essentially n
benefit so I don't want to go anywhere with this other than the partial similarity.

Basically it's just my brain going:
"Oh look, some pyramids. Hmm, you could store these much more efficiently
if you stacked them up against each other" :wink:

Say, you must be some kind of engineer or something! :wink:

···

on Mon Feb 22 2016, Janosch Hildebrand <jnosh-AT-jnosh.com> wrote:

--
-Dave


(Janosch Hildebrand) #20

But yeah, that would work as well and might be a nicer solution overall.
(And you could easily create your own type from these + unowned(unsafe))

The downside I see is that being free functions and working with
AnyObject makes them much more discoverable than being hidden
inside some other type.

How discoverable *are* free functions, really, after all? If you want
we could nest them:

enum UnsafeManualReferenceCounting {
static func retain(AnyObject) {...}
static func release(AnyObject) {...}
}

I try to have a general overview of the standard library in terms of what is
available and how it is structured. And the stdlib is compact enough that
that is feasible.

Now a type naturally scopes and encloses functionality so I don't (need to)
have as precise a mental overview over the type's members & methods.
Knowing the type or how to find it also means that 'lookups' via auto-
completion or documentation will produce a relatively narrow set of results
for me to process.

Finding a free function that way is much harder. The enclosing scope is
much larger so autocompletion for example becomes pretty much useless
unless you already know what you are looking for. This is even worse if you
have Darwin imported (unless you do 'Swift.').

So I want a decent overview over what free functions are available because
I probably should know about them before I need them. Being free functions
also signals that they are potentially special or different in some way so that
also makes them of interest.

Now this is mostly a sub- or semi-concious affair and I have no idea how
other people approach this...

That said if this has't been an issue with other unsafe* function then I
doubt it would be one in this case.
And nesting them doesn't feel right without any other precedent and
opens up the discussion about nesting other free functions which is
probably not something we want to do right now.

But given the other unsafe* free functions that's already exists that
might be fine.

Yeah, IMO that would be reasonable.

Also having a predefined wrapper type has some use, e.g. when you
want to store your unowned(unsafe) objects into some collection.

Yeah, though it seems to me that wrapper should be UnsafeReference<T>.

Yes, it would certainly be the simplest solution.

IIRC, my biggest concern with that was that this continues to combine
"transfer unannotated objects" and "manual reference counting" into a
single type. Which might make it harder for someone new to the topic
that just needs to use some object that comes from an unannotated API.

But again that is probably not a huge issue in practice and we have the
same issue with Unmanaged today. Also UnsafeReference's improved
method names & documentation should hopefully mitigate this as well.

A dedicated type or free functions have other tradeoffs so ultimately I'm
pretty much open to any of these given that, as you mentioned, MRC is
probably going to be retained (no pun intended) in some form...

But I'd guess you'll end up with a dedicated wrapper type anyway in
most circumstances where this is an issue so it's probably OK. And
this gets rid of having a separate type that also plays the role of
unowned(unsafe) which seems like a plus.

I have some more answers below but I'll summarize my opinion here.
Preferences (in descending order):

1) unsafeRetain() + unsafeRelease() + unowned(unsafe)
2) "UnsafeReferenceCountedPointer"
3) No dedicated functionality so just abuse UnsafeReference instead of
  Unmanaged

I think ultimately it's a question of whether we want to expose the
reference counting implementation to manual use...

Is it something I can live without? Absolutely.
Is it something I would use often? Absolutely not.
But then again, it is going to be exposed through UnsafeReference anyway.

And having access to a solid manual reference counting solution
that integrates very well with the rest of the language is kinda neat in
my opinion. And the integration with ARC is something an external
library cannot provide without becoming even more "hacky".
And I don't think it's any more dangerous than any other manual
memory management we have access to.

Okay. My current feeling about all this is that UnsafeReference should
have retain and release directly on it, for MRC.

The thing I'm really unclear about at this point is whether it's
reasonable to ask users to use “release()” for the maximally-safe usage
pattern, or if that's just too weird. My sense all along has been that
asking them to get used to it is a much better design choice than
expanding the API, especially when we don't really have a better name for
it than “release.” If a better name presented itself, that might change
the picture.

I've taken issue with it before but I've reread the thread and I still don't have
any new ideas.

I think 'transferByReleasing()' & 'transfer()' as suggested by Nevin might be
my favorite of the proposed alternatives. Other verbs than 'transfer' might
work too.
Whether it's better than release() and .object... I don't know.

At any rate, at least retain() would fit somewhat nicely in the state diagram
described by the "ownership states" section so retain() could be documented
as going from Unretained->Retained.

But I don't know what the plan would be regarding documentation if MRC
behaviour is done through UnsafeReference. Trying to document it would
probably really detract from and add confusion to the simple model that
is currently laid out.

[Quick aside : I was looking at the draft for UnsafeReference as quoted in
Proposal 6 and there is a period missing after
"No API should pass or return a *released* UnsafeReference`"
and a double-space right before the sentence (and in a few other places).]

I also wonder if it has some minor use for learning/teaching.
Yes, ARC is great because most of the time you don't need to think
about this but if you're trying to understand reference counting it's
kinda nice to be able to actually interact and play around with it.
Then again I'm the kind of person that likes doing that but YMMV...

As long as you don't need to share anything across threads, it's easy
enough to build MRC using malloc, free, and a counter in each allocated
block, so this doesn't sound like a strong argument to me.

Right and it's certainly no justification for the feature by itself.

I think it could be nice as a real world example / demonstration tool though.
Along the lines of:
"So you've been learning about ref counting now and you've also been using
Swift. Well guess what, Swift does that and you can even watch it do its thing".

Demystifying the "magic" in complex systems with simple building blocks is
something I love about all of Science and Engineering.

As Joe mentioned, `Unmanaged` has a use for manual ref counting
beyond immediate transfer from un-annotated APIs.

I have used it for performance reasons myself (~ twice) and while I
think it's a pretty small use case there isn't really any
alternative.
If it would help I can also describe my use-cases in more detail.

Yes please!

One place I used Unmanaged is in a small project where I experiment
with binary heaps in Swift. I've put the project on Github
--(https://github.com/Jnosh/SwiftBinaryHeapExperiments) but basically
I'm using `Unmanaged` in two places here:

1) Testing the 'overhead' of (A)RC.
Basically comparing the performance of using ARC-managed objects in
the heaps vs. using 'unmanaged' objects. In Swift 1.2 the difference
was still ~2x but with Swift 2+ it's likely approaching the cost of
the retain/release when entering and exiting the collection.

Now this could also be accomplished using `unowned(unsafe)` but
`Unmanaged` has some minor advantages:
  a) I can keep the objects alive without keeping them in a
separate collection. Not a big issue here since I'm doing that anyway
but I also find that `Unmanaged` makes it clearer that & how the
objects are (partly) manually managed.
  b) I had previously experimented with using `unowned(unsafe)`
for this purpose but found that `Unmanaged` performed better. However,
that was in a more complex example and in the Swift 1.2 era. A quick
test indicates that in this case and with Swift 2.1 `unowned(unsafe)`
and `Unmanaged` perform about equally.

They should. unowned(unsafe) var T is essentially just an
UnsafePointer. unowned/unowned(safe) do incur reference-counting cost
in exchange for their safety.

I'll come back to this further down.

2) A (object only) binary heap that uses `Unmanaged` internally
Not much practical use either in this case since the compiler seems to
do quite well by itself but still a somewhat interesting exercise.
`Unmanaged` is pretty much required here to make CoW work by manually
retaining the objects.

It's hard for me to imagine why that would be the case. Would I have
needed to use Unmanaged in implementing Arrays of objects, if it were?

Sorry, I wasn't clear enough. I (ab)use Unmanaged for two different reasons here.

1) To have a performance baseline where the ARC overhead inside the collection
is essentially zero beyond the mandatory retain on insert, i.e. as if the compiler was
able to eliminate all (redundant) retains and releases.

One part of this is exempting the objects from ARC which is is done by storing the
elements in Unmanaged instances but a wrapper type using unowned(unsafe)
would work just as well.

However, I still need a strong reference to the objects to keep them alive. Using a
separate data structure would work but that has a space, time and code complexity
cost.
Instead I use Unmanaged to manually retain the objects on insert and release on
removal. unowned cannot do that on its own hence the need for something like
unsafeRetain() & unsafeRelease().

OK.

2) I then abuse Unmanaged's capabilities a second time to retain the elements
when the collection is copied (which would happen 'automatically' with
ARC).

Btw, with Swift 2.2 under WMO the performance of a normal ManagedBuffer
is on par with this "hack". Go Swift team!

:slight_smile:

The other project was a simple 2D sprite engine (think a simplified
version of SpriteKit) I experimented with about a year ago.
Textures and Shaders were abstracted as value types privately backed
by reference types that managed the underlying OpenGL objects,
i.e. destroy the OpenGL texture object on deinit, etc...

I found this to be quite nice to use but ARC overhead during batching
& rendering amounted to something like 20-30% of CPU time IIRC. (This
was under Swift 1.2 and with WMO). Using `Unmanaged` was one of the
things I played around with to get around this and it worked very
well.

Another case where you can use unowned(unsafe), is it not?

Indeed, and that was what I originally tried to use.
Ultimately i settled on Unmanaged however. Now it's been a long time and
I don't recall the exact details so take this with a grain of salt:

One reason certainly was that I ended up needing Unmanaged anyway
to perform manual retain & releases at which point why not also use it
for 'storage'...

But I also vaguely recall that Unmanaged had more of a performance impact.
Now one possibility is that there was some issue with unowned(unsafe) (this
was with 1.2.β1) but much more likely is that Unmanaged was easier to apply
consistently and correctly.
e.g. assume you have some struct that contains an unowned(unsafe) variable.
Now if you extract that into a local variable you add a perhaps unwanted
retain/release so you might need to mark the local variable as unowned(unsafe)
as well, etc...

Right; makes sense.

What I'm trying to say is that with unowned you need to be careful and
considerate with how you use it at all times since the 'obvious' thing
generally leads to retain/release.

That's safer, though, but in this case you're more concerned about performance.

Yep. In my experience Unmanaged also worked surprisingly well.
You can (and will) of course make all the obvious mistakes but at least it forces
you to think about what exactly you want whenever you access it.

And whenever you screw up, all the potential failure points are relatively easy to
find since you can mostly just ignore all the "normal" objects.

Unmanaged is fine to pass around, store in a local variable,
etc... and any ARC related interactions are obvious because they
manifest as method calls on the Unmanaged instance.

For their main application, breaking retain cycles, weak and unowned work fine
because you want to retain the objects when they are not 'at rest'.
But if you want to avoid retains even when working with the objects, a type is
just a much more comfortable way to handle this.
Like I mentioned before, I imagine that in many cases you'll end up making a
custom wrapper anyway but it's something I'm a bit apprehensive about.

Still, I think unowned(unsafe) together with unsafeRetain() and unsafeRelease()
free functions makes for a nicer API and I don't think I can adequately judge it
beyond that. I've barely used this in it's current form have no real experience
with a potential future form and I hopefully won't use it (often) anyway.
So I think it's more than appropriate to prioritize the general API over making
this esoteric use case more comfortable to use.

I think you've already talked me into the idea that we need a type for
transporting manually reference-counted references.

The `Unmanaged` instances were created when draw commands are
submitted to the renderer so they were only used inside the rendering
pipeline.
I eventually switched to using the OpenGL names (i.e. UInts) directly
inside the renderer since they are already available anyway but that
also requires extra logic to ensure the resources are not destroyed
prematurely (e.g. retaining the object until the end of the frame or
delaying the cleanup of the OpenGL resources until the end of the
frame, ...). In many ways it's quite a bit messier than just using
`Unmanaged`.

I don't see how Unmanaged could have been less messy; don't you still
need a strong reference somewhere to ensure the lifetime?

Absolutely. You can retain the object directly through Unmanaged
(via passRetained() or retain()) and make the Unmanaged instance
a strong reference in effect.

Right. Thanks for refreshing my ability to think about these issues
again :slight_smile:

:slight_smile: I should know better by now but I'm still often surprised how something
that felt relatively simple and sensible when writing the code turns out to be
very hard to put into words and properly justify.

You had me going back to to some code more than once because I was
wondering myself why on earth I couldn't have just done X.

Not a big difference to retaining by putting the objects in some container.
Just a different set of tradeoffs.

I don't think it's the best solution for this case but it's pretty simple. Retain
when creating the draw command, release when discarding the draw
command - nothing different than malloc/free.

Collecting the objects in some collection is likely to be a cleaner solution
and more efficient too, since you don't retain objects multiple times if they are
used multiple times in the same frame (which is likely for shaders, textures).
But then someone somewhere needs to manage this, and you need to
access that state when creating or submitting the draw command.

Or perhaps make sure the referenced resources stay valid until the frame
is drawn so you don't need to retain here at all but now you need to track
all the scene contents, etc...

Hopefully I don't come across as too petulant. :slight_smile:

Not a bit.

I don't really want to argue in favor of or defend these approaches.
I'm merely trying to give some examples of when and what for I actually
used this stuff not to prove the merits of these cases but instead to argue
for the existence of better justified uses based on the same ideas.
Not sure if that makes any sense but there you go :slight_smile:

Thanks, it's been very helpful.

I don't think these are particularly great examples and I could
certainly live without 'native' MRC but ultimately I think it's an
interesting capability so I'd like to keep it around.
Although I'd be in favor of keeping it out of the stdlib but I don't
think that's really an option just yet...

It would also be interesting to be able to do the same with indirect
enum instances and closures but it's not like I have a particular use
case for that :wink:

I don't understand what you might be hinting at here.

Just that AFAIK closures and indirect enum instances also use ARCed
references under the hood. So in theory the could potentially also be
stored unowned and manually retained/released.

I just find it slightly interesting that with (Any)Objects certain things are
exposed (unsafeAddressOf, retain/release, ...) whereas with indirect enums
and closures they are not.

I don't want to imply that that would be a good idea and it would certainly
be hard, complicated, and annoying to implement with essentially n
benefit so I don't want to go anywhere with this other than the partial similarity.

Basically it's just my brain going:
"Oh look, some pyramids. Hmm, you could store these much more efficiently
if you stacked them up against each other" :wink:

Say, you must be some kind of engineer or something! :wink:

--
-Dave

- Janosch

···

On 22 Feb 2016, at 16:45, Dave Abrahams <dabrahams@apple.com> wrote:

on Mon Feb 22 2016, Janosch Hildebrand <jnosh-AT-jnosh.com> wrote: