Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Mark used offline region resources in batches #15521

Merged
merged 5 commits into from
Aug 29, 2019

Conversation

pozdnyakov
Copy link
Contributor

@pozdnyakov pozdnyakov commented Aug 29, 2019

Marking offline region resources "used" is on the hot code path for offline region download.
Previously, the engine attempted to update the database separetely for each resource and it caused a huge performance impact, especially when some tiles were already loaded.

Now, offline region resources are marked in batches (200 items at once).

Consider the time mesaurements of the mbgl-offline tool execution (Linux x64, release build):

time ./mbgl-offline --north 41.4664 --west 2.0407 --south 41.2724 --east 2.2680 --output barcelona.db

without patch:

1st time:

    real      0m9,505s
    user      0m3,315s
    sys       0m0,378s

2nd time (tiles are already loaded):

   real      0m9,480s
   user      0m0,403s
   sys       0m0,645s

with the patch:

1st time:

    real      0m7,399s
    user      0m2,994s
    sys       0m0,384s

2nd time (tiles are already loaded):

    real      0m0,149s
    user      0m0,059s
    sys       0m0,044s

So, the proposed changes make the repeated load ~64 times faster 馃帀

@pozdnyakov pozdnyakov added bug performance Speed, stability, CPU usage, memory usage, or power usage Core The cross-platform C++ core, aka mbgl labels Aug 29, 2019
@pozdnyakov pozdnyakov self-assigned this Aug 29, 2019
Copy link
Contributor

@alexshalamov alexshalamov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@alexshalamov
Copy link
Contributor

@LukasPaczos would you be able to test this PR for Android SDK?

@LukasPaczos
Copy link
Member

The improvements are amazing!

A small offline region that takes about 8s to download:

Previously Now
First re-download in the current process 8s 1.3s
Subsequent re-downloads in the same process 4.5s 0.7s

What's interesting, first re-download in the app process (for example after restarting the application when tiles are already in place) takes significantly longer than subsequent ones.

@andrewharvey
Copy link
Contributor

andrewharvey commented Aug 29, 2019

Wow I didn't realise you could call mbgl-offline twice on the same output and it would just update, I always thought it would just replace it and re-download everything. Could we update this in the documentation? PS. I can't find any of the offline sideload documentation anymore, I guess it must have been removed on purpose.

@LukasPaczos
Copy link
Member

A region with 5001 resources that originally takes ~47s to download:

Testing different batch sizes https://github.com/mapbox/mapbox-gl-native/pull/15521/files#diff-72890ff2093a9bfa6bfe3a9e8955d6dfR361:

For batches of 200 resources (as in the PR)

Previously Now
First re-download in the current process 41s 2.7s
Subsequent re-downloads in the same process 38s 1.3s

For batches of 1000 resources

Previously Now
First re-download in the current process 41s 1.6s
Subsequent re-downloads in the same process 38s 1.0s

@pozdnyakov pozdnyakov requested a review from a team August 29, 2019 13:46
@pozdnyakov
Copy link
Contributor Author

We could make batch size adaptive, i.e. depend on the total resources count, I'll investigate it and implement in a follow-up PR.

@pozdnyakov pozdnyakov merged commit e9d0fd1 into master Aug 29, 2019
@pozdnyakov pozdnyakov deleted the mikhail_offline_download branch August 29, 2019 14:28
@chloekraw
Copy link
Contributor

Wow, amazing work @pozdnyakov!

cc @zugaldia @tmpsantos

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Core The cross-platform C++ core, aka mbgl performance Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants