Show _all_ stations of an agency in the table, optimized for decent p…

…erformance

Squashed commit of the following:

commit dc116d0eb5936cf9bf717b61f94ce42c73226ab0
Author: Elliott Williams <[email protected]>
Date:   Sat Oct 14 15:12:18 2017 -0500

    Performance: Arrival fetching only when cell shown, fetch processed on a worker thread

commit cadcf019fefa59ebf82997a4aed75b2dcc31e9bf
Author: Elliott Williams <[email protected]>
Date:   Fri Oct 13 11:03:41 2017 -0500

    Performance: Only show stations on the map past a certain zoom

commit 26ddf218d94e19f57a6b42ffe2dadfc891d0b632
Author: Elliott Williams <[email protected]>
Date:   Fri Oct 13 11:02:35 2017 -0500

    Performance: Make Stations annotations, don't animate some table updates

commit 849caaa2970e45fc8bdbc2160c37d1cb1cbf76c9
Author: Elliott Williams <[email protected]>
Date:   Wed Oct 11 15:55:37 2017 -0500

    Show all stations in the table, regardless of map region

Proper/Controllers/POIMapViewController.swift

@@ -27,8 +27,12 @@ class POIMapViewController: UIViewController, ProperViewController {
  let isUserLocation: Property<Bool>
  var staticCenter: MKPointAnnotation? = nil
 
- fileprivate var annotationForView = NSMapTable<MKAnnotationView, POIStationAnnotation>()
- fileprivate var annotationForStation = [MutableStation: POIStationAnnotation]()
+ var shouldShowStationAnnotations: Bool {
+ // Hide station annotations above 2km
+ return map.region.span.latitudeDelta < 2.0/111
+ }
+
+ fileprivate var stationForAnnotationView = NSMapTable<MKAnnotationView, MutableStation>()
  fileprivate var polylines = [MutableRoute: MKPolyline]()
  fileprivate var routeForPolyline = [MKPolyline: MutableRoute]()
  fileprivate let updateRegionLock = NSLock()
@@ -101,16 +105,11 @@ class POIMapViewController: UIViewController, ProperViewController {
  }
  }
 
