Add support for updating and deleting MapData features

[rwrx]

Add support for updating and deleting MapData features. I have only created updating and deleting polyline features as I am not sure if you would want to add this functionality. If you would want it, I can make it more general and clean it up, because now it is only a very ugly hack.

[westnordost]

That is exactly what I need. Currently I re-set the whole MapData whenever one geometry is added or removed. I can imagine this is not very performant but there doesn't seem to be another option to do this right now.

[matteblair]

This does save some processing compared to re-setting all of the features in the layer, but since any feature change needs to propagate down to GPU buffers, we still need to rebuild those buffers and upload them to the GPU again.

It's not obvious that this feature would make enough difference in performance to justify the added complexity. Have you done any profiling to see where the bottlenecks are?

[westnordost]

Would love to see a comparison in terms of speed.

What this saves in any case is the necessity to keep the data in Java as well (to reach the same functionality). So, apart from any considerations of speed, it'd be a more convenient interface.

[rwrx]

@matteblair Unfortunately I have no experience with profiling C++ native code on Android. Do you have some hints how can I profile it?

[westnordost]

Couldn't you just count the milliseconds it takes for

1. A: set 10000 map features on a map data layer
2. B: add 1 feature to that map data layer

to complete and print it to console?

B then is the time it it takes for rebuilding the buffers and upload them to the GPU.
A-B is the time it takes to copy the data from Java, the time saved by this implementation.

[matteblair]

There's also work that happens asynchronously when map data is changed, that would have to be instrumented differently. But the measurement you described could be informative.

[rwrx]

Ok, I have profiled methods MapData.addFeature(Geometry feature) and my added method MapData.updatePolyline(long id, Map<String, String> properties) by adding and 586 polylines and on average MapData.updatePolyline(long id, Map<String, String> properties) was faster about 15%.

Both profiled methods also calls a C++ method ClientDataSource::generateTiles().

[westnordost]

Can you also say the absolute numbers?

Note on ClientDataSource::generateTiles(): Could this be optimized? After all, if you add just one feature, not all tiles need to be (re-)generated, but only those that contain the one feature updated.

[matteblair]

I'm interested in absolute numbers too - if makes a difference if the change is a few microseconds vs a few milliseconds.

The main operations in generateTiles() happen inside geojson-vt-cpp, a 3rd-party library. It's conceivable that we could modify its behavior to optimize our use cases, but I'd rather not diverge from the source if possible.

[rwrx]

Ok, profiling scenario was this: add polyline by MapData.addFeature then update polyline with MapData.updatePolyline and this repeated for 586 polylines.

For first couple of polylines it was under 10 milliseconds for both both methods. And then it started to slow down. At the end it was around 150 miliseconds for MapData.updatePolyline and around 170 miliseconds for MapData.addFeature.

I suggest that you can also profile it, so we will have better understanding of actual performance difference.

Overall in average there was 15% difference.

[westnordost]

So, the big majority of the time is not spent in copying the data from Java to native but in generating the tiles (or uploading to buffer). So from a sheer performance point of view, this (alone) does not make much of a difference, but what it does it to have a more convenient interface. And, it opens up the path for more performance improvement (on geojson-vt-cpp).

[tallytalwar]

When we first introduced ClientDataSources, we had support for updating MapData features, but performance bottleneck on geojson-vt library was present then also. I vaguely remember having similar discussions during good old eraser-map (ClientDataSource was implemented to fulfill the needs for Erasermap app) development days at mapzen, which is when we decided to remove this feature.