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

Batch up scheduling of deferred deletions #3030

Merged
merged 10 commits into from
Dec 13, 2024

Conversation

TimSylvester
Copy link
Collaborator

@TimSylvester TimSylvester commented Nov 19, 2024

In each tile cache (one per tile source), released tiles are kept in a vector and scheduled for release all at once, at the end of update, which is called once per frame.

The capture below is from ~150 seconds of the Android benchmark, 5300 frames. Of 384 total, it shows 138 instances of just one tile being scheduled, up to 3 instances of 13 being released at once. 440 total scheduled tasks were averted in this case, roughly half.

Manually zooming in and out on the same area for ~600 frames produces similar results.

If we only did the release after accumulating several tiles, we could reduce the number of tasks significantly more, but we might end up holding onto unused tiles for many frames in some cases.

Additionally, this avoids allocating a shared_ptr for each unique_ptr to make it copyable along with the lambda capture, using a more generic move-only function wrapper.

Copy link

github-actions bot commented Nov 19, 2024

Bloaty Results (iOS) 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.2% +24.5Ki  +0.1% +16.0Ki    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results-ios/pr-3030-compared-to-main.txt

Copy link

github-actions bot commented Nov 19, 2024

Bloaty Results 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.1% +79.0Ki  +0.0% +13.0Ki    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-3030-compared-to-main.txt

Compared to d387090 (legacy)

    FILE SIZE        VM SIZE    
 --------------  -------------- 
   +30% +34.6Mi  +433% +25.8Mi    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-3030-compared-to-legacy.txt

Copy link

github-actions bot commented Nov 19, 2024

Benchmark Results ⚡

Benchmark                                                     Time             CPU      Time Old      Time New       CPU Old       CPU New
------------------------------------------------------------------------------------------------------------------------------------------
OVERALL_GEOMEAN                                            +0.0013         +0.0010             0             0             0             0

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/benchmark-results/pr-3030-compared-to-main.txt

@TimSylvester TimSylvester requested a review from louwers December 4, 2024 19:30
@louwers
Copy link
Collaborator

louwers commented Dec 4, 2024

For future reference: once C++23 is out we can use std::move_only_function

@TimSylvester TimSylvester merged commit 12f596c into maplibre:main Dec 13, 2024
50 checks passed
@TimSylvester TimSylvester deleted the batch-defer branch December 13, 2024 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants