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

[core] Fix offline regions reporting the wrong number of tiles #14958

Merged
merged 2 commits into from
Jun 19, 2019
Merged

[core] Fix offline regions reporting the wrong number of tiles #14958

merged 2 commits into from
Jun 19, 2019

Conversation

tmpsantos
Copy link
Contributor

@tmpsantos tmpsantos commented Jun 18, 2019 •
edited
Loading

  • The SQL query used for checking if the tile was already in the cache was wrong.
  • Tiles being refereed by two or more regions were not always counted as one.
  • Despite making one extra query, benchmarks are showing 8% performance improvement, because we are narrowing the query results by adding one extra filter.

Fixes #13437.

@tmpsantos tmpsantos self-assigned this Jun 18, 2019
@tmpsantos tmpsantos added the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Jun 18, 2019
@friedbunny friedbunny added this to the release-p milestone Jun 18, 2019
@friedbunny friedbunny added needs changelog Indicates PR needs a changelog entry prior to merging. Core The cross-platform C++ core, aka mbgl offline labels Jun 18, 2019
@tmpsantos
[core] Fix offline regions reporting the wrong number of tiles
5f6277c 
- The SQL query used for checking if the tile was already in the
  cache was wrong.

- Tiles being refereed by two or more regions were not always
  counted as one.

Fixes #13437.
@tmpsantos
[core] Add storage unit tests for MapboxTileLimitExceeded
a5f34fd
@tmpsantos tmpsantos force-pushed the tmpsantos-region_max_tile_count branch from 6857f57 to a5f34fd Compare June 18, 2019 21:56
@tmpsantos tmpsantos removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Jun 18, 2019
alexshalamov
alexshalamov approved these changes Jun 19, 2019
View reviewed changes
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 :godmode:

@tmpsantos tmpsantos merged commit 24df7fe into master Jun 19, 2019
@tmpsantos tmpsantos deleted the tmpsantos-region_max_tile_count branch June 19, 2019 14:19
@friedbunny friedbunny removed the needs changelog Indicates PR needs a changelog entry prior to merging. label Jun 26, 2019
@friedbunny friedbunny mentioned this pull request Jun 26, 2019
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alexshalamov alexshalamov alexshalamov approved these changes

@tobrun tobrun Awaiting requested review from tobrun

@julianrex julianrex Awaiting requested review from julianrex

Assignees

@tmpsantos tmpsantos

Labels
Core The cross-platform C++ core, aka mbgl offline
Projects
None yet
Milestone
release-picklejuice
Development

Successfully merging this pull request may close these issues.

Android - OfflineDownload throwing "too many tiles" even after deleting offline region
3 participants
@tmpsantos @alexshalamov @friedbunny