CartoDB / mobile-sdk

CARTO Mobile SDK core project
https://carto.com/docs/carto-engine/mobile-sdk/
BSD 3-Clause "New" or "Revised" License
178 stars 65 forks source link

Andoid valhalla matchRoute memory leak #523

Open farfromrefug opened 1 year ago

farfromrefug commented 1 year ago

if i do something like this:

class MathRouteTask extends TimerTask
    {
        MathRouteTask(ValhallaOfflineRoutingService routingService, Projection projection, String profile) {
            super();
            this.profile = profile;
            this.projection = projection;
            this.routingService = routingService;
        }
        ValhallaOfflineRoutingService routingService;
        Projection projection;
        String profile;
        public void run()
        {
            matchRouteTest(this.routingService, this.projection, this.profile);
        }
    }
    public void matchRouteTest(final ValhallaOfflineRoutingService routingService, Projection projection, String profile) {
        Thread thread = new Thread(new Runnable() {
            @Override
            public void run() {
                try {
                    routingService.setProfile(profile);
                    MapPosVector vector = new MapPosVector();
                    vector.add(new MapPos(5.721619, 45.193549999999995));
                    vector.add(new MapPos(5.721585, 45.193549));
                    RouteMatchingRequest matchrequest = new RouteMatchingRequest(projection, vector, 1);
                    matchrequest.setCustomParameter("shape_match", new Variant("edge_walk"));
                    matchrequest.setCustomParameter("filters", Variant.fromString("{ \"attributes\": [\"edge.surface\", \"edge.road_class\", \"edge.sac_scale\", \"edge.use\"], \"action\": \"include\" }"));
                    RouteMatchingResult matchresult = routingService.matchRoute(matchrequest);
                    largeLog(TAG,"matchresult "+ matchresult.getRawResult());
                } catch (IOException e) {
                    e.printStackTrace();
                }
            }
        });
        thread.start();
    }
public void testMatchRoute(Options options) {
        Projection projection = options.getBaseProjection();
        ValhallaOfflineRoutingService routingService;
        try {
            routingService = new ValhallaOfflineRoutingService("/storage/1C05-0202/alpimaps_mbtiles/france.vtiles");
            LocalVectorDataSource localSource = new LocalVectorDataSource(projection);
            Timer timer = new Timer();
            TimerTask task = new MathRouteTask(routingService, options.getBaseProjection(), "pedestrian");
            timer.schedule(task, 0, 500);
        } catch (IOException e) {
            e.printStackTrace();
        }
    }
testMatchRoute(options);

Which is calling a matchRoute every 500ms, i can see the memory rising really fast then the app crashes. I tried to profile with Android Studio and i did not see any leak on the java side. However the native side clearly reported leaks but the report does not help much. It talks about "ERROR 1" and i cant find what it is.

Screenshot 2022-11-16 at 09 22 26

I would say the leak in on valhalla side. Which leads me to the work i started to upgrade valhalla to 3.2. As i merged the source i realised that some "cache" code was disabled and not disposed. Not sure if this could be related. I am not done with 3.2 as it build but valhalla complains about not finding route so something failed in the merge ;) (mostly carto hack changes as most of them were removed by the merge)

mtehver commented 1 year ago

Interesting, can you reproduce it on iOS also? The issue with Android is that due to the way SDK objects are wrapped, Java garbage collector may see the routing objects as very lightweight, though they are connected to really large native objects.

Regarding Valhalla 3.2, I think most the 'CARTO hacks' can be removed, as I introduced them to fix crashes coming from missing tiles in pre-3.0 Valhalla. But I think most of those (if not all) had been already fixed in 3.1.

farfromrefug commented 1 year ago

@mtehver you are talking about the string representation of the routing result? I don't think this is it in the sense that the profiler does not report a memory leak on string. It talks about that "Error" thing which I am not sure what it is. I ll dig deeper and try to see if can reproduce on iOS