Skip to content

Commit 469a939

Browse files
committed
attempt at dirty flag - need to try again
1 parent c113a98 commit 469a939

File tree

1 file changed

+215
-36
lines changed

1 file changed

+215
-36
lines changed

Sources/FoundationEssentials/ProgressManager/ProgressManager.swift

Lines changed: 215 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ internal struct FractionState {
2828
selfFraction + childFraction
2929
}
3030
var interopChild: ProgressManager? // read from this if self is actually an interop ghost
31+
32+
var isDirty: Bool = false // Flag to indicate fraction computation needs update
33+
var isProcessing: Bool = false
34+
var pendingCompletedCount: Int?
35+
var dirtyChildren: Set<ProgressManager> = Set() // Track which children are dirty
3136
}
3237

3338
internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
@@ -78,7 +83,7 @@ internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
7883
public var totalCount: Int? {
7984
_$observationRegistrar.access(self, keyPath: \.totalCount)
8085
return state.withLock { state in
81-
getTotalCount(fractionState: &state.fractionState)
86+
return getTotalCount(state: &state)
8287
}
8388
}
8489

@@ -87,8 +92,9 @@ internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
8792
public var completedCount: Int {
8893
_$observationRegistrar.access(self, keyPath: \.completedCount)
8994
return state.withLock { state in
90-
getCompletedCount(fractionState: &state.fractionState)
95+
return getCompletedCount(state: &state)
9196
}
97+
9298
}
9399

94100
/// The proportion of work completed.
@@ -97,7 +103,7 @@ internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
97103
public var fractionCompleted: Double {
98104
_$observationRegistrar.access(self, keyPath: \.fractionCompleted)
99105
return state.withLock { state in
100-
getFractionCompleted(fractionState: &state.fractionState)
106+
getFractionCompleted(state: &state)
101107
}
102108
}
103109

@@ -106,7 +112,7 @@ internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
106112
public var isIndeterminate: Bool {
107113
_$observationRegistrar.access(self, keyPath: \.isIndeterminate)
108114
return state.withLock { state in
109-
getIsIndeterminate(fractionState: &state.fractionState)
115+
getIsIndeterminate(state: &state)
110116
}
111117
}
112118

@@ -115,7 +121,7 @@ internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
115121
public var isFinished: Bool {
116122
_$observationRegistrar.access(self, keyPath: \.isFinished)
117123
return state.withLock { state in
118-
getIsFinished(fractionState: &state.fractionState)
124+
getIsFinished(state: &state)
119125
}
120126
}
121127

