Hi all,
Following feedback from the previous review, I have developed a second pitch to introduce a new progress reporting mechanism —ProgressManager — to support progress reporting in Swift Concurrency.
Feel free to leave your feedback, thoughts, comments or questions here!
For the complete proposal, check out the full proposal on the PR to the swift-foundation repo.
I didn't participate in the first round. I read the up-to-date proposal:
Subprogress.manager as a name; my original thought was that it returned the manager that created the Subprogress, while it creates a child. I think there could be a better name out there that doesn't have that ambiguity. Subprogress.open? Subprogress.begin?withProperties interact with the potential tree of ProgressManager instances? If two subprogress instances change properties.filename, which file name do I get when I ask for it on the parent manager?We have ProgressManager.isFinished, so perhaps the corresponding function on Subprocess is start?
func f(_ subprogress: Subprogress?) async {
let manager = subprogress.start(totalCount: 10)
// ...
manager.complete(count: 3) // etc
}
As for the "barrier", I think this will be a lot more obvious to developers once we get usage into common libraries. Foundation will adopt for its own progress-reporting functions, like Data(contentsOf:).
(Meta-note: I've just relocated this topic to Related Projects > Foundation category rather than Evolution > Pitches)
Thank you for taking the tame for the second revision.
I've seen a little bit of the pervious pitch discussion about the multi-parent case but I will say that I'm very surprised that we're taking such extreme complications of the API in order to support that rather rare use-case. The previous design was very focused, clean, and simple to use and understand -- the meaning of "report" was very clean and with just the two types, things were pretty manageable.
The newly introduced "manager" type is very confusing in its relationship to the other pieces of the proposal IMHO, and it seems its introduction is only in order to support a specific very narrow use-cases that could be worked around using alternative patterns. Like reporting to two progresses at once perhaps, etc.
Subprogress and ProgressReporter are very nice as they were in the initial review already. Relying on the moveonly semantics to avoid multiple completions etc remains as nice as it was before.
The changes to how we handle cancellation, i.e. don't handle it here, are also solid and look good in this revision.
Overall I'm rather dubious if this large change to the design to support a rare edge-case really warrant the confusion and complication they introduce.
Appreciate all the work you put in to address feedbacks.
I bring some naming alternatives, with consideration.
The "-er" suffix implies that this type is of a role that does something (an object that manages something is a manager; an object that reports something is a report, etc). The suffix frames the role of the object at the center. But alternative way to look at it is what data this type represents.
Alternatives for ProgressManager ("manager" reminds me of the bad old days in the MVC world where everything that's not a M, V or C has "manager" in its name):
ProgressHandle: since this type has the exclusive rights to mutation of a progress's state, "handle" conveys that special aspect. (in context of APIs: "get me the handle for a subprogress"; "complete some unit of progress via this handle")ProgressState: I believe that's what this type is meant to be, the source of truth for a Subprogress's states? "state" in Swift commonly refers to things with value semantics. But I don't think that's a strong enough convention to make this name inadmissible.An alternative for ProgressReporter: ProgressReport. It's shorter and I think conveys the same information. (In context with previous suggestions: "get a report from the progress's handle/state"; "add this report to that progress handle/state")
I can't speak for others, but multiple consumers (ie "multi-parent trees") has come up in almost every single app-based progress reporting scenario I've encountered in recent years.
As one who pushed for multi-parent support in the original pitch and review, I'm excited to see this come back for a second pitch so quickly. Thank you for your efforts.
I think the architecture works here, but the naming confuses me a little bit. Let me see if I understand it correctly:
ProgressManager represents the overall progress of an operation. It has methods for updating the progress (either directly, or by assigning it to subprogress instances). It is also observable, so that the progress can be observed.Subprogress represents a portion of the work belonging to a parent ProgressManagerProgressReporter provides a read-only view onto the progress of a manager, and can be incorporated into existing trees. It's well suited for being exposed as a public property on a long-running asynchronous operation and consumed by others.Am I understanding correctly? I think Subprogress is well-named. But ProgressManager and ProgressReporter don't feel quite right. After all, the proposal itself says:
ProgressManageris used to report progress.
It's odd that when I want to report progress, I use a ProgressManager, and not a ProgressReporter. That confused me as I was reading it; I expected ProgressReporter to be the "star" of the show, not a secondary advanced option.
As proposed, ProgressReporter is primarily used for reading or observing some progress, without exposing the ability to modify the status of that progress. It's like a read-only view into the primary object. I wonder if we should lean into the "reporting" role of the manager.
Proposal: What if we return ProgressManager to its original name of ProgressReporter, and call the read-only view something like ProgressReporter.Status or ProgressReporter.Report or ProgressReporter.Observer? Something that clearly indicates it is reflecting the status of another progress reporter?
Also, an unrelated question about the interoperability between Progress and ProgressManager. The proposal says this:
Due to the fact that the existing
Progressassumes a tree structure instead of an acyclic graph structure of the newProgressManager, interoperability betweenProgressandProgressManagersimilarly assumes the tree structure.
Does this mean that a Progress cannot participate at all in an acyclic graph? Or does it just mean that it cannot directly have multiple parents? Is it okay if a Progress has a single ProgressManager as a parent, but then that progress manager's reporter gets added to multiple trees?
Again, thank you for your work on this!
Trying to consolidate a reply to both you and @duan here ...
@duan says:
The "-er" suffix implies that this type is of a role that does something (an object that manages something is a manager; an object that reports something is a report, etc).
Exactly, so a ProgressReporter reports progress. A ProgressManager manages progress, by either completing progress itself or delegating it to a Subprocess.
Both ProgressReporter and ProgressManager were left @Observable for simple convenience. It feels pedantic to require someone who wishes to observe the progress of a ProgressManager to get a ProgressReporter from it.
@bjhomer says:
ProgressManageris used to report progress.
I think this is mostly just an editorial error in revisions to the document. Probably it should just say:
ProgressManageris used to manage progress for an operation.
Personally, I would rather not namespace the Reporter type, because upon discussion with @chloe-yeo it seemed clear that the type now named Reporter would likely show up in quite a few APIs that have a more object-oriented flavor to them.
I'm not opposed to new ideas for the Manager name, though. I do like that it has an active voice, which I feel helps with an understanding the concept that it is intended as a private, helper, internal type and not a currency to pass around. That could just be an instinct that I alone have, though.
A Subprogress has to be consumed and converted into a ProgressManager before it is being used to report progress. Each ProgressManager keeps track of its own filename, and also keeps track of the filename of all its children.
So when you ask for filename on any ProgressManager, it will provide its own filename. To get the filename of the entire subtree with ProgressManager as a root, we call values(of:) to get an array of filename.
We can illustrate this in code as follows:
let overallManager = ProgressManager(totalCount: 2)
overallManager.withProperties { properties in
properties.filename = "FileW"
}
func doSomething(subprogress: consuming Subprogress) async {
let manager = subprogress.manager(totalCount: 1)
manager.withProperties { properties in
properties.filename = "FileX"
}
}
func doSomething2(subprogress: consuming Subprogress) async {
let manager = subprogress.manager(totalCount: 1)
manager.withProperties { properties in
properties.filename = "FileY"
}
}
// If we try to read filename of overallManager, we get its own value
let overallManagerFilename = overallManager.withProperties(\.filename)
print(overallManagerFilename) // prints "FileW"
// If we get the values of filename property of overallManager, we get an array of all filename
let subtreeFilenames = overallManager.values(of: Filename.self)
print(subtreeFilenames) // prints ["FileW", "FileX", "FileY"]
This is what I said in my post in the next sentence. I think we are on the same page.
My feedback has been this doesn't have to be the framing. Any non-value-semantic types with mutable properties manages those properties, therefore "manager" is a low-information term as a part of type/variable name.
therefore "manager" is a low-information term as a part of type/variable name.
Fair, but we can't use Progress, because it is taken, so we need some kind of suffix. I guess I'm neutral-to-negative on Handle because it also has traditionally meant something related to pointers, and State feels about as low-information as Manager to me.
How about ProgressMonitor?
As in, something that's observing other things that "make progress".
I’m a bit worried that having both Subprogress and Subprocess be two similarly-named things for very different concepts will lead to confusion/typos. Is there another name for Subprogress that would be less confusable?
How about OverallProgress instead of ProgressManager? Subprogress flows naturally from that, and that seems more like a "root" or consolidation of subprogresses.
Well, being involved in both of them, this is clearly something that I need to look at. If only I had a Swift compiler to check my comment, it would remind me that they share zero API, because one is a ~Copyable type and the other is a module name.
In other words -- I think developers other than myself will be ok. 
This means that a Progress cannot directly have multiple parents. The functionality of ProgressManager's reporter having multiple parents doesn't get affected.
Thanks everyone for chiming in with suggestions for naming! I have a few thoughts on naming, and will try to consolidate my ideas here, so feel free to leave further comments if you have any!
While it does indicate that it is reflecting the status of another ProgressManager, we moved away from nesting the types because it would read too long for a simple type.
I am do think that the term ProgressManager conveys the nature of its role correctly, but also open to weighing the choice between ProgressHandler and ProgressManager. The term ProgressManager may suggest some operation that feels more heavyweight, whereas ProgressHandler may feel more lightweight in comparison.
Interestingly, we did initially consider having the current ProgressManager as ProgressReporter and ProgressReporter as ProgressMonitor. Upon discussion we landed at the current names because it felt like ProgressReporter reads more intuitively as something that has a read-only nature. The idea behind the name ProgressManager is to convey its nature of being something that reports or aggregates progress. It "manages" progress in the sense that it chooses to either "complete" or "assign" its count to a Subprogress.
I think OverallProgress feels like a variable name more than a type name to me. I agree that ProgressManager feels like a "root", but each Subprogress also gets consumed and converted to ProgressManager to report progress. If we were to name it OverallProgress that means we would have OverallProgress everywhere which may appear confusing. I do think the idea of prefixing the term "progress" with something is interesting though, as we've mostly been considering suffixing the term "progress" so far.
I totally hear and share your concern, but here's a common pattern that I see for current Progress clients:
public class Downloader {
// This instance is available immediately after instantiation of the download request
public var progress: Progress? { get }
// Returns a path to the download
public func download() async throws -> String
}
Clients are expected to observe the progress.
While I think it's valid to argue that they should use Subprogress pattern like in the previous pitch/proposal, which requires them to rewrite func download() to take a Subprogress as an argument, I also don't see any strong reasons for dissuading this pattern.
I also share your concern about having two similar-ish types, (temporarily) ProgressManager and ProgressReporter. This class Downloader might expose the progress as ProgressManager while they mean ProgressReporter . But that is just yet another challenge for advocating for any new API just like @Tony_Parker mentioned previously