[swift-evolution] Pitch: Progress Tracking in Swift

[Tony Parker]

Hi Charles,

> On Jan 19, 2016, at 2:24 PM, cocoadev at charlessoft.com wrote:
> 
> On 2016-01-19 14:01, Tony Parker wrote:
>> Hi Charles,
> 
> Hi Tony,
> 
> Thanks very much for responding. I do really appreciate the opportunity to communicate with members of the development team on these lists.
> 

Sure, and thanks for your thoughts on this.

>> I see Rob already pointed this out, but I’ll respond as well since I
>> own NSProgress and I also was involved in the WWDC session he
>> references.
>> NSProgress actually received a significant upgrade in iOS 9 and OS X
>> 10.11 to address most of your concerns. I’ve never really liked the
>> ‘current’ progress idea either, so I’ve changed Foundation to
>> make using explicit progress objects possible (and encouraged). There
>> are still cases where the idea of having a current progress is useful.
>> For example, when there is no opportunity to grab a progress object,
>> because the method you are calling is a proxy, or a global function,
>> etc.).
> 
> While this is true, I feel that the syntax for using it is quite verbose, which feels out of place in Swift code. Mainly what it comes down to, though, is that I actually rather like the concept behind what they were trying to do with the "current" progress object, which was to make it possible for all sorts of low-level methods and functions (like -[NSData dataWithContentsOfURL:options:error:]) to automatically and painlessly support progress operations, so that an application that was doing time-consuming operations could get a lot of this "for free." It's just the *implementation* that I quibble with, due to various issues it brings up. I feel like my pitch *works* like the explicit composition method while *feeling* like the implicit composition method, if that makes sense. At any rate, anything that would make progress more attractive to implement, thus increasing its prevalence in the frameworks, would be a plus; I would love to see something like NSFileManager's file copy APIs supporting some sort of progress reporting (in an extension, of course, unless we had some way for Objective-C to call into the Swift progress-reporting mechanism). I also feel like writing it in pure Swift and minimizing the amount of work that is done on the current thread could reduce the performance costs of using it, which is important for these sorts of APIs since you don't want to add unnecessary overhead in the case where you're not interested in monitoring the progress.
> 
>> You can now create progress trees explicitly. There are two parts to
>> this:
>> @available(OSX 10.11, *)
>> public /*not inherited*/ init(totalUnitCount unitCount: Int64,
>> parent: NSProgress, pendingUnitCount portionOfParentTotalUnitCount:
>> Int64)
>> and a new protocol:
>> public protocol NSProgressReporting : NSObjectProtocol {
>> @available(OSX 10.9, *)
>> public var progress: NSProgress { get }
>> }
>> So you can mark classes that publish progress with the protocol, and
>> clients can create their trees using the new initializer.
> 
> This seems oriented toward classes such as NSOperation subclasses that represent a singular operation of some sort, but doesn't seem as applicable to single-function APIs like NSFileManager.copyItemAtURL(toURL:). It can of course be used that way, but it feels somewhat awkward. It also encourages the use of one big monolithic NSProgress object for the whole operation/thread/dispatch queue, which undermines some of what makes NSProgress so cool; the tree-based approach is really great for complex operations which are composed of a lot of separate steps, each of which may have distinct sub-steps of their own.
> 

Yes, those cases are why we kept the ‘current’ idea around instead of deprecating it completely. Note that the discrete progress objects still totally embrace the composition pattern that NSProgress is all about. We went into some detail about this in the WWDC presentation.

