From 68e3cbacbc35ae6cd6c1e0fe2ab59ed262c32786 Mon Sep 17 00:00:00 2001 From: "Thiago Marcos P. Santos" Date: Fri, 14 Jun 2019 15:11:32 +0300 Subject: [core] Add method for setting a maximum size for the ambient cache - Removed this parameter from the constructor, now that we have a method to set it. - Add the method and enforce the limits if the ambient cache exceeds the size. --- include/mbgl/storage/default_file_source.hpp | 30 ++++++++++--- .../include/mbgl/storage/offline_database.hpp | 8 ++-- .../src/mbgl/storage/default_file_source.cpp | 24 +++++----- .../default/src/mbgl/storage/offline_database.cpp | 52 ++++++++++++++++++++-- test/storage/offline_database.test.cpp | 47 +++++++++++++------ 5 files changed, 125 insertions(+), 36 deletions(-) diff --git a/include/mbgl/storage/default_file_source.hpp b/include/mbgl/storage/default_file_source.hpp index ddbdab8cf..f46f41771 100644 --- a/include/mbgl/storage/default_file_source.hpp +++ b/include/mbgl/storage/default_file_source.hpp @@ -30,12 +30,8 @@ public: * regions, we want the database to remain fairly small (order tens or low hundreds * of megabytes). */ - DefaultFileSource(const std::string& cachePath, - const std::string& assetPath, - uint64_t maximumCacheSize = util::DEFAULT_MAX_CACHE_SIZE); - DefaultFileSource(const std::string& cachePath, - std::unique_ptr&& assetFileSource, - uint64_t maximumCacheSize = util::DEFAULT_MAX_CACHE_SIZE); + DefaultFileSource(const std::string& cachePath, const std::string& assetPath); + DefaultFileSource(const std::string& cachePath, std::unique_ptr&& assetFileSource); ~DefaultFileSource() override; bool supportsCacheOnlyRequests() const override { @@ -220,6 +216,28 @@ public: */ void clearAmbientCache(std::function); + /* + * Sets the maximum size in bytes for the ambient cache. + * + * This call is potentially expensive because it will try + * to trim the data in case the database is larger than the + * size defined. The size of offline regions are not affected + * by this settings, but the ambient cache will always try + * to not exceed the maximum size defined, taking into account + * the current size for the offline regions. + * + * If the maximum size is set to 50 MB and 40 MB are already + * used by offline regions, the cache size will be effectively + * 10 MB. + * + * Setting the size to 0 will disable the cache if there is no + * offline region on the database. + * + * This method should always be called before using the database, + * otherwise the default maximum size will be used. + */ + void setMaximumAmbientCacheSize(uint64_t size, std::function callback); + // For testing only. void setOnlineStatus(bool); diff --git a/platform/default/include/mbgl/storage/offline_database.hpp b/platform/default/include/mbgl/storage/offline_database.hpp index 59f41a723..afce87b54 100644 --- a/platform/default/include/mbgl/storage/offline_database.hpp +++ b/platform/default/include/mbgl/storage/offline_database.hpp @@ -40,7 +40,7 @@ class OfflineDatabase : private util::noncopyable { public: // Limits affect ambient caching (put) only; resources required by offline // regions are exempt. - OfflineDatabase(std::string path, uint64_t maximumCacheSize = util::DEFAULT_MAX_CACHE_SIZE); + OfflineDatabase(std::string path); ~OfflineDatabase(); void changePath(const std::string&); @@ -86,6 +86,7 @@ public: expected getRegionDefinition(int64_t regionID); expected getRegionCompletedStatus(int64_t regionID); + std::exception_ptr setMaximumAmbientCacheSize(uint64_t); void setOfflineMapboxTileCountLimit(uint64_t); uint64_t getOfflineMapboxTileCountLimit(); bool offlineMapboxTileCountLimitExceeded(); @@ -104,6 +105,7 @@ private: void migrateToVersion3(); void migrateToVersion6(); void cleanup(); + bool disabled(); mapbox::sqlite::Statement& getStatement(const char *); @@ -136,9 +138,9 @@ private: template T getPragma(const char *); - uint64_t maximumCacheSize; - + uint64_t maximumAmbientCacheSize = util::DEFAULT_MAX_CACHE_SIZE; uint64_t offlineMapboxTileCountLimit = util::mapbox::DEFAULT_OFFLINE_TILE_COUNT_LIMIT; + optional offlineMapboxTileCount; bool evict(uint64_t neededFreeSize); diff --git a/platform/default/src/mbgl/storage/default_file_source.cpp b/platform/default/src/mbgl/storage/default_file_source.cpp index b296e448a..19932d8d0 100644 --- a/platform/default/src/mbgl/storage/default_file_source.cpp +++ b/platform/default/src/mbgl/storage/default_file_source.cpp @@ -20,10 +20,10 @@ namespace mbgl { class DefaultFileSource::Impl { public: - Impl(std::shared_ptr assetFileSource_, std::string cachePath, uint64_t maximumCacheSize) + Impl(std::shared_ptr assetFileSource_, std::string cachePath) : assetFileSource(std::move(assetFileSource_)) , localFileSource(std::make_unique()) - , offlineDatabase(std::make_unique(cachePath, maximumCacheSize)) { + , offlineDatabase(std::make_unique(std::move(cachePath))) { } void setAPIBaseURL(const std::string& url) { @@ -196,6 +196,10 @@ public: callback(offlineDatabase->clearAmbientCache()); } + void setMaximumAmbientCacheSize(uint64_t size, std::function callback) { + callback(offlineDatabase->setMaximumAmbientCacheSize(size)); + } + private: expected getDownload(int64_t regionID) { auto it = downloads.find(regionID); @@ -220,17 +224,13 @@ private: std::unordered_map> downloads; }; -DefaultFileSource::DefaultFileSource(const std::string& cachePath, - const std::string& assetPath, - uint64_t maximumCacheSize) - : DefaultFileSource(cachePath, std::make_unique(assetPath), maximumCacheSize) { +DefaultFileSource::DefaultFileSource(const std::string& cachePath, const std::string& assetPath) + : DefaultFileSource(cachePath, std::make_unique(assetPath)) { } -DefaultFileSource::DefaultFileSource(const std::string& cachePath, - std::unique_ptr&& assetFileSource_, - uint64_t maximumCacheSize) +DefaultFileSource::DefaultFileSource(const std::string& cachePath, std::unique_ptr&& assetFileSource_) : assetFileSource(std::move(assetFileSource_)) - , impl(std::make_unique>("DefaultFileSource", assetFileSource, cachePath, maximumCacheSize)) { + , impl(std::make_unique>("DefaultFileSource", assetFileSource, cachePath)) { } DefaultFileSource::~DefaultFileSource() = default; @@ -351,6 +351,10 @@ void DefaultFileSource::clearAmbientCache(std::functionactor().invoke(&Impl::clearAmbientCache, std::move(callback)); } +void DefaultFileSource::setMaximumAmbientCacheSize(uint64_t size, std::function callback) { + impl->actor().invoke(&Impl::setMaximumAmbientCacheSize, size, std::move(callback)); +} + // For testing only: void DefaultFileSource::setOnlineStatus(const bool status) { diff --git a/platform/default/src/mbgl/storage/offline_database.cpp b/platform/default/src/mbgl/storage/offline_database.cpp index 7caa013f9..24ad3e843 100644 --- a/platform/default/src/mbgl/storage/offline_database.cpp +++ b/platform/default/src/mbgl/storage/offline_database.cpp @@ -13,9 +13,8 @@ namespace mbgl { -OfflineDatabase::OfflineDatabase(std::string path_, uint64_t maximumCacheSize_) - : path(std::move(path_)), - maximumCacheSize(maximumCacheSize_) { +OfflineDatabase::OfflineDatabase(std::string path_) + : path(std::move(path_)) { try { initialize(); } catch (const util::IOException& ex) { @@ -88,6 +87,19 @@ void OfflineDatabase::cleanup() { } } +bool OfflineDatabase::disabled() { + if (maximumAmbientCacheSize) { + return false; + } + + auto regions = listRegions(); + if (regions && !regions.value().empty()) { + return false; + } + + return true; +} + void OfflineDatabase::handleError(const mapbox::sqlite::Exception& ex, const char* action) { if (ex.code == mapbox::sqlite::ResultCode::NotADB || ex.code == mapbox::sqlite::ResultCode::Corrupt || @@ -177,6 +189,10 @@ mapbox::sqlite::Statement& OfflineDatabase::getStatement(const char* sql) { } optional OfflineDatabase::get(const Resource& resource) try { + if (disabled()) { + return nullopt; + } + auto result = getInternal(resource); return result ? optional{ result->first } : nullopt; } catch (const util::IOException& ex) { @@ -209,6 +225,11 @@ std::pair OfflineDatabase::put(const Resource& resource, const R if (!db) { initialize(); } + + if (disabled()) { + return { false, 0 }; + } + mapbox::sqlite::Transaction transaction(*db, mapbox::sqlite::Transaction::Immediate); auto result = putInternal(resource, response, true); transaction.commit(); @@ -1120,7 +1141,7 @@ bool OfflineDatabase::evict(uint64_t neededFreeSize) { // The addition of pageSize is a fudge factor to account for non `data` column // size, and because pages can get fragmented on the database. - while (usedSize() + neededFreeSize + pageSize > maximumCacheSize) { + while (usedSize() + neededFreeSize + pageSize > maximumAmbientCacheSize) { // clang-format off mapbox::sqlite::Query accessedQuery{ getStatement( "SELECT max(accessed) " @@ -1187,6 +1208,29 @@ bool OfflineDatabase::evict(uint64_t neededFreeSize) { return true; } +std::exception_ptr OfflineDatabase::setMaximumAmbientCacheSize(uint64_t size) { + uint64_t previousMaximumAmbientCacheSize = maximumAmbientCacheSize; + + try { + maximumAmbientCacheSize = size; + + uint64_t databaseSize = getPragma("PRAGMA page_size") + * getPragma("PRAGMA page_count"); + + if (databaseSize > maximumAmbientCacheSize) { + evict(0); + db->exec("VACUUM"); + } + + return nullptr; + } catch (const mapbox::sqlite::Exception& ex) { + maximumAmbientCacheSize = previousMaximumAmbientCacheSize; + handleError(ex, "set maximum ambient cache size"); + + return std::current_exception(); + } +} + void OfflineDatabase::setOfflineMapboxTileCountLimit(uint64_t limit) { offlineMapboxTileCountLimit = limit; } diff --git a/test/storage/offline_database.test.cpp b/test/storage/offline_database.test.cpp index 5801c77fe..882a10e9e 100644 --- a/test/storage/offline_database.test.cpp +++ b/test/storage/offline_database.test.cpp @@ -318,7 +318,7 @@ TEST(OfflineDatabase, TEST_REQUIRES_WRITE(GetResourceFromOfflineRegion)) { deleteDatabaseFiles(); util::copyFile(filename, "test/fixtures/offline_database/satellite_test.db"); - OfflineDatabase db(filename, mapbox::sqlite::ReadOnly); + OfflineDatabase db(filename); Resource resource = Resource::style("mapbox://styles/mapbox/satellite-v9"); ASSERT_TRUE(db.get(resource)); @@ -755,7 +755,8 @@ TEST(OfflineDatabase, PutReturnsSize) { TEST(OfflineDatabase, PutEvictsLeastRecentlyUsedResources) { FixtureLog log; - OfflineDatabase db(":memory:", 1024 * 100); + OfflineDatabase db(":memory:"); + db.setMaximumAmbientCacheSize(1024 * 100); Response response; response.data = randomString(1024); @@ -773,7 +774,9 @@ TEST(OfflineDatabase, PutEvictsLeastRecentlyUsedResources) { TEST(OfflineDatabase, PutRegionResourceDoesNotEvict) { FixtureLog log; - OfflineDatabase db(":memory:", 1024 * 100); + OfflineDatabase db(":memory:"); + db.setMaximumAmbientCacheSize(1024 * 100); + OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0, true }; auto region = db.createRegion(definition, OfflineRegionMetadata()); ASSERT_TRUE(region); @@ -793,7 +796,8 @@ TEST(OfflineDatabase, PutRegionResourceDoesNotEvict) { TEST(OfflineDatabase, PutFailsWhenEvictionInsuffices) { FixtureLog log; - OfflineDatabase db(":memory:", 1024 * 100); + OfflineDatabase db(":memory:"); + db.setMaximumAmbientCacheSize(1024 * 100); Response big; big.data = randomString(1024 * 100); @@ -849,7 +853,9 @@ TEST(OfflineDatabase, GetRegionCompletedStatus) { TEST(OfflineDatabase, HasRegionResource) { FixtureLog log; - OfflineDatabase db(":memory:", 1024 * 100); + OfflineDatabase db(":memory:"); + db.setMaximumAmbientCacheSize(1024 * 100); + OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0, true }; auto region = db.createRegion(definition, OfflineRegionMetadata()); ASSERT_TRUE(region); @@ -873,7 +879,9 @@ TEST(OfflineDatabase, HasRegionResource) { TEST(OfflineDatabase, HasRegionResourceTile) { FixtureLog log; - OfflineDatabase db(":memory:", 1024 * 100); + OfflineDatabase db(":memory:"); + db.setMaximumAmbientCacheSize(1024 * 100); + OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0, false }; auto region = db.createRegion(definition, OfflineRegionMetadata()); ASSERT_TRUE(region); @@ -968,7 +976,9 @@ TEST(OfflineDatabase, OfflineMapboxTileCount) { TEST(OfflineDatabase, BatchInsertion) { FixtureLog log; - OfflineDatabase db(":memory:", 1024 * 100); + OfflineDatabase db(":memory:"); + db.setMaximumAmbientCacheSize(1024 * 100); + OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0, true }; auto region = db.createRegion(definition, OfflineRegionMetadata()); ASSERT_TRUE(region); @@ -993,8 +1003,10 @@ TEST(OfflineDatabase, BatchInsertion) { TEST(OfflineDatabase, BatchInsertionMapboxTileCountExceeded) { FixtureLog log; - OfflineDatabase db(":memory:", 1024 * 100); + OfflineDatabase db(":memory:"); db.setOfflineMapboxTileCountLimit(1); + db.setMaximumAmbientCacheSize(1024 * 100); + OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0, false }; auto region = db.createRegion(definition, OfflineRegionMetadata()); ASSERT_TRUE(region); @@ -1031,7 +1043,9 @@ TEST(OfflineDatabase, MigrateFromV2Schema) { util::copyFile(filename, "test/fixtures/offline_database/v2.db"); { - OfflineDatabase db(filename, 0); + OfflineDatabase db(filename); + db.setMaximumAmbientCacheSize(0); + auto regions = db.listRegions(); ASSERT_TRUE(regions); for (auto& region : regions.value()) { @@ -1053,7 +1067,9 @@ TEST(OfflineDatabase, MigrateFromV3Schema) { util::copyFile(filename, "test/fixtures/offline_database/v3.db"); { - OfflineDatabase db(filename, 0); + OfflineDatabase db(filename); + db.setMaximumAmbientCacheSize(0); + auto regions = db.listRegions().value(); for (auto& region : regions) { db.deleteRegion(std::move(region)); @@ -1072,7 +1088,9 @@ TEST(OfflineDatabase, MigrateFromV4Schema) { util::copyFile(filename, "test/fixtures/offline_database/v4.db"); { - OfflineDatabase db(filename, 0); + OfflineDatabase db(filename); + db.setMaximumAmbientCacheSize(0); + auto regions = db.listRegions().value(); for (auto& region : regions) { db.deleteRegion(std::move(region)); @@ -1098,7 +1116,9 @@ TEST(OfflineDatabase, MigrateFromV5Schema) { util::copyFile(filename, "test/fixtures/offline_database/v5.db"); { - OfflineDatabase db(filename, 0); + OfflineDatabase db(filename); + db.setMaximumAmbientCacheSize(0); + auto regions = db.listRegions().value(); for (auto& region : regions) { db.deleteRegion(std::move(region)); @@ -1126,7 +1146,8 @@ TEST(OfflineDatabase, DowngradeSchema) { util::copyFile(filename, "test/fixtures/offline_database/v999.db"); { - OfflineDatabase db(filename, 0); + OfflineDatabase db(filename); + db.setMaximumAmbientCacheSize(0); } EXPECT_EQ(6, databaseUserVersion(filename)); -- cgit v1.2.3