Replace threadsafe var with new concurrency model (Actor?)

This just gets more and more confusing.

Let me state what the situation is…

Unlike a normal Folder; a Smart folder relies on NSMetadataQueryDidFinishGathering and NSMetadataQueryDidUpdate notifications, sent from the default NotificationCenter.

The handling closure receives a notification object, which contains an array of all URLs that match the query's predicate. It is these URLs that need to be passed to a "buffer", to be returned by the fileURLs property on demand from a CollectionView.

The updating notifications only happen as a result of a user manually deleting or adding files to a folder, so it's hardly a time-critical operation.

Once the notification handler has assigned the changed URLs to the "buffer", it then posts a message to the default NotificationCenter, which is caught by the UI and provokes a refresh of the CollectionView, which then reads the "buffer" via the fileURLs property, in the same way as it would read a static list of URLs in a normal folder.

in other words, the rest of the app's code should know absolutely nothing about how the URLs are supplied, hence the protocol.

The code I have for the ThreadsafeVar works fine but, with all the hype about actors and isolation, I was trying to see if I could make it work in this situation.

Or is all this new stuff a solution looking for a problem? :wink:

Actors are a tool in your toolbox, it is not the solution to every problem. The hype is real (like with many new tech such as this one) and causes to overuse them (in my opinion) without regards to the cost. About your specific problem, to be honest I don't understand why you need thread safety at all, it seems to me Spotlight does its thing in the background and can simply call you back with the results on the main thread. Or am I missing something that requires you to explicitly run code on a background thread?

The update notifications are received, and the array is built, on a background thread but the code that reads the URLs comes from the main thread.

If a user is scrolling the CollectionView immediately after a file gets added or removed from the folder on disk, there is a chance (which I have had happen) that the incorrect URLs get requested when the update happens when the items in the CollectionView have changed.

Looks like you can receive those notifications on the main queue and get rid of your ThreadsafeVar wrapper.

        NotificationCenter.default.addObserver(forName: Notification.Name("xxx"), object: nil, queue: .main) { note in
            dispatchPrecondition(.onQueue(.main))
            change your items here unprotected, no fear
        }

Personally I would first try to receive the notifications directly on the main thread as it would make things a lot easier and updating the array with the results would be fast enough. If it actually causes issues like a lagging GUI, you could also do whatever you need from your background thread then periodically dispatch to the main thread so the array is only touched from there. Locking a shared array as you are doing would also work, seems more complicated but you know your use case better than me, maybe it works best in your situation.

Hmmm. Interesting but…

Quite a lot of processing has to go on once the notification has been received - reconciling the list of URLs now in the folder against those already there and determining the indexPaths of which ones have been removed or added. This is why I felt it is preferable to do all this "grunt work" on a background thread before simply signalling the UI to update.

1 Like

Added, removed, moved, updated, yeah. That's quite error prone to do right in general case when all these things can happen simultaneously. This is easier with SwiftUI, and if you are limited to UIKit - you may try its iOS 13+ diffable data source API that performs this heavy lifting for you. Interestingly its main apply(snapshot) call is not limited to the main thread (contrary to the classic reloadRows / reloadData):

Yes, you can do that work on a background queue and update UI on the main thread (or even background thread as discussed above). However, you don't have to protect that data if you only read / modify it on a single background queue... Example:

        let queue = OperationQueue()
        queue.maxConcurrentOperationCount = 1
        ...
        NotificationCenter.default.addObserver(forName: Notification.Name(""), object: nil, queue: queue) { n in
            dispatchPrecondition(condition: .notOnQueue(.main))
            // we are in background
            urls = ... // change urls here. single queue access → can be unprotected
            // you may apply diffable snapshot here (see above)
            
            let urlsCopy = urls // grab a copy. single queue access → can be unprotected

            DispatchQueue.main.async {
                ... urlsCopy // use urlsCopy here
                // you may apply diffable snapshot here (see above)
                // or you may apply insert / delete / move / modify updates here manually
            }
        }

I am talking about a macOS app, not iOS, and I certainly wouldn't be attempting to use SwiftUI for such complex app.

Thank you for pointing out the NSCollectionViewDiffableDataSource class. This wasn't available a couple of years ago, when I started on this app. It may make reconciling the changes to the old array easier, but I am still going to be left with having to assign and read the new array across thread boundaries in order for the collection view to be able to access it from the main thread after updating it on the background thread - unless I have misunderstood how the new datasource works.

Replying to my own post…

It would seem that the new NSCollectionViewDiffableDataSource requires a determinate number of sections, defined in an enum. If this is really the case, it is unusable for my app as the number of sections is determined at runtime.

Up to you of course. I found SwiftUI code about 3 to 4 times shorter than the equivalent AppKit / UIKit code, and for me shorter / easier / more understandable code helps especially if the app is complex.

No, that's just the particular example code you found. Section or Item can be anything of your choice so long they are Hashable. e.g. a String. Or a struct. In the following partial example I am using a struct for Section:

Partial Example
struct Row: Hashable {
    let string: String
}

struct Section: Hashable {
    let header: String
    let rows: [Row]
}

typealias Snapshot = NSDiffableDataSourceSnapshot<Section, Row>
typealias Datasource = NSCollectionViewDiffableDataSource<Section, Row>

class Model {
    var sections: [Section] = []
    