@@ -141,7 +147,7 @@ internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
141147
/// The total units of work.
142148
public var totalCount: Int? {
143149
mutating get {
144-
manager.getTotalCount(fractionState: &state.fractionState)
150+
manager.getTotalCount(state: &state)
145151
}
146152

147153
set {
@@ -172,15 +178,15 @@ internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
172178
/// The completed units of work.
173179
public var completedCount: Int {
174180
mutating get {
175-
manager.getCompletedCount(fractionState: &state.fractionState)
181+
manager.getCompletedCount(state: &state)
176182
}
177183

178184
set {
179-
let prev = state.fractionState.overallFraction
180-
state.fractionState.selfFraction.completed = newValue
181-
manager.updateFractionCompleted(from: prev, to: state.fractionState.overallFraction)
182-
manager.ghostReporter?.notifyObservers(with: .fractionUpdated)
185+
//TODO: I am scared, this might introduce recursive lock again
186+
// let existingCompleted = state.fractionState.selfFraction.completed
187+
// manager.complete(count: newValue - existingCompleted)
183188

189+
manager.ghostReporter?.notifyObservers(with: .fractionUpdated)
184190
manager.monitorInterop.withLock { [manager] interop in
185191
if interop == true {
186192
manager.notifyObservers(with: .fractionUpdated)
@@ -306,8 +312,20 @@ internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
306312
/// Increases `completedCount` by `count`.
307313
/// - Parameter count: Units of work.
308314
public func complete(count: Int) {
309-
let updateState = updateCompletedCount(count: count)
310-
updateFractionCompleted(from: updateState.previous, to: updateState.current)
315+
// First update pendingCompletedCount
316+
updatePendingCompletedCount(increment: count)
317+
318+
// Then mark self to be dirty
319+
state.withLock { state in
320+
state.fractionState.isDirty = true
321+
}
322+
323+
// Add self to all parent's dirtyChildren list & mark parents as dirty recursively
324+
parents.withLock { parents in
325+
for (parent, _) in parents {
326+
parent.addDirtyChild(self)
327+
}
328+
}
311329

312330
// Interop updates stuff
313331
ghostReporter?.notifyObservers(with: .fractionUpdated)
@@ -371,24 +389,28 @@ internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
371389
//MARK: ProgressManager Properties getters
372390
/// Returns nil if `self` was instantiated without total units;
373391
/// returns a `Int` value otherwise.
374-
private func getTotalCount(fractionState: inout FractionState) -> Int? {
375-
if let interopChild = fractionState.interopChild {
392+
private func getTotalCount(state: inout State) -> Int? {
393+
if let interopChild = state.fractionState.interopChild {
376394
return interopChild.totalCount
377395
}
378-
if fractionState.indeterminate {
396+
if state.fractionState.indeterminate {
379397
return nil
380398
} else {
381-
return fractionState.selfFraction.total
399+
return state.fractionState.selfFraction.total
382400
}
383401
}
384402

385403
/// Returns 0 if `self` has `nil` total units;
386404
/// returns a `Int` value otherwise.
387-
private func getCompletedCount(fractionState: inout FractionState) -> Int {
388-
if let interopChild = fractionState.interopChild {
405+
private func getCompletedCount(state: inout State) -> Int {
406+
if let interopChild = state.fractionState.interopChild {
389407
return interopChild.completedCount
390408
}
391-
return fractionState.selfFraction.completed
409+
410+
// Trigger updates all the way from leaf
411+
processDirtyStates(state: &state)
412+
413+
return state.fractionState.selfFraction.completed
392414
}
393415

394416
/// Returns 0.0 if `self` has `nil` total units;
@@ -397,30 +419,30 @@ internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
397419
///
398420
/// The calculation of fraction completed for a ProgressManager instance that has children
399421
/// will take into account children's fraction completed as well.
400-
private func getFractionCompleted(fractionState: inout FractionState) -> Double {
401-
if let interopChild = fractionState.interopChild {
422+
private func getFractionCompleted(state: inout State) -> Double {
423+
if let interopChild = state.fractionState.interopChild {
402424
return interopChild.fractionCompleted
403425
}
404-
if fractionState.indeterminate {
426+
if state.fractionState.indeterminate {
405427
return 0.0
406428
}
407-
guard fractionState.selfFraction.total > 0 else {
408-
return fractionState.selfFraction.fractionCompleted
429+
guard state.fractionState.selfFraction.total > 0 else {
430+
return state.fractionState.selfFraction.fractionCompleted
409431
}
410-
return (fractionState.selfFraction + fractionState.childFraction).fractionCompleted
432+
return (state.fractionState.selfFraction + state.fractionState.childFraction).fractionCompleted
411433
}
412434

413435

414436
/// Returns `true` if completed and total units are not `nil` and completed units is greater than or equal to total units;
415437
/// returns `false` otherwise.
416-
private func getIsFinished(fractionState: inout FractionState) -> Bool {
417-
return fractionState.selfFraction.isFinished
438+
private func getIsFinished(state: inout State) -> Bool {
439+
return state.fractionState.selfFraction.isFinished
418440
}
419441

420442

421443
/// Returns `true` if `self` has `nil` total units.
422-
private func getIsIndeterminate(fractionState: inout FractionState) -> Bool {
423-
return fractionState.indeterminate
444+
private func getIsIndeterminate(state: inout State) -> Bool {
445+
return state.fractionState.indeterminate
424446
}
425447

426448
//MARK: FractionCompleted Calculation methods
@@ -429,16 +451,173 @@ internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
429451
let current: _ProgressFraction
430452
}
431453

432-
private func updateCompletedCount(count: Int) -> UpdateState {
454+
// Called only by complete(count:)
455+
private func updatePendingCompletedCount(increment count: Int) {
456+
print("called update pending completed count")
433457
// Acquire and release child's lock
434-
let (previous, current) = state.withLock { state in
435-
let prev = state.fractionState.overallFraction
436-
state.fractionState.selfFraction.completed += count
437-
return (prev, state.fractionState.overallFraction)
458+
state.withLock { state in
459+
// If there was a pending update before
460+
if let updatedCompletedCount = state.fractionState.pendingCompletedCount {
461+
state.fractionState.pendingCompletedCount = updatedCompletedCount + count
462+
} else {
463+
// If this is the first pending update
464+
state.fractionState.pendingCompletedCount = state.fractionState.selfFraction.completed + count
465+
}
466+
}
467+
}
468+
469+
private func addDirtyChild(_ child: ProgressManager) {
470+
print("called dirty child")
471+
state.withLock { state in
472+
// If already dirty, don't continue adding
473+
if state.fractionState.dirtyChildren.contains(child) {
474+
return
475+
} else {
476+
state.fractionState.dirtyChildren.insert(child)
477+
state.fractionState.isDirty = true
478+
}
479+
}
480+
481+
parents.withLock { parents in
482+
for (parent, _) in parents {
483+
parent.addDirtyChild(child)
484+
}
485+
}
486+
}
487+
488+
private func processDirtyStates(state: inout State) {
489+
if state.fractionState.isProcessing { return }
490+
491+
if !state.fractionState.isDirty { return }
492+
493+
state.fractionState.isProcessing = true
494+
495+
496+
// Collect dirty nodes in DFS order
497+
let nodesToProcess = collectAllDirtyNodes(state: &state)
498+
499+
// Process in bottom-up order
500+
for node in nodesToProcess.reversed() {
501+
node.processLocalState()
502+
}
503+
504+
state.fractionState.isProcessing = false
505+
506+
}
507+
508+
private func collectAllDirtyNodes(state: inout State) -> [ProgressManager] {
509+
var result = [ProgressManager]()
510+
var visited = Set<ProgressManager>()
511+
collectDirtyNodesRecursive(&result, visited: &visited, state: &state)
512+
return result.reversed()
513+
}
514+
515+
private func collectDirtyNodesRecursive(_ result: inout [ProgressManager], visited: inout Set<ProgressManager>, state: inout State) {
516+
if visited.contains(self) { return }
517+
visited.insert(self)
518+
519+
let dirtyChildren = state.fractionState.dirtyChildren
520+
521+
// TODO: Any danger here because children can be concurrently added?
522+
523+
for dirtyChild in dirtyChildren {
524+
dirtyChild.state.withLock { state in
525+
dirtyChild.collectDirtyNodesRecursive(&result, visited: &visited, state: &state)
526+
}
527+
}
528+
529+
// if state.fractionState.isDirty {
530+
result.append(self)
531+
// }
532+
}
533+
534+
private func processLocalState() {
535+
// 1. Apply our own pending completed count first
536+
537+
538+
state.withLock { state in
539+
// Update our own self fraction
540+
if let updatedCompletedCount = state.fractionState.pendingCompletedCount {
541+
state.fractionState.selfFraction.completed = updatedCompletedCount
542+
}
543+
// IMPORTANT: We don't need to update our parent here
544+
// Parents will call updateFractionForChild on us later
545+
}
546+
547+
548+
// 2. Get dirty children before clearing the set
549+
let dirtyChildren = state.withLock { state in
550+
let children = Array(state.fractionState.dirtyChildren)
551+
state.fractionState.dirtyChildren.removeAll()
552+
return children
553+
}
554+
555+
// 3. Reset our child fraction since we'll recalculate it
556+
state.withLock { state in
557+
state.fractionState.childFraction = _ProgressFraction(completed: 0, total: 0)
558+
}
559+
560+
// 4. Update for each dirty child
561+
for child in dirtyChildren {
562+
// THIS is where we update our childFraction based on each child
563+
updateFractionForChild(child)
564+
}
565+
566+
// 5. Recalculate overall fraction
567+
// recalculateOverallFraction()
568+
569+
// 6. Clear dirty flag
570+
state.withLock { state in
571+
state.fractionState.isDirty = false
572+
}
573+
}
574+
575+
// THIS is the key method where parent updates its childFraction based on a child
576+
private func updateFractionForChild(_ child: ProgressManager) {
577+
// Get child's CURRENT fraction (which includes its updated selfFraction)
578+
let childFraction = child.getCurrentFraction()
579+
580+
// Get the portion assigned to this child
581+
let childPortion = getChildPortion(child)
582+
583+
state.withLock { state in
584+
// Calculate the child's contribution to our progress
585+
let multiplier = _ProgressFraction(completed: childPortion, total: state.fractionState.selfFraction.total)
586+
587+
// Add child's contribution to our childFraction
588+
// This is how our childFraction gets updated based on the child's state
589+
state.fractionState.childFraction = state.fractionState.childFraction + (childFraction * multiplier)
590+
}
591+
}
592+
593+
// Helper to get a child's current fraction WITHOUT triggering processing
594+
internal func getCurrentFraction() -> _ProgressFraction {
595+
return state.withLock { state in
596+
// Direct access to fraction state without processing
597+
return state.fractionState.overallFraction
438598
}
439-
return UpdateState(previous: previous, current: current)
440599
}
441600

601+
private func getChildPortion(_ child: ProgressManager) -> Int {
602+
return child.parents.withLock { parents in
603+
for (parent, portionOfParent) in parents {
604+
if parent == self {
605+
return portionOfParent
606+
}
607+
}
608+
return 0
609+
}
610+
}
611+
612+
// Recalculate overall fraction
613+
// private func recalculateOverallFraction() {
614+
// state.withLock { state in
615+
// // Combine self fraction and child fraction
616+
// // This should make the "0" is not equal to "13" test pass
617+
// state.fractionState.overallFraction = state.fractionState.selfFraction + state.fractionState.childFraction
618+
// }
619+
// }
620+
442621
private func updateFractionCompleted(from: _ProgressFraction, to: _ProgressFraction) {
443622
_$observationRegistrar.withMutation(of: self, keyPath: \.fractionCompleted) {
444623
if from != to {

0 commit comments

Comments
 (0)