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
}
}
lukasa
(Cory Benfield)
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
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
Karl
(👑🦆)
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
lukasa
(Cory Benfield)
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.
Joe_Groff
(Joe Groff)
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
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.
lukasa
(Cory Benfield)
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.
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.
lukasa
(Cory Benfield)
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.