uber / RIBs

Uber's cross-platform mobile architecture framework.
https://eng.uber.com/tag/ribs/
Apache License 2.0
7.75k stars 903 forks source link

Add more Routing lifecycle events to support RIB Tree tracing #373

Open dangthaison91 opened 4 years ago

dangthaison91 commented 4 years ago

We are trying to improve the overall performance of our app. The first thing we think we should do beside of manual Profiling is tracking performance for specific code.

For UIView/UIViewController, swizzle help to automatically record a trace then send to a Telemetry tracing API. We also want to do that for Interactor/Router but they are pure Swift so no swizzle at all.

We tried to do that by adding more lifecycle events for Router and Interactor like below:

Router:

/// The lifecycle stages of a router scope.
public enum RouterLifecycle {

    /// Router did load.
    case didLoad

    /// Router will attach child
    case willAttachChild(Routing)

    /// Router did attach child
    case didAttachChild(Routing)

    /// Router will detach child
    case willDetachChild(Routing)

    /// Router did detach child
    case didDetachChild(Routing)
}

open class Router<InteractorType>: Routing {
    /// Attaches the given router as a child.
    ///
    /// - parameter child: The child `Router` to attach.
    public final func attachChild(_ child: Routing) {
        assert(!(children.contains { $0 === child }), "Attempt to attach child: \(child), which is already attached to \(self).")
        lifecycleSubject.onNext(.willAttachChild(child))

        children.append(child)

        // Activate child first before loading. Router usually attaches immutable children in didLoad.
        // We need to make sure the RIB is activated before letting it attach immutable children.
        child.interactable.activate()
        child.load()

        lifecycleSubject.onNext(.didAttachChild(child))
    }

    /// Detaches the given `Router` from the tree.
    ///
    /// - parameter child: The child `Router` to detach.
    public final func detachChild(_ child: Routing) {
        lifecycleSubject.onNext(.willDetachChild(child))

        child.interactable.deactivate()
        children.removeElementByReference(child)

        lifecycleSubject.onNext(.didDetachChild(child))
    }
}

Then we can do recursive observe for child RIBs lifecycle to add performance trace. This enables as we can add observer only one in Root:

RootRouter:

func observeRouter(_ router: Routing) {
    let didBecomeActive = router.interactable.isActiveStream.skip(1).filter { $0 == false }

    let willAttachChild = router
        .lifecycle
        .compactMap { lifecycle -> Routing? in
            if case .willAttachChild(let child) = lifecycle {
                return child
            } else {
                return nil
            }
        }
        .takeUntil(didBecomeActive)

    let didAttachChild = router
        .lifecycle
        .compactMap { lifecycle -> Routing? in
            if case .didAttachChild(let child) = lifecycle {
                return child
            } else {
                return nil
            }
        }
        .takeUntil(didBecomeActive)

    _ = willAttachChild
        .subscribe(onNext: { child in            
            /// Recursive observe child router
            observeRouter(child)
        })

    _ = willAttachChild
        .flatMap { child -> Observable<(Routing)> in
            return child.interactable.isActiveStream.filter { $0 }.map { _ in return child }
        }
        .subscribe(onNext: { child in
            /// Start Performance Tracing Interator didBecomeActive
        })

    _ = didAttachChild
        .subscribe(onNext: { child in
            /// Stop Performance Tracing here
        })
}

We can simply add observer at the Root RIB and for example, use os_signpost to tracing performance in Instrument:

Screen Shot 2020-07-21 at 00 16 41

Adding more lifecycle event for Router also help to easier to implement a RIB Tree real-time viewer like https://github.com/srea/RIBsTreeViewerClient or https://github.com/imairi/RIBsTreeMaker

I really want to know your ideas and willing to open a PR if this will make sense to include in Uber RIBs

ermik commented 4 years ago

I think this should be implemented via Observable, not individual delegate methods. Also, I wouldn't allow to pass Routing object reference around, I would limit it to a String:

String(describing: type(of: router))

or perhaps a struct type implementing a protocol:

public protocol RIBInfo {
    var name: String { get }
    var childrenCount: Int { get }
    var isActive: Bool { get }
}