Skip to content

Conversation

@Coeur
Copy link
Collaborator

@Coeur Coeur commented Dec 29, 2023

Fix bdkjones/VDKQueue#22.

Instead of polling kevent every second, we remove the timeout and we send a one-shot event when we want to stop watching.

Testing: with DEBUG_LOG_THREAD_LIFETIME enabled, I can see "watcherThread finished." instantly after turning off the "Watch for torrent files" option in the Transmission settings.

Notes: Improved macOS UI code to use less CPU.

@Coeur Coeur added this to the 4.1.0 milestone Dec 29, 2023
@Coeur Coeur requested a review from nevack December 29, 2023 02:19
@mikedld
Copy link
Member

mikedld commented Dec 29, 2023

Instead of keeping to patch VDKQueue, maybe consider replacing it with libtransmission/watchdir-kqueue.cc? The two provide essentially duplicate functionality but we're using libtr implementation in the daemon anyway and it's not going away. I wouldn't suggest it if the needed functionality was provided by the SDK (as is the case with Qt and GTK) and we weren't reinventing the wheel... Granted, I didn't look at how easy it'll be to use so won't insist.

@Coeur Coeur force-pushed the coeur/shutdownWatcherThread branch from 3e510a3 to 7b2ce2c Compare December 29, 2023 02:54
Copy link
Member

@nevack nevack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I've read upstream issue
@Coeur btw, is there any plan to switch to FSEvents?

@Coeur
Copy link
Collaborator Author

Coeur commented Dec 29, 2023

@Coeur btw, is there any plan to switch to FSEvents?

Current solution works pretty well and is the underlying mechanism used by GCD (Obj-C) and DispatchSource (Swift), so I don't see any need to take the risk to change it at the moment. Let's prioritize moving Transmission to Swift instead.

@gobbledegook
Copy link

In fact, FSEvents also uses kqueue under the hood. (If you ever misuse FSEvents you'll see some kqueue error messages scroll past your console!)

@ckerr ckerr force-pushed the coeur/shutdownWatcherThread branch from 7b2ce2c to b705182 Compare December 31, 2023 20:05
@ckerr ckerr merged commit f8f6806 into transmission:main Dec 31, 2023
@Coeur Coeur deleted the coeur/shutdownWatcherThread branch December 31, 2023 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants