Should memberwise init for `weak var` be passed as `__shared`?

Using Swift
1

this is definitely a micro-optimization, but should a memberwise init for a property with the weak storage modifier take its argument __shared or __owned?

public
actor C {}

public
struct S
{
    weak var c:C?

    public
    init(c:__shared C)
    {
        self.c = c
    }
}

vs just

extension S
{
    public
    init(c:C)
    {
        self.c = c
    }
}
2

Here's my hot take: the performance cost of using weak so vastly outweighs this micro-optimisation that it doesn't matter at all.

2 Likes
3

what would you recommend using instead of weak var? the struct is being used as a context for a monitoring thread, so it needs to halt when C is deinitialized

4

While it is true that weak references come with overheads, lots of things can impact performance, and the benefits of expressing lifetime information in the language stretch to more than just performance. So I wouldn't say it is never worth doing.

There is a clear "best" answer in this case - weak references should be __shared (i.e. borrowed). They do not extend the lifetime of the class instance.

Example:

class StoresWeakFoo {
    weak var instance: Foo?

    init(_ instance: Foo) {
        self.instance = instance
    }
}

class Foo {
    var x: Int = 42
}

No ownership annotation, so using the default __owned/take convention for initialisers:

output.StoresWeakFoo.__allocating_init(output.Foo) -> output.StoresWeakFoo:
        ...
        call    swift_allocObject@PLT
        ...
        call    swift_weakInit@PLT
        ...
        call    swift_beginAccess@PLT
        ...
        call    swift_weakAssign@PLT
        ...
        call    swift_release@PLT   ; <--- HERE
        ...
        ret

In the default case, since we are not actually consuming the passed value, we need to release it. That also means callers have to defensively retain the instance before calling the initialiser, unless they have some existing known +1 ownership they can transfer. This is all wasted effort.

With __shared/borrow:

output.StoresWeakFoo.__allocating_init(__shared output.Foo) -> output.StoresWeakFoo:
        ...
        call    swift_allocObject@PLT
        ...
        call    swift_weakInit@PLT
        ...
        call    swift_beginAccess@PLT
        ...
        call    swift_weakAssign@PLT
        ...
        ret

By being specific - which is not unsafe or dangerous or fragile, it is just expressing our intention more precisely - we can avoid that release, and callers will not have to defensively retain the instance.

2 Likes
5

Ideally, C would be responsible for owning that monitoring thread, such that C being deinited is directly the signal for termination of the monitoring thread.

6

I would counter that, if the fact that the storage is a weak reference is an implementation detail of the type, that you shouldn't indirectly expose that fact in the initializer's convention, since you'd want to change it back if you ever change your object graph around and need to make the type store a strong reference instead.

1 Like
7

thank you @Karl for the detailed answer.

i don’t understand what you mean by this, the monitoring task periodically reports its status back to C, so it has to capture a reference, either strong or weak, to C. so if the monitoring task holds a strong reference to C, then C will never get deinitialized in the first place.

in my instance, C.deinit sends the signal to halt the monitoring task, so the monitor must hold a weak reference and not a strong reference, so this is not an implementation detail.

8

Alternatively, the monitoring task can publish into an AsyncSequence that C consumes, allowing the deinit of that sequence to communicate the absence of C.

9

this would take the weak modifier out of the monitoring context, but (at least with what i am imagining) the stream consumer would still have to capture a weak self in order to write its updates back to self.

10

This is where we're hurt by the abstract nature of the conversation. It's certainly possible that C requires a separate publishing loop, but I suspect it probably doesn't. However, I can't make that case, because I don't know what it is or how you're trying to use it.