Merge pull request #59 from ChristophKaser/keep_actor_instance

Only initialize Valhalla actor once and use it for multiple route requests
Rallista · Feb 1, 2025 · 18a7cc4 · 18a7cc4
2 parents ec6de7a + 69f5c03
commit 18a7cc4
Show file tree

Hide file tree

Showing 8 changed files with 93 additions and 45 deletions.
diff --git a/apple/Sources/Valhalla/Valhalla.swift b/apple/Sources/Valhalla/Valhalla.swift
@@ -6,21 +6,21 @@ public protocol ValhallaProviding {
 
     init(_ config: ValhallaConfig) throws
 
-    init(configPath: String)
+    init(configPath: String) throws
 
     func route(request: RouteRequest) throws -> RouteResponse
 }
 
 public final class Valhalla: ValhallaProviding {
-    private let actor = ValhallaWrapper()
+    private let actor: ValhallaWrapper?
     private let configPath: String
 
     public convenience init(_ config: ValhallaConfig) throws {
         let configURL = try ValhallaFileManager.saveConfigTo(config)
-        self.init(configPath: configURL.relativePath)
+        try self.init(configPath: configURL.relativePath)
     }
 
-    public required init(configPath: String) {
+    public required init(configPath: String) throws {
         do {
             try ValhallaFileManager.injectTzdataIntoLibrary()
         } catch {
@@ -29,6 +29,13 @@ public final class Valhalla: ValhallaProviding {
         }
 
         self.configPath = configPath
+        do {
+            self.actor = try ValhallaWrapper(configPath: configPath)
+        } catch let error as NSError {
+            throw ValhallaError.valhallaError(error.code, error.domain)
+        } catch {
+            throw ValhallaError.valhallaError(-1, error.localizedDescription)
+        }
     }
 
     public func route(request: RouteRequest) throws -> RouteResponse {
@@ -50,6 +57,6 @@ public final class Valhalla: ValhallaProviding {
     }
 
     public func route(rawRequest request: String) -> String {
-        actor!.route(request, configPath: configPath)
+        actor!.route(request)
     }
 }
diff --git a/apple/Sources/ValhallaObjc/ValhallaWrapper.mm b/apple/Sources/ValhallaObjc/ValhallaWrapper.mm
@@ -4,25 +4,46 @@
 
 @implementation ValhallaWrapper
 
-- (instancetype)init
+- (instancetype)initWithConfigPath:(NSString*)config_path error:(__autoreleasing NSError **)error
 {
     self = [super init];
+    std::string path = std::string([config_path UTF8String]);
+    try {
+        _actor = create_valhalla_actor(path.c_str());
+    } catch (NSException *exception) {
+        *error = [[NSError alloc] initWithDomain:exception.name code:0 userInfo:@{
+            NSUnderlyingErrorKey: exception,
+            NSLocalizedDescriptionKey: exception.reason,
+            @"CallStackSymbols": exception.callStackSymbols
+        }];
+        return nil;
+    } catch (const std::exception &err) {
+        *error = [[NSError alloc] initWithDomain: [NSString stringWithUTF8String:err.what()] code:-1 userInfo: nil];
+        return nil;
+    } catch (...) {
+        *error = [[NSError alloc] initWithDomain: @"unknown exception" code:-1 userInfo: nil];
+        return nil;
+    }
     return self;
 }
 
-- (NSString*)route:(NSString*)request configPath:(NSString*)config_path
+- (NSString*)route:(NSString*)request
 {
-    // Get the config path
-    std::string path = std::string([config_path UTF8String]);
-    std::string config_file(path);
-
-    // Convert the NSString to std::string
-    std::string req = std::string([request UTF8String]);
-
-    // Generate the valhalla response
-    std::string res = route(req.c_str(), path.c_str());
-
-    return [NSString stringWithUTF8String:res.c_str()];
+    @synchronized(self) {
+        // Convert the NSString to std::string
+        std::string req = std::string([request UTF8String]);
+
+        // Generate the valhalla response
+        std::string res = route(req.c_str(), _actor);
+
+        return [NSString stringWithUTF8String:res.c_str()];
+    }
+}
+
+- (void) dealloc
+{
+    delete_valhalla_actor(_actor);
+    _actor = nil;
 }
 
 @end
diff --git a/apple/Sources/ValhallaObjc/include/ValhallaWrapper.h b/apple/Sources/ValhallaObjc/include/ValhallaWrapper.h
@@ -5,11 +5,14 @@
 
 @class ValhallaWrapper;
 
-@interface ValhallaWrapper : NSObject {}
+@interface ValhallaWrapper : NSObject {
+    @private
+    void* _actor;
+}
 
-- (instancetype)init;
+- (instancetype)initWithConfigPath:(NSString*)config_path error:(__autoreleasing NSError **)error;
 
-- (NSString*)route:(NSString*)request configPath:(NSString*)config_path;
+- (NSString*)route:(NSString*)request;
 
 @end
 

diff --git a/apple/Tests/ValhallaTests/TestValhallaWithTar.swift b/apple/Tests/ValhallaTests/TestValhallaWithTar.swift
@@ -13,18 +13,18 @@ final class TestValhallaWithTar: XCTestCase {
 
     /// Validate an incorrect configuration (config file not found).
     func testNoConfigFile() throws {
-        let valhalla = Valhalla(configPath: "missing.json")
+        do {
+            let valhalla = try Valhalla(configPath: "missing.json")
 
-        let request = RouteRequest(
-            locations: [
-                RoutingWaypoint(lat: 38.429719, lon: -108.827425),
-                RoutingWaypoint(lat: 38.4604331, lon: -108.8817009)
-            ],
-            costing: .auto,
-            directionsOptions: DirectionsOptions(units: .mi)
-        )
+            let request = RouteRequest(
+                locations: [
+                    RoutingWaypoint(lat: 38.429719, lon: -108.827425),
+                    RoutingWaypoint(lat: 38.4604331, lon: -108.8817009)
+                ],
+                costing: .auto,
+                directionsOptions: DirectionsOptions(units: .mi)
+            )
 
-        do {
             let _ = try valhalla.route(request: request)
             XCTFail("route should throw cannot open file missing.json")
         } catch let error as ValhallaError {

diff --git a/src/wrapper/include/main.h b/src/wrapper/include/main.h
@@ -22,8 +22,10 @@ JNIEXPORT jstring JNICALL Java_com_valhalla_valhalla_ValhallaKotlin_route(JNIEnv
 
 #elif __APPLE__
 
-std::string route(const char *request, const char *config_path);
+std::string route(const char *request, void* actor);
+void* create_valhalla_actor(const char *config_path);
+void delete_valhalla_actor(void* actor);
 
 #endif
 
-#endif // WRAPPER_H
+#endif // WRAPPER_H
diff --git a/src/wrapper/include/valhalla_actor.h b/src/wrapper/include/valhalla_actor.h
@@ -5,8 +5,12 @@
 #include <valhalla/tyr/actor.h>
 
 class ValhallaActor {
+private:
+    valhalla::tyr::actor_t actor;
 public:
-    std::string route(const std::string& request, const std::string& config_path);
+    ValhallaActor(const std::string& config_path);
+
+    std::string route(const std::string& request);
 };
 
-#endif // VALHALLAACTOR_H
+#endif // VALHALLAACTOR_H
diff --git a/src/wrapper/main.cpp b/src/wrapper/main.cpp
@@ -21,8 +21,9 @@ Java_com_valhalla_valhalla_ValhallaKotlin_route(JNIEnv *env,
 
     std::string result;
     try {
-        ValhallaActor valhallaActor;
-        result = valhallaActor.route(request, config_path);
+        // TODO: Android currently creates a new actor every time. Optimize to be like iOS later.
+        ValhallaActor valhallaActor(config_path);
+        result = valhallaActor.route(request);
     } catch (const valhalla::valhalla_exception_t &err) {
         printf("[ValhallaActor] route valhalla_exception: %s\n", err.what());
         std::string code = std::to_string(err.code);
@@ -44,11 +45,18 @@ Java_com_valhalla_valhalla_ValhallaKotlin_route(JNIEnv *env,
 }
 
 #elif __APPLE__
-std::string route(const char *request, const char *config_path) {
+void* create_valhalla_actor(const char *config_path) {
+    return new ValhallaActor(config_path);
+}
+
+void delete_valhalla_actor(void* actor) {
+    delete ((ValhallaActor*) actor);
+}
+
+std::string route(const char *request, void* actor) {
     std::string result;
     try {
-        ValhallaActor valhallaActor;
-        result = valhallaActor.route(request, config_path);
+        result = ((ValhallaActor*) actor)->route(request);
     } catch (const valhalla::valhalla_exception_t &err) {
         printf("[ValhallaActor] route valhalla_exception: %s\n", err.what());
         std::string code = std::to_string(err.code);
@@ -65,4 +73,4 @@ std::string route(const char *request, const char *config_path) {
 
     return result;
 }
-#endif
+#endif
diff --git a/src/wrapper/valhalla_actor.cpp b/src/wrapper/valhalla_actor.cpp
@@ -4,17 +4,20 @@
 #include <valhalla/loki/worker.h>
 #include "valhalla_actor.h"
 
-std::string ValhallaActor::route(const std::string& request, const std::string& config_path) {
+ValhallaActor::ValhallaActor(const std::string& config_path): actor([&config_path]() {
     // Get the config path
     std::string config_file(config_path);
 
     // Set up the config object
-    boost::property_tree::ptree(config);
+    boost::property_tree::ptree config;
     rapidjson::read_json(config_file, config);
 
     // Setup the actor
-    valhalla::tyr::actor_t actor = valhalla::tyr::actor_t(config);
-
+    return valhalla::tyr::actor_t(config);
+}()) // IIFE to prepare actor
+{}
+
+std::string ValhallaActor::route(const std::string& request) {
     // Convert the request to a std::string
     std::string req = std::string(request);