>> Yes, agreed. I would encourage the discrete progress if possible, but
>> I’ve had lots of requests for a block-based version of
>> becomeCurrent. This is another good reason to add one.
> 
> I've been using this rather simple one, which alleviates things quite a bit:
> 
> (caveat: written from memory in Mail, as I'm not at my main development machine at the moment)
> 
> extension NSProgress {
>    public func doWithPendingUnitCount<T>(unitCount: Int64, block: () throws -> T) rethrows -> T {
>        self.becomeCurrentWithPendingUnitCount(unitCount)
>        defer { self.resignCurrent() }
>        return try block()
>    }
> }
> 
> It does not, however, address the performance concerns, nor is it quite as clean (IMO of course) as my suggestion.

I see you filed a bug for this - Thanks for that, and I’ll take it into consideration.

> 
>> I changed the algorithm for how we add child progress objects, so now
>> we only will add the first one. This wasn’t strictly backwards
>> compatible, but it was far too easy to add more progress objects than
>> you intended.
> 
> When did this change occur? I feel like I've managed to get the fractionCompleted to go over 1.0 this way on El Capitan, but don't remember exactly how I did it. Good to know this is being worked on, though; if I manage to get this to happen again, I'll file a Radar.

This change was made for 10.11 and 9.0.

> 
>>> 5. NSProgress posts KVO notifications on the main thread.
>> This actually isn’t true. NSProgress posts KVO notifications on the
>> thread in which the change was made.
> 
> As I mentioned in my followup post written a few minutes after the original one, this was a typo (or, perhaps, a Freudian slip, since posting on the main thread, or more generally on a dispatch queue settable by the user, is what I wish it did). Sorry about that.
> 
>>> In addition to complicating the interface and causing undefined
>>> behavior if one mistakenly binds it to a UI element without
>>> inserting something in between to forward the KVO notifications to
>>> the main thread, this also hurts performance in the worker thread,
>>> since KVO notifications carry a non-trivial performance penalty,
>>> involving not only message sends but also packaging things into an
>>> NSDictionary for the "change" parameter. Notifications for multiple
>>> properties are fired every time completedUnitCount is changed, which
>>> then can cause similar KVO notifications to occur up the family
>>> tree, depending on how many KVO observers are installed. This can
>>> show up as a non-trivial performance cost in Instruments if the
>>> NSProgress object is updated frequently enough.
>> There is a cost associated with updating progress, it’s true. We can
>> always do more to optimize it (and we continue to do so), but there
>> comes a point at which the updater of the progress object has to add
>> some intelligence of its own. This is why the documentation and
>> release notes encourage callers to avoid doing things like updating
>> the progress for each byte in a file (because that is far too many
>> updates).
> 
> It seems to me that a lot of the work for this could be handled by the progress object. Of course, you still wouldn't want to update after every byte, but you wouldn't need to be nearly as vigilant as you do currently. I also feel like it would be better for this to be in pure Swift and to use mechanisms more performant than KVO, since by its very nature this is an API that is intended to run in worker code, which is where you really do care about performance. I realize of course the need for NSProgress to exist, since a Swift-native solution would not be available to pure Objective-C code, but moving forward it would be really nice to have a Swift-native way to do this.
> 
> My ideal implementation for the progress objects would probably look something like this:
> 
> - The progress object contains some simple integer and boolean properties for the usual things: total units, completed units, and cancelled.
> 
> - When one of these properties is changed, the change is propagated to the parent progress for completed units, or to the children for cancelled. This is the only real work that's done on the current thread.
> 
> - If and only if someone has set something for at least one of our handlers:
>    * Optionally: Check if we've fired the handlers within some threshold, either time based (only fire if we haven't fired in some interval of time) or fraction-based (only fire if the fraction changed has increased enough for us to care about the change), and exit if the threshold hasn't been reached.
>    * Fire any handlers that are non-nil, on the dispatch queue which we have as a separate property (defaulting to the main thread). Calculated properties such as fractionCompleted, localizedDescription (in its default implementation), etc. are computed on the dispatch queue as well, rather on the current thread.
> 
> This way, the overhead on the current thread when nothing's watching would be limited to a few integer stores, which are fast in Swift, and some nil checks. In the case where we have handlers, we add whatever it costs to run dispatch_async (and some floating-point operations if we implement the coalescing behavior). This should be much more performant than all the dynamic lookups, message sends, change dictionary building, multiple string operations (I'm finding that the KVO notification for localizedDescription seems to get called twice per change when I test it, busting out NSNumberFormatter to rebuild the "X% completed" string each time), and other overhead that we have now.
> 
> I could of course end up writing my own implementation, and I may end up doing that. It would be nice to have something like this available as part of the standard distribution, though.
> 
> Charles
> 

NSProgress actually has some throttling in place for cross-process progress, and I’ve found it to be very tricky to get right. As far as dropping notifications on the floor, you have to be careful that they aren’t the ones that certain clients care a lot about (e.g., getting to 100% finished). If attempting to limit it to a certain percentage, remember that any given progress object’s idea of a significant percentage may be an insignificant one for the parent progress object - and therefore too frequent. Some clients may really want every progress update and some will want it limited to something like 60Hz.

I don’t think anything about this problem is going to be solved by simply reimplementing the entire thing in Swift. Ditching KVO may seem like a way to increase performance, but then you have to think about the global picture - how is anyone supposed to know when the progress has been updated? You will likely have to invent a different notification mechanism to replace it. At that point you’ve reinvented a chunk of KVO itself instead of focusing on the primary domain of progress reporting. Plus, the notification mechanism we’ve invented is specific to NSProgress and not general enough to plug into other systems effectively.

- Tony