- disposable += stations.producer.combinePrevious([]).startWithValues { prev, next in
- let diff = Dwifft.diff(prev, next)
- for step in diff {
- switch step {
- case let .insert(_, station):
- self.addAnnotation(for: station)
- case let .delete(_, station):
- self.deleteAnnotations(for: station)
- }
- }
+ disposable += stations.producer
+ .filter({ _ in self.shouldShowStationAnnotations })
+ .map(Set.init).combinePrevious(Set()).startWithValues { prev, next in
+ self.map.removeAnnotations(prev.filter(next.contains))
+ self.map.addAnnotations(next.filter(prev.contains))
  }
 
  disposable += routes.producer.flatMap(.latest, transform: { routes -> SignalProducer<Void, NoError> in
@@ -153,32 +152,18 @@ class POIMapViewController: UIViewController, ProperViewController {
  }).map({ _, _ in () })
  }
 
- func addAnnotation(for station: MutableStation) {
- guard let position = station.position.value else {
- return
- }
- let distanceString = POIViewModel.distanceString(self.center.producer.map({ ($0, position) }))
- let annotation = POIStationAnnotation(station: station,
- locatedAt: position,
- distance: distanceString)
- map.addAnnotation(annotation)
- annotationForStation[station] = annotation
- }
-
- func deleteAnnotations(for station: MutableStation) {
- if let annotation = annotationForStation.removeValue(forKey: station) {
- map.removeAnnotation(annotation)
- }
+ func contains(point: Point) -> Bool {
+ return MKMapRectContainsPoint(map.visibleMapRect, MKMapPoint(point: point))
  }
 }
 
 // MARK: - Map view delegate
 extension POIMapViewController: MKMapViewDelegate {
  func mapView(_ mapView: MKMapView, viewFor annotation: MKAnnotation) -> MKAnnotationView? {
- if let annotation = annotation as? POIStationAnnotation {
+ if let annotation = annotation as? MutableStation {
  let view = mapView.dequeueReusableAnnotationView(withIdentifier: "station") as? MKMarkerAnnotationView ??
  MKMarkerAnnotationView(annotation: annotation, reuseIdentifier: "station")
- annotationForView.setObject(annotation, forKey: view)
+ stationForAnnotationView.setObject(annotation, forKey: view)
  return view
  }
 
@@ -200,8 +185,8 @@ extension POIMapViewController: MKMapViewDelegate {
  }
 
  func mapView(_ mapView: MKMapView, didSelect view: MKAnnotationView) {
- if let annotation = annotationForView.object(forKey: view) {
- disposable += onSelect.apply(annotation.station).start()
+ if let station = stationForAnnotationView.object(forKey: view) {
+ disposable += onSelect.apply(station).start()
  }
  }
 
@@ -211,8 +196,14 @@ extension POIMapViewController: MKMapViewDelegate {
  }
  // TODO: I shouldn't update the center point, because that's based on the nearby view's representative location.
  // But I should make sure whatever region shown by the map is being searched.
- center.swap(Point(coordinate: mapView.region.center))
- zoom.swap(mapView.region.span)
+// center.swap(Point(coordinate: mapView.region.center))
+// zoom.swap(mapView.region.span)
  updateRegionLock.unlock()
+
+ if shouldShowStationAnnotations {
+ map.addAnnotations(self.stations.value)
+ } else {
+ map.removeAnnotations(self.stations.value)
+ }
  }
 }

Proper/Controllers/POITableViewController.swift

@@ -21,8 +21,7 @@ class POITableViewController: UITableViewController, ProperViewController {
  var disposable = CompositeDisposable()
 
  fileprivate let config: ConfigSP
- fileprivate var headerDisposables: [UIView: Disposable] = [:]
- fileprivate var headerBadges: [UIView: Badge] = [:]
+ fileprivate var fetchActions: [Int: ScopedDisposable<AnyDisposable>] = [:]
  fileprivate let stations: Property<[MutableStation]>
 
  lazy var dataSource: POITableDataSource = {
@@ -37,10 +36,13 @@ class POITableViewController: UITableViewController, ProperViewController {
 
  tableView.translatesAutoresizingMaskIntoConstraints = false
  tableView.dataSource = dataSource
+ tableView.allowsSelection = false
  tableView.register(UINib(nibName: "ArrivalTableViewCell", bundle: nil),
  forCellReuseIdentifier: "arrivalCell")
  tableView.register(UINib(nibName: "POIStationHeaderFooterView", bundle: nil),
  forHeaderFooterViewReuseIdentifier: "stationHeader")
+ tableView.register(UITableViewCell.self, forCellReuseIdentifier: "loading")
+ tableView.register(UITableViewCell.self, forCellReuseIdentifier: "none")
  }
 
  required init?(coder aDecoder: NSCoder) {
@@ -69,6 +71,32 @@ extension POITableViewController {
  }
 }
 
+// MARK: Private
+private extension POITableViewController {
+ func stopFetching(ifSectionObscured section: Int) {
+ guard let visibleSections = tableView.indexPathsForVisibleRows?.map({ $0.section }) else {
+ // Release all fetch actions if no sections are being shown.
+ fetchActions = [:]
+ return
+ }
+
+ if !Set(visibleSections).contains(section) {
+ // Release the ScopedDisposable, disposing the fetch action.
+ fetchActions[section] = nil
+ }
+ }
+
+ func startFetching(ifSectionIsVisible section: Int) {
+ guard fetchActions[section] == nil else {
+ return
+ }
+ let station = dataSource.station(at: section)
+ let fetchAction = dataSource.fetchArrivals(for: station).startWithFailed(displayError(_:))
+ let stationTopic = station.producer.startWithFailed(displayError(_:))
+ fetchActions[section] = ScopedDisposable(CompositeDisposable([fetchAction, stationTopic]))
+ }
+}
+
 // MARK: Table View Delegate
 extension POITableViewController {
  override func tableView(_ tableView: UITableView, heightForRowAt indexPath: IndexPath) -> CGFloat {
@@ -97,13 +125,19 @@ extension POITableViewController {
  return POITableViewController.headerViewHeight
  }
 
+ override func tableView(_ tableView: UITableView, willDisplay cell: UITableViewCell, forRowAt indexPath: IndexPath) {
+ startFetching(ifSectionIsVisible: indexPath.section)
+ }
+
  override func tableView(_ tableView: UITableView, willDisplayHeaderView view: UIView, forSection section: Int) {
- let station = dataSource.station(at: section)
- let disposable = dataSource.fetchArrivals(for: station).startWithFailed(displayError(_:))
- headerDisposables[view] = disposable
+ startFetching(ifSectionIsVisible: section)
  }
 
  override func tableView(_ tableView: UITableView, didEndDisplayingHeaderView view: UIView, forSection section: Int) {
- headerDisposables.removeValue(forKey: view)?.dispose()
+ stopFetching(ifSectionObscured: section)
+ }
+
+ override func tableView(_ tableView: UITableView, didEndDisplaying cell: UITableViewCell, forRowAt indexPath: IndexPath) {
+ stopFetching(ifSectionObscured: indexPath.section)
  }
 }

Proper/Controllers/POIViewController.swift

@@ -33,8 +33,8 @@ class POIViewController: UIViewController, ProperViewController, UISearchControl
  return property
  }()
 
- /// The area represented by the map, which stations are searched for within.
- lazy var zoom = MutableProperty<MKCoordinateSpan>(MKCoordinateSpan(latitudeDelta: 0.1, longitudeDelta: 0.15)) // Default zoom
+ /// The area represented by the map.
+ lazy var zoom = MutableProperty<MKCoordinateSpan>(MKCoordinateSpan(latitudeDelta: 0.1, longitudeDelta: 0.15)) // Default zoom, city-wide
 
  lazy var isUserLocation = MutableProperty(true)
 
@@ -127,7 +127,7 @@ class POIViewController: UIViewController, ProperViewController, UISearchControl
  bar.setBackgroundImage(UIImage(), for: UIBarMetrics.default)
  }
 
- let searchProducer = point.producer.combineLatest(with: zoom.producer.map({ $0 * 1.3 })) // widen search radius
+ let searchProducer = point.producer
  .observe(on: searchScheduler)
  .throttle(0.5, on: searchScheduler)
  .logEvents(identifier: "NearbyStationsViewModel searchProducer",
@@ -148,8 +148,10 @@ class POIViewController: UIViewController, ProperViewController, UISearchControl
  disposable += location.startWithResult { result in
  switch result {
  case let .success(point, name, isUserLocation):
- self.point.swap(point)
- self.navigationItem.title = name
+ if self.mapController.contains(point: point) {
+ self.point.swap(point)
+ self.navigationItem.title = name
+ }
  self.isUserLocation.swap(isUserLocation)
  case let .failure(error):
  self.displayError(error)

Proper/Data Sources/POITableDataSource.swift

@@ -13,12 +13,13 @@ import Dwifft
 
 class POITableDataSource: NSObject {
  let stations: Property<[MutableStation]>
- let diffCalculator: TableViewDiffCalculator<MutableStation, Arrival>
+ fileprivate var diffCalculator: TableViewDiffCalculator<MutableStation, ArrivalState>
  let config: ConfigSP
 
  fileprivate let tableView: UITableView
  private let disposable = ScopedDisposable(CompositeDisposable())
  private let fetchScheduler = QueueScheduler(qos: .userInitiated, name: "POITableDataSource.fetchScheduler")
+ private let defaultArrivals = Array(repeating: ArrivalState.loading, count: 1)
 
  init(tableView: UITableView, stations: Property<[MutableStation]>, config: ConfigSP) {
  self.tableView = tableView
@@ -28,8 +29,9 @@ class POITableDataSource: NSObject {
  super.init()
 
  disposable += stations.producer.startWithValues { [weak self] stations in
+ let shouldAnimate = tableView.numberOfSections != 0 || stations.isEmpty
  // Populate the table with nearby stations.
- self?.update(withStations: stations)
+ self?.update(withStations: stations, animated: shouldAnimate)
  }
  }
 
@@ -52,36 +54,30 @@ class POITableDataSource: NSObject {
  }
 
  private func update(section: MutableStation, with newArrivals: [Arrival]) {
- let replacement = diffCalculator.sectionedValues.sectionsAndValues.map({ station, arrivals -> (MutableStation, [Arrival]) in
- if station == section {
- return (station, newArrivals)
- } else {
- return (station, arrivals)
- }
+ let wrappedArrivals = newArrivals.isEmpty ? [ArrivalState.none] : newArrivals.map({ ArrivalState.loaded($0) })
+ let replacement = diffCalculator.sectionedValues.sectionsAndValues.map({ station, currentArrivals in
+ (station, station == section ? wrappedArrivals : currentArrivals)
  })
 
  diffCalculator.sectionedValues = SectionedValues(replacement)
  }
 
- private func update(withStations stations: [MutableStation]) {
- let knownArrivals: [MutableStation: [Arrival]] = diffCalculator.sectionedValues.sectionsAndValues.reduce(into: [:]) { (dict, tuple) in
+ private func update(withStations stations: [MutableStation], animated: Bool) {
+ let knownArrivals: [MutableStation: [ArrivalState]] = diffCalculator.sectionedValues.sectionsAndValues.reduce(into: [:]) { (dict, tuple) in
  let (station, arrivals) = tuple
  dict[station] = arrivals
  }
- let replacement = stations.map({ station in (station, knownArrivals[station] ?? []) })
- diffCalculator.sectionedValues = SectionedValues(replacement)
- for idx in stations.indices {
- colorHeader(at: idx, with: ColorBrewer.purpleRed)
+ let replacement = stations.map({ station in (station, knownArrivals[station] ?? defaultArrivals) })
+ if animated {
+ diffCalculator.sectionedValues = SectionedValues(replacement)
+ } else {
+ let tableView = diffCalculator.tableView
+ diffCalculator = TableViewDiffCalculator(tableView: tableView,
+ initialSectionedValues: SectionedValues(replacement))
+ tableView?.reloadData()
  }
  }
 
- private func colorHeader(at idx: Int, with scheme: ColorBrewer) {
- guard let header = tableView.headerView(forSection: idx) as? POIStationHeaderFooterView else {
- return
- }
- header.color = colorForHeader(at: idx, with: scheme)
- }
-
  func colorForHeader(at idx: Int, with scheme: ColorBrewer) -> UIColor {
  let idx = CGFloat(idx)
  let n = CGFloat(numberOfSections(in: tableView))
@@ -99,13 +95,13 @@ extension POITableDataSource {
  return diffCalculator.sectionedValues.sectionsAndValues.index(where: { station, _ in station == section })
  }
 
- func arrival(at indexPath: IndexPath) -> Arrival {
- return diffCalculator.value(atIndexPath: indexPath)
+ func arrival(at indexPath: IndexPath) -> Arrival? {
+ return diffCalculator.value(atIndexPath: indexPath).arrival
  }
 
  func arrivals(atSection index: Int) -> [Arrival] {
- let (_, arrivals) =  diffCalculator.sectionedValues.sectionsAndValues[index]
- return arrivals
+ let (_, arrivals) = diffCalculator.sectionedValues.sectionsAndValues[index]
+ return arrivals.flatMap({ $0.arrival })
  }
 }
 
@@ -120,9 +116,46 @@ extension POITableDataSource: UITableViewDataSource {
  }
 
  func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
- let cell = tableView.dequeueReusableCell(withIdentifier: "arrivalCell") as! ArrivalTableViewCell
- let arrival = diffCalculator.value(atIndexPath: indexPath)
- cell.apply(arrival: arrival)
- return cell
+ switch diffCalculator.value(atIndexPath: indexPath) {
+ case .loaded(let arrival):
+ let cell = tableView.dequeueReusableCell(withIdentifier: "arrivalCell") as! ArrivalTableViewCell
+ let route = station(at: indexPath.section).routes.producer.map({ routes in
+ routes.first(where: { $0 == arrival.route })
+ }).skipNil()
+ cell.apply(arrival: arrival, route: route)
+ return cell
+ case .loading:
+ let cell = tableView.dequeueReusableCell(withIdentifier: "loading")!
+ let activityIndicator = UIActivityIndicatorView(activityIndicatorStyle: .gray)
+ activityIndicator.startAnimating()
+ cell.accessoryView = activityIndicator
+ return cell
+ case .none:
+ let cell = tableView.dequeueReusableCell(withIdentifier: "none")!
+ cell.textLabel?.text = "No upcoming departures."
+ return cell
+ }
+ }
+}
+
+private enum ArrivalState: Equatable {
+ case loaded(Arrival)
+ case loading
+ case none
+
+ var arrival: Arrival? {
+ switch self {
+ case .loaded(let arrival): return arrival
+ default: return nil
+ }
+ }
+
+ static func == (a: ArrivalState, b: ArrivalState) -> Bool {
+ switch (a, b) {
+ case (.loaded(let a), .loaded(let b)): return a == b
+ case (.loading, .loading): return true
+ case (.none, .none): return true
+ default: return false
+ }
  }
 }