Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move json alternative parsing away from the main thread #4729

Open
wants to merge 1 commit into
base: lts/v2
Choose a base branch
from
Open

Move json alternative parsing away from the main thread #4729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changes to the Mapbox Navigation SDK for iOS

## v2.20.0

### Routing

* Move alternative route parsing to the background thread. ([#4729](https://github.com/mapbox/mapbox-navigation-ios/pull/4729))

## v2.19.0

### Packaging
Expand All @@ -12,6 +18,7 @@
* Added handling `RouteResponse.refreshTTL` into account when refreshing a route. Now it will no longer be possible to attmept to refresh and outdated route, and `Router` will inform that current route has expired using `RouterDelegate.routerDidFailToRefreshExpiredRoute(:_)` method. ([#4672](https://github.com/mapbox/mapbox-navigation-ios/pull/4672))

### Other changes

* Fixed next banner view correctly appearing when steps list view is expanded. ([#4708](https://github.com/mapbox/mapbox-navigation-ios/pull/4708))
* Fixed rare route simulation issue where user's speed was calculated and NaN and the puck did not move. ([#4708](https://github.com/mapbox/mapbox-navigation-ios/pull/4708))
* Fixed a possibly not-updating `StepsViewController` after reroutes when using a custom top bar. ([#4716](https://github.com/mapbox/mapbox-navigation-ios/pull/4716))
Expand Down
37 changes: 29 additions & 8 deletions Sources/MapboxCoreNavigation/AlternativeRoute.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,22 @@ public struct AlternativeRoute: Identifiable {
/// The difference of expected travel time between alternative and the main routes
public let expectedTravelTimeDelta: TimeInterval

init?(mainRoute: Route, alternativeRoute nativeRouteAlternative: RouteAlternative) {
self.id = nativeRouteAlternative.id
guard let decoded = RerouteController.decode(routeRequest: nativeRouteAlternative.route.getRequestUri(),
routeResponse: nativeRouteAlternative.route.getResponseJsonRef()) else {
init?(alternativeRoute nativeRouteAlternative: RouteAlternative) {
guard let indexedRouteResponse = nativeRouteAlternative.indexedRouteResponse() else {
return nil
}
self.init(indexedRouteResponse: indexedRouteResponse, alternativeRoute: nativeRouteAlternative)
}

init?(indexedRouteResponse: IndexedRouteResponse, alternativeRoute nativeRouteAlternative: RouteAlternative) {
var indexedRouteResponse = indexedRouteResponse
indexedRouteResponse.routeIndex = Int(nativeRouteAlternative.route.getRouteIndex())
guard let mainRoute = indexedRouteResponse.currentRoute else {
return nil
}
self.id = nativeRouteAlternative.id
self.indexedRouteResponse = indexedRouteResponse

self.indexedRouteResponse = .init(routeResponse: decoded.routeResponse,
routeIndex: Int(nativeRouteAlternative.route.getRouteIndex()),
responseOrigin: nativeRouteAlternative.route.getRouterOrigin())

var legIndex = Int(nativeRouteAlternative.mainRouteFork.legIndex)
var segmentIndex = Int(nativeRouteAlternative.mainRouteFork.segmentIndex)

Expand Down Expand Up @@ -122,3 +127,19 @@ extension Route {
return leg.steps[stepindex].intersections?[intersectionIndex]
}
}

extension RouteAlternative {
func indexedRouteResponse() -> IndexedRouteResponse? {
guard let decoded =
RerouteController.decode(routeRequest: route.getRequestUri(),
routeResponse: route.getResponseJsonRef()) else {
return nil
}

return IndexedRouteResponse(
routeResponse: decoded.routeResponse,
routeIndex: Int(route.getRouteIndex()),
responseOrigin: route.getRouterOrigin()
)
}
}
56 changes: 34 additions & 22 deletions Sources/MapboxCoreNavigation/RerouteController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ class RerouteController {
}

// MARK: Internal State Management


private static let defaultParsingQueueLabel = "com.mapbox.navigation.reroute.parsing"
private let parsingQueue: DispatchQueue

private let defaultRerouteController: DefaultRerouteControllerInterface
private let rerouteDetector: RerouteDetectorInterface

Expand All @@ -92,12 +95,17 @@ class RerouteController {
}
}

required init(_ navigator: MapboxNavigationNative.Navigator, config: ConfigHandle) {
init(
_ navigator: MapboxNavigationNative.Navigator,
config: ConfigHandle,
parsingQueue: DispatchQueue = .init(label: RerouteController.defaultParsingQueueLabel, qos: .default)
) {
self.navigator = navigator
self.config = config
self.defaultRerouteController = DefaultRerouteControllerInterface(nativeInterface: navigator.getRerouteController())
self.navigator?.setRerouteControllerForController(defaultRerouteController)
self.rerouteDetector = navigator.getRerouteDetector()
self.parsingQueue = parsingQueue
self.navigator?.addRerouteObserver(for: self)
}

Expand All @@ -121,16 +129,18 @@ class RerouteController {

extension RerouteController: RerouteObserver {
func onSwitchToAlternative(forRoute route: RouteInterface) {
guard let decoded = Self.decode(routeRequest: route.getRequestUri(),
routeResponse: route.getResponseJsonRef()) else {
return
parsingQueue.async { [weak self] in
guard let self else { return }
guard let decoded = Self.decode(routeRequest: route.getRequestUri(),
routeResponse: route.getResponseJsonRef()) else {
return
}
self.delegate?.rerouteControllerWantsSwitchToAlternative(self,
response: decoded.routeResponse,
routeIndex: Int(route.getRouteIndex()),
options: decoded.routeOptions,
routeOrigin: route.getRouterOrigin())
}

delegate?.rerouteControllerWantsSwitchToAlternative(self,
response: decoded.routeResponse,
routeIndex: Int(route.getRouteIndex()),
options: decoded.routeOptions,
routeOrigin: route.getRouterOrigin())
}

func onRerouteDetected(forRouteRequest routeRequest: String) -> Bool {
Expand All @@ -156,18 +166,20 @@ extension RerouteController: RerouteObserver {
routeOrigin: origin)
self.latestRouteResponse = nil
} else {
guard let responseData = routeResponse.data(using: .utf8),
let decodedResponse = Self.decode(routeResponse: responseData,
routeOptions: decodedRequest.routeOptions,
credentials: decodedRequest.credentials) else {
delegate?.rerouteControllerDidFailToReroute(self, with: DirectionsError.invalidResponse(nil))
return
parsingQueue.async { [weak self] in
guard let self else { return }
guard let responseData = routeResponse.data(using: .utf8),
let decodedResponse = Self.decode(routeResponse: responseData,
routeOptions: decodedRequest.routeOptions,
credentials: decodedRequest.credentials) else {
self.delegate?.rerouteControllerDidFailToReroute(self, with: DirectionsError.invalidResponse(nil))
return
}
self.delegate?.rerouteControllerDidRecieveReroute(self,
response: decodedResponse,
options: decodedRequest.routeOptions,
routeOrigin: origin)
}

delegate?.rerouteControllerDidRecieveReroute(self,
response: decodedResponse,
options: decodedRequest.routeOptions,
routeOrigin: origin)
}
}

Expand Down
99 changes: 61 additions & 38 deletions Sources/MapboxCoreNavigation/RouteController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ open class RouteController: NSObject {
/// Holds currently alive instance of `RouteController`.
private static weak var instance: RouteController?
private static let instanceLock: NSLock = .init()
private static let defaultParsingQueueLabel = "com.mapbox.navigation.route.parsing"

let sessionUUID: UUID = .init()
private var isInitialized: Bool = false
Expand Down Expand Up @@ -224,7 +225,10 @@ open class RouteController: NSObject {
var routeTask: NavigationProviderRequest?

public private(set) var continuousAlternatives: [AlternativeRoute] = []


private let parsingQueue: DispatchQueue
private let delegateQueue: DispatchQueue

/**
Enables automatic switching to online version of the current route when possible.

Expand Down Expand Up @@ -285,40 +289,47 @@ open class RouteController: NSObject {
private func updateNavigator(with indexedRouteResponse: IndexedRouteResponse,
fromLegIndex legIndex: Int,
reason: RouteChangeReason,
completion: ((Result<(RouteInfo?, [AlternativeRoute]), Error>) -> Void)?) {
guard let newMainRoute = indexedRouteResponse.currentRoute,
let routesData = indexedRouteResponse.routesData(routeParserType: routeParserType) else {
completion: ((Result<Void, Error>) -> Void)?) {
guard let routesData = indexedRouteResponse.routesData(routeParserType: routeParserType) else {
completion?(.failure(RouteControllerError.failedToSerializeRoute))
return
}
sharedNavigator.setRoutes(routesData,
uuid: sessionUUID,
legIndex: UInt32(legIndex),
reason: reason,
completion: { [weak self] result in
completion: { [weak self] result in
guard let self = self else { return }

switch result {
case .success(let info):
let alternativeRoutes = info.1.compactMap {
AlternativeRoute(mainRoute: newMainRoute,
alternativeRoute: $0)
}
let alerts = routesData.primaryRoute().getRouteInfo().alerts
self.routeAlerts = alerts.reduce(into: [:]) { dictionary, alert in
let routeAlerts = alerts.reduce(into: [:]) { dictionary, alert in
dictionary[alert.roadObject.id] = alert
}
completion?(.success((info.0, alternativeRoutes)))

let removedRoutes = self.continuousAlternatives.filter { alternative in
!alternativeRoutes.contains(where: {
alternative.id == $0.id
})
self.routeAlerts = routeAlerts
completion?(.success(()))

self.parsingQueue.async { [weak self] in
guard let self else { return }
let alternativeRoutes = info.1.compactMap {
AlternativeRoute(alternativeRoute: $0)
}

self.delegateQueue.async { [weak self] in
guard let self else { return }

let removedRoutes = self.continuousAlternatives.filter { alternative in
!alternativeRoutes.contains(where: {
alternative.id == $0.id
})
}
self.continuousAlternatives = alternativeRoutes
self.report(newAlternativeRoutes: alternativeRoutes,
removedAlternativeRoutes: removedRoutes)
}
}
self.continuousAlternatives = alternativeRoutes
self.report(newAlternativeRoutes: alternativeRoutes,
removedAlternativeRoutes: removedRoutes)


case .failure(let error):
completion?(.failure(error))
}
Expand Down Expand Up @@ -677,6 +688,8 @@ open class RouteController: NSObject {

self.routeProgress = RouteProgress(route: indexedRouteResponse.currentRoute!, options: options)
self.refreshesRoute = isRouteOptions && options.profileIdentifier == .automobileAvoidingTraffic && options.refreshingEnabled
self.parsingQueue = DispatchQueue(label: RouteController.defaultParsingQueueLabel, qos: .default)
self.delegateQueue = .main

super.init()

Expand All @@ -690,7 +703,9 @@ open class RouteController: NSObject {
dataSource source: RouterDataSource,
navigatorType: CoreNavigator.Type,
routeParserType: RouteParser.Type,
navigationSessionManager: NavigationSessionManager) {
navigationSessionManager: NavigationSessionManager,
parsingQueue: DispatchQueue = .init(label: RouteController.defaultParsingQueueLabel, qos: .default),
delegateQueue: DispatchQueue = .main) {
Self.checkUniqueInstance()

self.navigatorType = navigatorType
Expand All @@ -700,6 +715,8 @@ open class RouteController: NSObject {
let options = indexedRouteResponse.validatedRouteOptions
self.routeProgress = RouteProgress(route: indexedRouteResponse.currentRoute!, options: options)
self.navigationSessionManager = navigationSessionManager
self.parsingQueue = parsingQueue
self.delegateQueue = delegateQueue

super.init()

Expand Down Expand Up @@ -1095,22 +1112,28 @@ extension RouteController {

switch result {
case .success(let routeAlternatives):
let alternativeRoutes = routeAlternatives.compactMap {
AlternativeRoute(mainRoute: self.route,
alternativeRoute: $0)
}

removedRoutes.append(contentsOf: alternatives.filter { alternative in
!routeAlternatives.contains(where: {
alternative.id == $0.id
self.parsingQueue.async { [weak self] in
guard let self else { return }

let alternativeRoutes = routeAlternatives.compactMap {
AlternativeRoute(alternativeRoute: $0)
}

removedRoutes.append(contentsOf: alternatives.filter { alternative in
!routeAlternatives.contains(where: {
alternative.id == $0.id
})
}.compactMap {
AlternativeRoute(alternativeRoute: $0)
})
}.compactMap {
AlternativeRoute(mainRoute: self.route,
alternativeRoute: $0)
})
self.continuousAlternatives = alternativeRoutes
self.report(newAlternativeRoutes: alternativeRoutes,
removedAlternativeRoutes: removedRoutes)

self.delegateQueue.async { [weak self] in
guard let self else { return }
self.continuousAlternatives = alternativeRoutes
self.report(newAlternativeRoutes: alternativeRoutes,
removedAlternativeRoutes: removedRoutes)
}
}
case .failure(let updateError):
let error = AlternativeRouteError.failedToUpdateAlternativeRoutes(reason: updateError.localizedDescription)
var userInfo = [RouteController.NotificationUserInfoKey: Any]()
Expand All @@ -1127,11 +1150,11 @@ extension RouteController {
guard NavigationSettings.shared.alternativeRouteDetectionStrategy != nil else {
return
}

var userInfo = [RouteController.NotificationUserInfoKey: Any]()
userInfo[.updatedAlternativesKey] = newAlternativeRoutes
userInfo[.removedAlternativesKey] = removedAlternativeRoutes

NotificationCenter.default.post(name: .routeControllerDidUpdateAlternatives,
object: self,
userInfo: userInfo)
Expand Down
Loading