    func createSnapshot() -> Snapshot {
        var snapshot = Snapshot()
        snapshot.appendSections(sections)
        sections.forEach { section in
            snapshot.appendItems(section.rows, toSection: section)
        }
    }
    
    private var calledOnMainQueue: Bool?
    
    func applySnapshot(dataSource: Datasource, snapshot: Snapshot) {
        // can be on background or main queue, but be consistent
        if calledOnMainQueue == nil {
            calledOnMainQueue = Thread.isMainThread
        }
        dispatchPrecondition(condition: calledOnMainQueue! ? .onQueue(.main) : .notOnQueue(.main))
        dataSource.apply(snapshot, animatingDifferences: true)
    }
}

Whether section contains items themselves or just the key using which you can obtain the items for the section is up to you. Ditto for the Item (row) - can be the "whole thing" or just the key.

You may find this full example applicable. It is for iOS but there's virtually no difference (just a change from UIDiffableDataSource to NSDiffableDataSource and UITableView / UICollectionView to NSTableView / NSCollectionView.

I doubt it for this app…

The interactions are particularly complex and, from what I have seen, a lot of logic code in SwiftUI gets subsumed into View classes - something I detest. And so far, it has taken nearly 3 years to get to its current state, so SwiftUI was very immature and I'm not even thinking about rewriting it.

I only see standard controls on this screenshot so the UI itself looks a 3 week job implemented in SwiftUI (with perhaps 70% implemented in 3 hours and 90% implemented in 3 days). Here I am talking of UI only plus a mock data model to drive it. For AppKit implementation I'd have to multiply those estimates by 3. We can take that part of conversation offline as we've significantly deviated from the topic. :wink:

PS. I appreciate the final percent can take an unbound amount of time no matter what technology is used.

PPS. Enbolded the relevant points to not reiterate them.

Wow! So you know how to write a basic UI, but do you know how to:

  • read and write metadata to image files or XMP sidecars
  • create dynamic lookups for an NSTokenField
  • create popup menus for the thumbnails
  • create metadata search queries
  • integrate with macOS Finder tags
  • implement drag and drop of image files that contain keywords
  • implement Quicklook preview panels
  • implement a search UI that allows both "starts with" and "contains" logic, that also allows for ANDing and ORing of predicates?
  • implement a star rating search tool that allows multiple ratings
  • etc, etc.

What you see in the screenshot might appear simple but, speaking as a developer with 30 years experience in DOS, Windows, Mac and iOS development, I would call your assertions about the alleged simplicity of SwiftUI a wee bit presumptuous.

It is rarely what you see that takes the time, it is what you don't see. Something I have taught many developers in my role as a software analysis and design consultant.

2 Likes

This is not any less true with any other synchronization patterns under concurrency. Fine-grained synchronous locking and Joanna's ThreadsafeVar have exactly the same interleaving and ordering problems you're identifying with actors. Getting programmers to think carefully about atomicity/transactionality is just an inherent problem in concurrent programming.

1 Like

The point I was trying to make is that code that is isolated to the context of a thread/queue/actor is safer when it does not call async functions because it cannot be the victim of interleaved code execution. Therefore creating a basic async primitive such as this one would be a bad idea (in my opinion) because it would invite async calls in many places way too easily.

Except atomicity is much simpler to guarantee when you can write using synchronous constructs rather than requiring async everywhere, for the reasons @tclementdev outlines. Swift's concurrency features don't allow for such constructs and there's no way to mark existing constructs (locks, etc.) in such a way as the compiler can see them as safe.

You can have a function that calls a synchronous function while holding a lock.

Yes... that's what makes it easier to reason about. I can guarantee I hold exclusive access to a resource until all of my work is done. With concurrency you can only make a similar guarantee if you stay synchronous within the awaited work. A single need to await means you lose your atomicity or, at the least, make it much harder to reason about. Plus you require every caller to adopt async syntax in the first place, making it rather viral.

1 Like

I’m saying that making things synchronous in order to prevent suspension and interleaving is a tool provided by the language which you can use in order to get the effect you want. You don’t have to make actor functions async; they’ll be observed as async from non-isolated code, but internally they won’t allow suspension.

1 Like

This is the best I have come up with so far in terms of wrapping thread safe actors in external access properties. Still need a separation of read and write state... At least I can now confirm that any global state access writes will be thread safe.

NOTE: The idea is completely unworkable for anything by simple value types. Especially if you have a struct that is altered by multiple code paths. Taking a copy of the state and then writing it will mean you cannot guarantee any persistence.

// A use case requires it to have access to a global state controller...
protocol UseCase: AnyObject, CustomStringConvertible {
    var state: GlobalState { get async }
    var mutableState: GlobalState { get async }
    var stateController: GlobalStateController { get }
    var queue: DispatchQueue { get }
}

extension UseCase {
    var description: String {
        "\(String(describing: type(of: self))): This use case does not have a description..."
    }

    var state: GlobalState {
        get async {
            await stateController.readState()
        }
    }

    var mutableState: GlobalState {
        get {
            fatalError("Mutable state has no read component")
        }

        set {
            Task {
                await stateController.writeState(newValue)
            }
        }
    }
}

actor GlobalStateController {
 ... lots of funky read/write stuff here 
}

struct GlobalState: Storable { // Storable is Equatable and Hashable
 ... global state goes here 
}