diff --git a/platform/default/mbgl/storage/offline_download.cpp b/platform/default/mbgl/storage/offline_download.cpp index bf75989d256..293166b807a 100644 --- a/platform/default/mbgl/storage/offline_download.cpp +++ b/platform/default/mbgl/storage/offline_download.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -49,44 +50,6 @@ void OfflineDownload::setState(OfflineRegionDownloadState state) { observer->statusChanged(status); } -std::vector OfflineDownload::spriteResources(const style::Parser& parser) const { - std::vector result; - - if (!parser.spriteURL.empty()) { - result.push_back(Resource::spriteImage(parser.spriteURL, definition.pixelRatio)); - result.push_back(Resource::spriteJSON(parser.spriteURL, definition.pixelRatio)); - } - - return result; -} - -std::vector OfflineDownload::glyphResources(const style::Parser& parser) const { - std::vector result; - - if (!parser.glyphURL.empty()) { - for (const auto& fontStack : parser.fontStacks()) { - for (uint32_t i = 0; i < 256; i++) { - result.push_back( - Resource::glyphs(parser.glyphURL, fontStack, getGlyphRange(i * 256))); - } - } - } - - return result; -} - -std::vector -OfflineDownload::tileResources(SourceType type, uint16_t tileSize, const Tileset& tileset) const { - std::vector result; - - for (const auto& tile : definition.tileCover(type, tileSize, tileset.zoomRange)) { - result.push_back( - Resource::tile(tileset.tiles[0], definition.pixelRatio, tile.x, tile.y, tile.z, tileset.scheme)); - } - - return result; -} - OfflineRegionStatus OfflineDownload::getStatus() const { if (status.downloadState == OfflineRegionDownloadState::Active) { return status; @@ -106,29 +69,27 @@ OfflineRegionStatus OfflineDownload::getStatus() const { result.requiredResourceCountIsPrecise = true; for (const auto& source : parser.sources) { - switch (source->baseImpl->type) { + SourceType type = source->baseImpl->type; + + switch (type) { case SourceType::Vector: case SourceType::Raster: { style::TileSourceImpl* tileSource = static_cast(source->baseImpl.get()); const variant& urlOrTileset = tileSource->getURLOrTileset(); + const uint16_t tileSize = tileSource->getTileSize(); if (urlOrTileset.is()) { result.requiredResourceCount += - tileResources(source->baseImpl->type, tileSource->getTileSize(), - urlOrTileset.get()) - .size(); + definition.tileCover(type, tileSize, urlOrTileset.get().zoomRange).size(); } else { result.requiredResourceCount += 1; const std::string& url = urlOrTileset.get(); optional sourceResponse = offlineDatabase.get(Resource::source(url)); if (sourceResponse) { result.requiredResourceCount += - tileResources(source->baseImpl->type, tileSource->getTileSize(), - style::TileSourceImpl::parseTileJSON( - *sourceResponse->data, url, source->baseImpl->type, - tileSource->getTileSize())) - .size(); + definition.tileCover(type, tileSize, style::TileSourceImpl::parseTileJSON( + *sourceResponse->data, url, type, tileSize).zoomRange).size(); } else { result.requiredResourceCountIsPrecise = false; } @@ -140,7 +101,7 @@ OfflineRegionStatus OfflineDownload::getStatus() const { style::GeoJSONSource::Impl* geojsonSource = static_cast(source->baseImpl.get()); - if (!geojsonSource->loaded) { + if (geojsonSource->getURL()) { result.requiredResourceCount += 1; } break; @@ -152,8 +113,13 @@ OfflineRegionStatus OfflineDownload::getStatus() const { } } - result.requiredResourceCount += spriteResources(parser).size(); - result.requiredResourceCount += glyphResources(parser).size(); + if (!parser.glyphURL.empty()) { + result.requiredResourceCount += parser.fontStacks().size() * GLYPH_RANGES_PER_FONT_STACK; + } + + if (!parser.spriteURL.empty()) { + result.requiredResourceCount += 2; + } return result; } @@ -161,9 +127,7 @@ OfflineRegionStatus OfflineDownload::getStatus() const { void OfflineDownload::activateDownload() { status = OfflineRegionStatus(); status.downloadState = OfflineRegionDownloadState::Active; - - requiredSourceURLs.clear(); - + status.requiredResourceCount++; ensureResource(Resource::style(definition.styleURL), [&](Response styleResponse) { status.requiredResourceCountIsPrecise = true; @@ -182,15 +146,16 @@ void OfflineDownload::activateDownload() { const uint16_t tileSize = tileSource->getTileSize(); if (urlOrTileset.is()) { - ensureTiles(type, tileSize, urlOrTileset.get()); + queueTiles(type, tileSize, urlOrTileset.get()); } else { const std::string& url = urlOrTileset.get(); status.requiredResourceCountIsPrecise = false; + status.requiredResourceCount++; requiredSourceURLs.insert(url); ensureResource(Resource::source(url), [=](Response sourceResponse) { - ensureTiles(type, tileSize, style::TileSourceImpl::parseTileJSON( - *sourceResponse.data, url, type, tileSize)); + queueTiles(type, tileSize, style::TileSourceImpl::parseTileJSON( + *sourceResponse.data, url, type, tileSize)); requiredSourceURLs.erase(url); if (requiredSourceURLs.empty()) { @@ -206,7 +171,7 @@ void OfflineDownload::activateDownload() { static_cast(source->baseImpl.get()); if (geojsonSource->getURL()) { - ensureResource(Resource::source(*geojsonSource->getURL())); + queueResource(Resource::source(*geojsonSource->getURL())); } break; } @@ -217,30 +182,73 @@ void OfflineDownload::activateDownload() { } } - for (const auto& resource : spriteResources(parser)) { - ensureResource(resource); + if (!parser.glyphURL.empty()) { + for (const auto& fontStack : parser.fontStacks()) { + for (uint32_t i = 0; i < GLYPH_RANGES_PER_FONT_STACK; i++) { + queueResource(Resource::glyphs(parser.glyphURL, fontStack, getGlyphRange(i * GLYPHS_PER_GLYPH_RANGE))); + } + } } - for (const auto& resource : glyphResources(parser)) { - ensureResource(resource); + if (!parser.spriteURL.empty()) { + queueResource(Resource::spriteImage(parser.spriteURL, definition.pixelRatio)); + queueResource(Resource::spriteJSON(parser.spriteURL, definition.pixelRatio)); } + + continueDownload(); }); } +/* + Fill up our own request queue by requesting the next few resources. This is called + when activating the download, or when a request completes successfully. + + Note "successfully"; it's not called when a requests receives an error. A request + that errors will be retried after some delay. So in that sense it's still "active" + and consuming resources, notably the request object, its timer, and network resources + when the timer fires. + + We could try to squeeze in subsequent requests while we wait for the errored request + to retry. But that risks overloading the upstream request queue -- defeating our own + metering -- if there are a lot of errored requests that all come up for retry at the + same time. And many times, the cause of a request error will apply to many requests + of the same type. For instance if a server is unreachable, all the requests to that + host are going to error. In that case, continuing to try subsequent resources after + the first few errors is fruitless anyway. +*/ +void OfflineDownload::continueDownload() { + if (resourcesRemaining.empty() && status.complete()) { + setState(OfflineRegionDownloadState::Inactive); + return; + } + + while (!resourcesRemaining.empty() && requests.size() < HTTPFileSource::maximumConcurrentRequests()) { + ensureResource(resourcesRemaining.front()); + resourcesRemaining.pop_front(); + } +} + void OfflineDownload::deactivateDownload() { + requiredSourceURLs.clear(); + resourcesRemaining.clear(); requests.clear(); } -void OfflineDownload::ensureTiles(SourceType type, uint16_t tileSize, const Tileset& info) { - for (const auto& resource : tileResources(type, tileSize, info)) { - ensureResource(resource); +void OfflineDownload::queueResource(Resource resource) { + status.requiredResourceCount++; + resourcesRemaining.push_front(std::move(resource)); +} + +void OfflineDownload::queueTiles(SourceType type, uint16_t tileSize, const Tileset& tileset) { + for (const auto& tile : definition.tileCover(type, tileSize, tileset.zoomRange)) { + status.requiredResourceCount++; + resourcesRemaining.push_back( + Resource::tile(tileset.tiles[0], definition.pixelRatio, tile.x, tile.y, tile.z, tileset.scheme)); } } void OfflineDownload::ensureResource(const Resource& resource, std::function callback) { - status.requiredResourceCount++; - auto workRequestsIt = requests.insert(requests.begin(), nullptr); *workRequestsIt = util::RunLoop::Get()->invokeCancellable([=]() { requests.erase(workRequestsIt); @@ -260,11 +268,7 @@ void OfflineDownload::ensureResource(const Resource& resource, } observer->statusChanged(status); - - if (status.complete()) { - setState(OfflineRegionDownloadState::Inactive); - } - + continueDownload(); return; } @@ -299,9 +303,7 @@ void OfflineDownload::ensureResource(const Resource& resource, return; } - if (status.complete()) { - setState(OfflineRegionDownloadState::Inactive); - } + continueDownload(); }); }); } diff --git a/platform/default/mbgl/storage/offline_download.hpp b/platform/default/mbgl/storage/offline_download.hpp index 77389d63db6..f29a053a875 100644 --- a/platform/default/mbgl/storage/offline_download.hpp +++ b/platform/default/mbgl/storage/offline_download.hpp @@ -1,17 +1,18 @@ #pragma once #include +#include #include #include #include +#include namespace mbgl { class OfflineDatabase; class FileSource; class AsyncRequest; -class Resource; class Response; class Tileset; @@ -36,19 +37,15 @@ class OfflineDownload { private: void activateDownload(); + void continueDownload(); void deactivateDownload(); - std::vector spriteResources(const style::Parser&) const; - std::vector glyphResources(const style::Parser&) const; - std::vector tileResources(SourceType, uint16_t, const Tileset&) const; - /* * Ensure that the resource is stored in the database, requesting it if necessary. * While the request is in progress, it is recorded in `requests`. If the download * is deactivated, all in progress requests are cancelled. */ void ensureResource(const Resource&, std::function = {}); - void ensureTiles(SourceType, uint16_t, const Tileset&); bool checkTileCountLimit(const Resource& resource); int64_t id; @@ -57,8 +54,13 @@ class OfflineDownload { FileSource& onlineFileSource; OfflineRegionStatus status; std::unique_ptr observer; + std::list> requests; std::unordered_set requiredSourceURLs; + std::deque resourcesRemaining; + + void queueResource(Resource); + void queueTiles(SourceType, uint16_t tileSize, const Tileset&); }; } // namespace mbgl diff --git a/platform/default/online_file_source.cpp b/platform/default/online_file_source.cpp index 00c66f004da..4f191f6cf72 100644 --- a/platform/default/online_file_source.cpp +++ b/platform/default/online_file_source.cpp @@ -193,7 +193,7 @@ std::unique_ptr OnlineFileSource::request(const Resource& resource break; } - return std::make_unique(res, callback, *impl); + return std::make_unique(std::move(res), std::move(callback), *impl); } OnlineFileRequest::OnlineFileRequest(Resource resource_, Callback callback_, OnlineFileSource::Impl& impl_) diff --git a/src/mbgl/text/glyph_range.hpp b/src/mbgl/text/glyph_range.hpp index aa11b35f5ea..dd39e092b74 100644 --- a/src/mbgl/text/glyph_range.hpp +++ b/src/mbgl/text/glyph_range.hpp @@ -17,4 +17,7 @@ struct GlyphRangeHash { typedef std::unordered_set GlyphRangeSet; +constexpr uint32_t GLYPHS_PER_GLYPH_RANGE = 256; +constexpr uint32_t GLYPH_RANGES_PER_FONT_STACK = 256; + } // end namespace mbgl diff --git a/test/src/mbgl/test/fake_file_source.hpp b/test/src/mbgl/test/fake_file_source.hpp index 7ebb4cf0cb4..3ed3f90a172 100644 --- a/test/src/mbgl/test/fake_file_source.hpp +++ b/test/src/mbgl/test/fake_file_source.hpp @@ -2,6 +2,7 @@ #include +#include #include namespace mbgl { diff --git a/test/storage/offline_download.cpp b/test/storage/offline_download.cpp index 76a772dd1b2..27e57771c84 100644 --- a/test/storage/offline_download.cpp +++ b/test/storage/offline_download.cpp @@ -1,8 +1,10 @@ #include +#include #include #include #include +#include #include #include #include @@ -240,6 +242,29 @@ TEST(OfflineDownload, Activate) { test.loop.run(); } +TEST(OfflineDownload, DoesNotFloodTheFileSourceWithRequests) { + FakeFileSource fileSource; + OfflineTest test; + OfflineRegion region = test.createRegion(); + OfflineDownload download( + region.getID(), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + test.db, fileSource); + + auto observer = std::make_unique(); + + download.setObserver(std::move(observer)); + download.setState(OfflineRegionDownloadState::Active); + test.loop.runOnce(); + + EXPECT_EQ(1u, fileSource.requests.size()); + + fileSource.respond(Resource::Kind::Style, test.response("style.json")); + test.loop.runOnce(); + + EXPECT_EQ(HTTPFileSource::maximumConcurrentRequests(), fileSource.requests.size()); +} + TEST(OfflineDownload, GetStatusNoResources) { OfflineTest test; OfflineRegion region = test.createRegion();