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.

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

First map rendering takes a long time when you have a lot of offline data stored #1815

Open
geolives-contact opened this issue Oct 30, 2023 · 6 comments
Assignees
Labels
bug Something isn't working cpp-core

Comments

@geolives-contact
Copy link
Contributor

geolives-contact commented Oct 30, 2023

Describe the bug
I found a nasty issue on MapLibre native when you have a lot of data downloaded. The time for the map first rendering increases significantly. With 1,5GB of offline data, the first map rendering lasts 5 seconds instead of 1 second.

To Reproduce
Steps to reproduce the behavior with the test app:

  1. Change the default style for the test app with this style: https://carto.geolives.com/vector/geolivestopoosm_vector_grey.json
  2. Go to folder directory to download offline areas.
  3. Download an amount of 1,5GB+ of offline data.
  4. Exit the app and re-run it.
  5. The first map rendering will take more time to render completely.

The time increases as the local database also increases.
The referenced style includes multiple sources that emphasizes the issue, but you can use any style that includes sources with a lot of data.

Expected behavior
A first map rendering that takes the same time or near the same time as when no offline data are stored.

Screenshots
https://github.com/maplibre/maplibre-native/assets/13694294/a7c2bf01-54ff-43ed-86b1-675efb79e00a

Platform information:

  • OS: iOS 16. I suspect Android to have the same issue, but less noticable.
  • branch main, version 6.0.0.

Additional context
Historically, this issue was found in Mapbox in 2020 : mapbox/mapbox-gl-native#16519
I searched in the code and I agree with the initial issue : the initialization of the ambient cache size (OfflineDatabase::initAmbientCacheSize) takes a long time when the database gets a lot of stored tiles.

I modified the code by ignoring the request and returning directly 0 for the ambient cache size, and the first map rendering was instantly done.

@geolives-contact geolives-contact added the bug Something isn't working label Oct 30, 2023
@geolives-contact
Copy link
Contributor Author

2 possible alternatives to solve this issue:

  • Store the current ambient cache size inside the database, and update it each time his cache size is modified. This way, the initialization simply request the computed cache size from the last execution.
  • Store a flag inside the tiles and resources table that tells if the resource/tile is ambient or not. That way we simplify the request by deleting the need to make a left join. With the cost that for each download/cleaning, the flag must be re-set correctly.

@louwers
Copy link
Collaborator

louwers commented Oct 30, 2023

Thanks for this super detailed bug report.

Which of the two solutions you suggest would you prefer?

@geolives-contact
Copy link
Contributor Author

With more study on this issue, i could see a third option that is better than the two previous ones.

The main problem I have with the previous 2 solutions are in term of database corruption. If something wrong happens between the initial modification and the computation modification, you'll have a database with incoherent information stored. That could leads to issues difficult to overcome.

A third (and my preference) solution is to split the ambient cache from the offline database cache.
In terms of database structure, that means create two new tables. Let's say ambient_tiles and ambient_resources.

When we need to store resource for ambient cache:

  • We store it into the ambient_resources table.

When we need to store tile for ambient cache:

  • We store it into the ambient_tiles table.

When we need to store resource for offline data:

  • We store it into the resources table with the associated region inside the region_resources table.
  • We delete the same resource from the ambient cache (*).

When we need to store tile for offline data:

  • We store it into the tile table with the associated region inside the region_tiles table.
  • We delete the same tile from the ambient cache (*).

(*) Even if something wrong happens and the delete instruction cannot be done for a specified resource/tile, that only means the ambient cache will waste some size.
But with the continuous refresh of the ambient cache, this waste will be erased after some time.

By doing this, the initialization of the ambient cache will take no time, because we'll doing the request on a table that is limited to the Max Ambient Cache Size anyway.

About database structure migration, we can simply clear the ambient cache at first upgrade to the new database version.

Maybe a design proposal for this kind of modification could be useful?

@geolives-contact
Copy link
Contributor Author

geolives-contact commented Nov 6, 2023

Hello.

What do you think about the implementation of the third option (via the two new tables ambient_resources and ambient_tiles)?

We could start working on that change this week in order to implement it this month.

Thanks in advance.

@louwers
Copy link
Collaborator

louwers commented Nov 6, 2023

It sounds like a solid plan! 👍

If you can basically copy what you wrote in a design proposal template and make a PR, that would be helpful.

https://github.com/maplibre/maplibre-native/blob/main/design-proposals/2022-09-02-design-proposal-template.md

Thank you for looking into this and I'm looking forward to having this issue resolved.

@geolives-contact
Copy link
Contributor Author

Thank you, we have created the design proposal template and made a PR : #1839

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cpp-core
Projects
None yet
Development

No branches or pull requests

2 participants