When should you use __shared in an initializer?

Using Swift
1

sometimes i really struggle to decide if an init should take its argument as __owned (which is the default) and when it should take its argument __shared. i think when the result is intended to be stored directly in the structure unchanged, the default __owned convention should be left as-is.

but when there is more post-processing that needs to be done, i don’t know if i should use __owned or __shared:

struct Metadata:Equatable, Sendable 
{
    let traits:[Int]

    init(traits:[Int])
    {
        self.traits = traits 
    }
    init(traits:__shared [UInt8])
    {
        self.init(traits: traits.map(Int.init(_:)))
    }
}
1 Like
2

Most of the time I would say just don’t worry about it.

But, if you want to think about it, the questions I ask myself in this situation are:

  1. is it probable that passing this to the initializer is the last use of it in the caller?
  2. does this escape the initializer, eg by being stored in a property?

Yes/Yes: owned saves a retain/copy (moves the release from the caller to the initializer, then deletes the matching retain-release pair)
Yes/No: doesn’t matter (just moves a release from the caller to the initializer)
No/Yes: doesn’t matter (actually has two live references, so needs a retain regardless)
No/No: shared saves a retain/copy (doesn’t insert a retain to keep it live after the initializer)

1 Like
3

the thing is i don’t usually know the answer to #1, and as for #2 i’m not really sure when it applies or not. in the

    init(traits:__shared [UInt8])
    {
        self.init(traits: traits.map(Int.init(_:)))
    }

are the traits escaping the init or not? because they don’t get stored directly, but they are copied explicitly inside the init through the map operation. but preemptively copying the [UInt8] seems kind of pointless, because that gives another [UInt8], not an [Int] like the map creates.

4

yeah, you can’t always know what the best answer is. Sometimes there isn’t a best answer.

I believe map should consume self, so owned would be better. But, it should also get inlined, so in practice it’s probably fine either way.

2 Likes
5

I'm not sure about that. There is clearly a best answer, IMO - neither the init nor map should retain the traits Array, because both could work just as well with a +0 borrow.

Unfortunately we have no way of specifying which convention to use for map. It has a default, based on what the standard library maintainers think is appropriate for most contexts, but it ends up applying to every use of map in every context, and it simply might not always be the best choice.

I've been struggling with similar issues recently with iterators. I've seen significant performance improvements by using indexing for iteration (rather than a for loop), because makeIterator() is consuming, but it shouldn't be in my context (I'm iterating the collection twice), and the extra retain had a bigger performance impact than the loop itself.

It reminds me of branch hints. The conventional wisdom is that developers are really bad at predicting which paths get taken and how it affects the generated code, so adding hints often ends up hurting performance. A similar thing applies here, IMO. I wish we could override it.

Inlining can sometimes help but it is also unpredictable.

1 Like
6

This is what I meant by the bit you quoted. There isn’t a right answer for the general case, there is for any specific case.

2 Likes
7

i don’t really care about what map does, we all know it’s transparent and i’m fine with letting the compiler decide what to do there (if only it would get that far without crashing :smiling_face_with_tear:).

my question is about what init(traits:[UInt8]) should do, because that is not transparent, or at least i don’t want it to be since i am trying to use less @inlinable everywhere…

it sounds like my gut instinct of making it __shared was right, so i’m going to go with that. thanks for the discussion!

8

This is relevant to the discussion of borrow and take in argument position. I believe the current design leaves the ABI unchanged, and the markers are only used to statically assert ABI expectations from the caller side. But, for the specific reason that motivated this thread, I’ve wondered if it would be a good idea to augment the function ABI so that (some) parameters can be passed in one of many ways.

1 Like
9

Using indexing for iteration is indeed probably almost always better, and will be necessary to really support iteration over move-only collections. makeIterator is consuming not necessarily because it's a good idea, but because the entire Sequence protocol is kind of busted, but it's a requirement of Collection we're stuck with, and having makeIterator be consuming is the only way it can be implemented by a future move-only collection type. It would only be useful for consuming iteration of such a collection, though, and we would need generator- or index-based iteration to do borrowing or mutating iteration of move-only collections. Most of the shared/owned annotations on standard library protocols were added with the consideration to make move-only conformers possible at all, not always ideal performance.

4 Likes
10

Obviously within a module we can use function signature optimizations to do this automatically, but for public API this is a super interesting question. I don't think I have a good mental model for the likely tradeoffs between the costs of exposing additional entry points and the costs of suboptimal calling conventions.

(…a JIT would be real nice here wouldn't it. Ah well)

1 Like
11

I mentioned the possibility of changing the ABI in the thread about borrow and take.

12

Can Xcode give us guidance?

13

Let me remember this for future reference.

Thanks for finding this out for us the clueless!