Garbage Collection

[iaburton]

Hi there,

I noticed here that there's a TODO for garbage collection.
https://github.com/shurcooL/trayhost/blob/master/trayhost.go#L152

It seems that the display function always appends the value of the struct into that slice. As implied by the TODO, does this mean the memory usage of an app using this to create notifications over time will see its memory usage grow? I tried reading the non-Go code to figure out if it was somehow being removed from that side, but I don't know Cocoa well enough. Thanks

[dmitshur]

Hi @iaburton,

I noticed here that there's a TODO for garbage collection.
https://github.com/shurcooL/trayhost/blob/master/trayhost.go#L152

It seems that the display function always appends the value of the struct into that slice. As implied by the TODO, does this mean the memory usage of an app using this to create notifications over time will see its memory usage grow?

Your understanding is correct.

It's currently appending to that slice for each displayed notification, and there is no code that removes it. That's indeed what the TODO is about.

It's a little tricky because of the Go and cgo boundaries, it's not obvious what the best time/place to clean up unneeded notifications is (and how to be sure they're no longer needed).

In practice, the memory usage in an actual application doing this is going to be so low even after opening hundreds of notifications, which is why I forgot about this TODO and did not prioritize it more highly.

Fixing it would be nice and should be done.

[iaburton]

The point of appending it into the slice in the first place is so Go has a reference and doesn't GC it while the C code is using it correct?

While I haven't programmed in CGO (know some C++ but not C so much) would it be possible to have the C code tell the Go code when it's done? Or perhaps have a background goroutine (spawned by EnterLoop?) that has a for/select with a chan of pointers to Notifications (created by the values in Display) and another case for a ticker/timer that periodically deletes the oldest notification? Using the notifications timeout field, and if none is set, however long OSX keeps a notification displayed by default.

Unfortunately I don't have a Mac to test this on (cross compiling from Linux which has other problems), but I'd like to help figure this out the best I can. Thanks for the package btw, and all your contributions to the Go community 😃

[dmitshur]

The point of appending it into the slice in the first place is so Go has a reference and doesn't GC it while the C code is using it correct?

Yes. The notification data is needed immediately to display the notification, but it's also potentially needed some time later (seconds or minutes later) if a user clicks on it while Notification.Handler click handler is set.

So we'd need to detect when the notification has gone away in cgo and report back to Go so it can remove the notification data, since it's guaranteed to no longer be needed.

While I haven't programmed in CGO (know some C++ but not C so much) would it be possible to have the C code tell the Go code when it's done?

It should be possible, but it requires figuring out the exact details of when the notification goes away.

I hope that explains some of the challenges. It certainly can be done (by someone with a Mac), just needs time and additional investigation. Since it's low priority (low real world negative impact) and hard/time-intensive, I won't be able to get to it very soon.

Thanks for the package btw, and all your contributions to the Go community 😃

Happy to hear it's appreciated and useful! Thank you! :)