From 78eeffe1717a37a91e6c59681421a20c52a4d003 Mon Sep 17 00:00:00 2001 From: "Thiago Marcos P. Santos" Date: Fri, 14 Jun 2019 16:53:09 +0300 Subject: [core] Clear/Invalidate should also work on non-tile resources These methods were only affecting tiles, but they should really work on every resource like style, glyphs, etc. --- benchmark/storage/offline_database.benchmark.cpp | 8 +- include/mbgl/storage/default_file_source.hpp | 20 +++-- .../include/mbgl/storage/offline_database.hpp | 4 +- .../src/mbgl/storage/default_file_source.cpp | 16 ++-- .../default/src/mbgl/storage/offline_database.cpp | 59 ++++++++++++--- test/storage/offline_database.test.cpp | 88 +++++++++++++++------- 6 files changed, 134 insertions(+), 61 deletions(-) diff --git a/benchmark/storage/offline_database.benchmark.cpp b/benchmark/storage/offline_database.benchmark.cpp index cfb1e4c0f2..be17201f33 100644 --- a/benchmark/storage/offline_database.benchmark.cpp +++ b/benchmark/storage/offline_database.benchmark.cpp @@ -26,7 +26,7 @@ public: void resetAmbientTiles() { using namespace mbgl; - db.clearTileCache(); + db.clearAmbientCache(); for (unsigned i = 0; i < tileCount; ++i) { const Resource ambient = Resource::tile("mapbox://tile_ambient" + util::toString(i), 1, 0, 0, 0, Tileset::Scheme::XYZ); @@ -82,13 +82,13 @@ BENCHMARK_F(OfflineDatabase, InsertTileRegion)(benchmark::State& state) { db.putRegionResource(regionID, offline, response); } } -BENCHMARK_F(OfflineDatabase, InvalidateTileCache)(benchmark::State& state) { +BENCHMARK_F(OfflineDatabase, InvalidateAmbientCache)(benchmark::State& state) { while (state.KeepRunning()) { - db.invalidateTileCache(); + db.invalidateAmbientCache(); } } -BENCHMARK_F(OfflineDatabase, ClearTileCache)(benchmark::State& state) { +BENCHMARK_F(OfflineDatabase, ClearAmbientCache)(benchmark::State& state) { while (state.KeepRunning()) { resetAmbientTiles(); } diff --git a/include/mbgl/storage/default_file_source.hpp b/include/mbgl/storage/default_file_source.hpp index 6a63007a96..ddbdab8cf3 100644 --- a/include/mbgl/storage/default_file_source.hpp +++ b/include/mbgl/storage/default_file_source.hpp @@ -195,24 +195,30 @@ public: void resetDatabase(std::function); /* - * Forces revalidation of tiles in the ambient cache. + * Forces revalidation of the ambient cache. * - * Forces Mapbox GL Native to revalidate tiles stored in the ambient + * Forces Mapbox GL Native to revalidate resources stored in the ambient * cache with the tile server before using them, making sure they * are the latest version. This is more efficient than cleaning the - * cache because if the tile is considered valid after the server + * cache because if the resource is considered valid after the server * lookup, it will not get downloaded again. + * + * Resources overlapping with offline regions will not be affected + * by this call. */ - void invalidateTileCache(std::function); + void invalidateAmbientCache(std::function); /* - * Erase tiles from the ambient cache, freeing storage space. + * Erase resources from the ambient cache, freeing storage space. * - * Erases the tile cache, freeing resources. This operation can be + * Erases the ambient cache, freeing resources. This operation can be * potentially slow because it will trigger a VACUUM on SQLite, * forcing the database to move pages on the filesystem. + * + * Resources overlapping with offline regions will not be affected + * by this call. */ - void clearTileCache(std::function); + void clearAmbientCache(std::function); // 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 f9c03dc35d..59f41a723f 100644 --- a/platform/default/include/mbgl/storage/offline_database.hpp +++ b/platform/default/include/mbgl/storage/offline_database.hpp @@ -56,12 +56,12 @@ public: // are the latest version. This is more efficient than cleaning the // cache because if the tile is considered valid after the server // lookup, it will not get downloaded again. - std::exception_ptr invalidateTileCache(); + std::exception_ptr invalidateAmbientCache(); // Clear the tile cache, freeing resources. This operation can be // potentially slow because it will trigger a VACUUM on SQLite, // forcing the database to move pages on the filesystem. - std::exception_ptr clearTileCache(); + std::exception_ptr clearAmbientCache(); expected listRegions(); diff --git a/platform/default/src/mbgl/storage/default_file_source.cpp b/platform/default/src/mbgl/storage/default_file_source.cpp index 0f786f3868..b296e448af 100644 --- a/platform/default/src/mbgl/storage/default_file_source.cpp +++ b/platform/default/src/mbgl/storage/default_file_source.cpp @@ -188,12 +188,12 @@ public: callback(offlineDatabase->resetDatabase()); } - void invalidateTileCache(std::function callback) { - callback(offlineDatabase->invalidateTileCache()); + void invalidateAmbientCache(std::function callback) { + callback(offlineDatabase->invalidateAmbientCache()); } - void clearTileCache(std::function callback) { - callback(offlineDatabase->clearTileCache()); + void clearAmbientCache(std::function callback) { + callback(offlineDatabase->clearAmbientCache()); } private: @@ -343,12 +343,12 @@ void DefaultFileSource::resetDatabase(std::function c impl->actor().invoke(&Impl::resetDatabase, std::move(callback)); } -void DefaultFileSource::invalidateTileCache(std::function callback) { - impl->actor().invoke(&Impl::invalidateTileCache, callback); +void DefaultFileSource::invalidateAmbientCache(std::function callback) { + impl->actor().invoke(&Impl::invalidateAmbientCache, std::move(callback)); } -void DefaultFileSource::clearTileCache(std::function callback) { - impl->actor().invoke(&Impl::clearTileCache, callback); +void DefaultFileSource::clearAmbientCache(std::function callback) { + impl->actor().invoke(&Impl::clearAmbientCache, std::move(callback)); } // For testing only: diff --git a/platform/default/src/mbgl/storage/offline_database.cpp b/platform/default/src/mbgl/storage/offline_database.cpp index ab04cde5ed..7caa013f9f 100644 --- a/platform/default/src/mbgl/storage/offline_database.cpp +++ b/platform/default/src/mbgl/storage/offline_database.cpp @@ -604,9 +604,9 @@ bool OfflineDatabase::putTile(const Resource::TileData& tile, return true; } -std::exception_ptr OfflineDatabase::invalidateTileCache() try { +std::exception_ptr OfflineDatabase::invalidateAmbientCache() try { // clang-format off - mapbox::sqlite::Query query{ getStatement( + mapbox::sqlite::Query tileQuery{ getStatement( "UPDATE tiles " "SET expires = 0, must_revalidate = 1 " "WHERE id NOT IN (" @@ -615,16 +615,29 @@ std::exception_ptr OfflineDatabase::invalidateTileCache() try { ) }; // clang-format on - query.run(); + tileQuery.run(); + + // clang-format off + mapbox::sqlite::Query resourceQuery{ getStatement( + "UPDATE resources " + "SET expires = 0, must_revalidate = 1 " + "WHERE id NOT IN (" + " SELECT resource_id FROM region_resources" + ")" + ) }; + // clang-format on + + resourceQuery.run(); + return nullptr; } catch (const mapbox::sqlite::Exception& ex) { - handleError(ex, "invalidate tile cache"); + handleError(ex, "invalidate ambient cache"); return std::current_exception(); } -std::exception_ptr OfflineDatabase::clearTileCache() try { +std::exception_ptr OfflineDatabase::clearAmbientCache() try { // clang-format off - mapbox::sqlite::Query query{ getStatement( + mapbox::sqlite::Query tileQuery{ getStatement( "DELETE FROM tiles " "WHERE id NOT IN (" " SELECT tile_id FROM region_tiles" @@ -632,20 +645,31 @@ std::exception_ptr OfflineDatabase::clearTileCache() try { ) }; // clang-format on - query.run(); + tileQuery.run(); + + // clang-format off + mapbox::sqlite::Query resourceQuery{ getStatement( + "DELETE FROM resources " + "WHERE id NOT IN (" + " SELECT resource_id FROM region_resources" + ")" + ) }; + // clang-format on + + resourceQuery.run(); db->exec("VACUUM"); return nullptr; } catch (const mapbox::sqlite::Exception& ex) { - handleError(ex, "clear tile cache"); + handleError(ex, "clear ambient cache"); return std::current_exception(); } std::exception_ptr OfflineDatabase::invalidateRegion(int64_t regionID) try { { // clang-format off - mapbox::sqlite::Query query{ getStatement( + mapbox::sqlite::Query tileQuery{ getStatement( "UPDATE tiles " "SET expires = 0, must_revalidate = 1 " "WHERE id IN (" @@ -654,8 +678,21 @@ std::exception_ptr OfflineDatabase::invalidateRegion(int64_t regionID) try { ) }; // clang-format on - query.bind(1, regionID); - query.run(); + tileQuery.bind(1, regionID); + tileQuery.run(); + + // clang-format off + mapbox::sqlite::Query resourceQuery{ getStatement( + "UPDATE resources " + "SET expires = 0, must_revalidate = 1 " + "WHERE id IN (" + " SELECT resource_id FROM region_resources WHERE region_id = ?" + ")" + ) }; + // clang-format on + + resourceQuery.bind(1, regionID); + resourceQuery.run(); } assert(db); diff --git a/test/storage/offline_database.test.cpp b/test/storage/offline_database.test.cpp index d1aa7e2372..5801c77fef 100644 --- a/test/storage/offline_database.test.cpp +++ b/test/storage/offline_database.test.cpp @@ -536,9 +536,12 @@ TEST(OfflineDatabase, TEST_REQUIRES_WRITE(DeleteRegion)) { auto region = db.createRegion(definition, metadata); - for (unsigned i = 0; i < 100; ++i) { + for (unsigned i = 0; i < 50; ++i) { const Resource tile = Resource::tile("mapbox://tile_" + std::to_string(i), 1, 0, 0, 0, Tileset::Scheme::XYZ); db.putRegionResource(region->getID(), tile, response); + + const Resource style = Resource::style("mapbox://style_" + std::to_string(i)); + db.putRegionResource(region->getID(), style, response); } db.deleteRegion(std::move(*region)); @@ -553,7 +556,7 @@ TEST(OfflineDatabase, TEST_REQUIRES_WRITE(DeleteRegion)) { // After clearing the cache, the size of the database // should get back to the original size. - db.clearTileCache(); + db.clearAmbientCache(); } EXPECT_EQ(initialSize, util::read_file(filename).size()); @@ -571,49 +574,73 @@ TEST(OfflineDatabase, Invalidate) { response.mustRevalidate = false; response.expires = util::now() + 1h; - const Resource ambient = Resource::tile("mapbox://tile_ambient", 1, 0, 0, 0, Tileset::Scheme::XYZ); - db.put(ambient, response); + const Resource ambientTile = Resource::tile("mapbox://tile_ambient", 1, 0, 0, 0, Tileset::Scheme::XYZ); + db.put(ambientTile, response); + + const Resource ambientStyle = Resource::style("mapbox://style_ambient"); + db.put(ambientStyle, response); OfflineTilePyramidRegionDefinition definition { "mapbox://style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0, true }; OfflineRegionMetadata metadata {{ 1, 2, 3 }}; auto region1 = db.createRegion(definition, metadata); - const Resource offline1 = Resource::tile("mapbox://tile_offline_region1", 1.0, 0, 0, 0, Tileset::Scheme::XYZ); - db.putRegionResource(region1->getID(), offline1, response); + const Resource region1Tile = Resource::tile("mapbox://tile_offline_region1", 1.0, 0, 0, 0, Tileset::Scheme::XYZ); + db.putRegionResource(region1->getID(), region1Tile, response); + + const Resource region1Style = Resource::style("mapbox://style_offline_region1"); + db.putRegionResource(region1->getID(), region1Style, response); auto region2 = db.createRegion(definition, metadata); - const Resource offline2 = Resource::tile("mapbox://tile_offline_region2", 1.0, 0, 0, 0, Tileset::Scheme::XYZ); - db.putRegionResource(region2->getID(), offline2, response); + const Resource region2Tile = Resource::tile("mapbox://tile_offline_region2", 1.0, 0, 0, 0, Tileset::Scheme::XYZ); + db.putRegionResource(region2->getID(), region2Tile, response); + + const Resource region2Style = Resource::style("mapbox://style_offline_region2"); + db.putRegionResource(region2->getID(), region2Style, response); // Prior to invalidation, all tiles are usable. - EXPECT_TRUE(db.get(ambient)->isUsable()); - EXPECT_TRUE(db.get(offline1)->isUsable()); - EXPECT_TRUE(db.get(offline2)->isUsable()); + EXPECT_TRUE(db.get(ambientTile)->isUsable()); + EXPECT_TRUE(db.get(ambientStyle)->isUsable()); + EXPECT_TRUE(db.get(region1Tile)->isUsable()); + EXPECT_TRUE(db.get(region1Style)->isUsable()); + EXPECT_TRUE(db.get(region2Tile)->isUsable()); + EXPECT_TRUE(db.get(region2Style)->isUsable()); // Invalidate a region will not invalidate ambient // tiles or other regions. EXPECT_TRUE(db.invalidateRegion(region1->getID()) == nullptr); - EXPECT_TRUE(db.get(ambient)->isUsable()); - EXPECT_FALSE(db.get(offline1)->isUsable()); - EXPECT_TRUE(db.get(offline2)->isUsable()); + EXPECT_TRUE(db.get(ambientTile)->isUsable()); + EXPECT_TRUE(db.get(ambientStyle)->isUsable()); + EXPECT_FALSE(db.get(region1Tile)->isUsable()); + EXPECT_FALSE(db.get(region1Style)->isUsable()); + EXPECT_TRUE(db.get(region2Tile)->isUsable()); + EXPECT_TRUE(db.get(region2Style)->isUsable()); // Invalidate the ambient cache will not invalidate // the regions that are still valid. - EXPECT_TRUE(db.invalidateTileCache() == nullptr); + EXPECT_TRUE(db.invalidateAmbientCache() == nullptr); - EXPECT_FALSE(db.get(ambient)->isUsable()); - EXPECT_FALSE(db.get(offline1)->isUsable()); - EXPECT_TRUE(db.get(offline2)->isUsable()); + EXPECT_FALSE(db.get(ambientTile)->isUsable()); + EXPECT_FALSE(db.get(ambientStyle)->isUsable()); + EXPECT_FALSE(db.get(region1Tile)->isUsable()); + EXPECT_FALSE(db.get(region1Style)->isUsable()); + EXPECT_TRUE(db.get(region2Tile)->isUsable()); + EXPECT_TRUE(db.get(region2Style)->isUsable()); // Sanity check. - EXPECT_TRUE(db.get(ambient)->expires < util::now()); - EXPECT_TRUE(db.get(offline1)->expires < util::now()); - EXPECT_TRUE(db.get(offline2)->expires > util::now()); - - EXPECT_TRUE(db.get(ambient)->mustRevalidate); - EXPECT_TRUE(db.get(offline1)->mustRevalidate); - EXPECT_FALSE(db.get(offline2)->mustRevalidate); + EXPECT_TRUE(db.get(ambientTile)->expires < util::now()); + EXPECT_TRUE(db.get(ambientStyle)->expires < util::now()); + EXPECT_TRUE(db.get(region1Tile)->expires < util::now()); + EXPECT_TRUE(db.get(region1Style)->expires < util::now()); + EXPECT_TRUE(db.get(region2Tile)->expires > util::now()); + EXPECT_TRUE(db.get(region2Style)->expires > util::now()); + + EXPECT_TRUE(db.get(ambientTile)->mustRevalidate); + EXPECT_TRUE(db.get(ambientStyle)->mustRevalidate); + EXPECT_TRUE(db.get(region1Tile)->mustRevalidate); + EXPECT_TRUE(db.get(region1Style)->mustRevalidate); + EXPECT_FALSE(db.get(region2Tile)->mustRevalidate); + EXPECT_FALSE(db.get(region2Style)->mustRevalidate); // Should not throw. EXPECT_TRUE(db.invalidateRegion(region2->getID()) == nullptr); @@ -627,7 +654,7 @@ TEST(OfflineDatabase, Invalidate) { EXPECT_EQ(0u, log.uncheckedCount()); } -TEST(OfflineDatabase, TEST_REQUIRES_WRITE(ClearTileCache)) { +TEST(OfflineDatabase, TEST_REQUIRES_WRITE(ClearAmbientCache)) { FixtureLog log; deleteDatabaseFiles(); @@ -643,12 +670,15 @@ TEST(OfflineDatabase, TEST_REQUIRES_WRITE(ClearTileCache)) { OfflineDatabase db(filename); - for (unsigned i = 0; i < 100; ++i) { + for (unsigned i = 0; i < 50; ++i) { const Resource tile = Resource::tile("mapbox://tile_" + std::to_string(i), 1, 0, 0, 0, Tileset::Scheme::XYZ); db.put(tile, response); + + const Resource style = Resource::style("mapbox://style_" + std::to_string(i)); + db.put(style, response); } - db.clearTileCache(); + db.clearAmbientCache(); } EXPECT_EQ(initialSize, util::read_file(filename).size()); @@ -693,7 +723,7 @@ TEST(OfflineDatabase, TEST_REQUIRES_WRITE(ConcurrentUse)) { EXPECT_TRUE(bool(db2.get(fixture::resource))); if (i == 50) { - db2.clearTileCache(); + db2.clearAmbientCache(); } } }); -- cgit v1.2.1