[core] offline database - batch region resource insertsupstream/offline-download-batches

5 files changed, 131 insertions, 20 deletions

diff --git a/platform/default/mbgl/storage/offline_database.cpp b/platform/default/mbgl/storage/offline_database.cpp
index e8402d6fb2..15cbf3a7b2 100644
--- a/platform/default/mbgl/storage/offline_database.cpp
+++ b/platform/default/mbgl/storage/offline_database.cpp
@@ -614,6 +614,43 @@ optional<int64_t> OfflineDatabase::hasRegionResource(int64_t regionID, const Res
 }
 
 uint64_t OfflineDatabase::putRegionResource(int64_t regionID, const Resource& resource, const Response& response) {
+    mapbox::sqlite::Transaction transaction(*db);
+    auto size = putRegionResourceInternal(regionID, resource, response);
+    transaction.commit();
+    return size;
+}
+
+void OfflineDatabase::putRegionResources(int64_t regionID, const std::list<std::tuple<Resource, Response>>& resources, OfflineRegionStatus& status) {
+    mapbox::sqlite::Transaction transaction(*db);
+
+    for (const auto& elem : resources) {
+        const auto& resource = std::get<0>(elem);
+        const auto& response = std::get<1>(elem);
+
+        try {
+            uint64_t resourceSize = putRegionResourceInternal(regionID, resource, response);
+            status.completedResourceCount++;
+            status.completedResourceSize += resourceSize;
+            if (resource.kind == Resource::Kind::Tile) {
+                status.completedTileCount += 1;
+                status.completedTileSize += resourceSize;
+            }
+        } catch (MapboxTileLimitExceededException) {
+            // Commit the rest of the batch and retrow
+            transaction.commit();
+            throw;
+        }
+    }
+
+    // Commit the completed batch
+    transaction.commit();
+}
+
+uint64_t OfflineDatabase::putRegionResourceInternal(int64_t regionID, const Resource& resource, const Response& response) {
+    if (exceedsOfflineMapboxTileCountLimit(resource)) {
+        throw MapboxTileLimitExceededException();
+    }
+
     uint64_t size = putInternal(resource, response, false).second;
     bool previouslyUnused = markUsed(regionID, resource);
 
@@ -884,4 +921,10 @@ uint64_t OfflineDatabase::getOfflineMapboxTileCount() {
     return *offlineMapboxTileCount;
 }
 
+bool OfflineDatabase::exceedsOfflineMapboxTileCountLimit(const Resource& resource) {
+    return resource.kind == Resource::Kind::Tile
+        && util::mapbox::isMapboxURL(resource.url)
+        && offlineMapboxTileCountLimitExceeded();
+}
+
 } // namespace mbgl
diff --git a/platform/default/mbgl/storage/offline_database.hpp b/platform/default/mbgl/storage/offline_database.hpp
index 9673ad8212..ae6afa67cf 100644
--- a/platform/default/mbgl/storage/offline_database.hpp
+++ b/platform/default/mbgl/storage/offline_database.hpp
@@ -2,6 +2,7 @@
 
 #include <mbgl/storage/resource.hpp>
 #include <mbgl/storage/offline.hpp>
+#include <mbgl/util/exception.hpp>
 #include <mbgl/util/noncopyable.hpp>
 #include <mbgl/util/optional.hpp>
 #include <mbgl/util/constants.hpp>
@@ -10,6 +11,7 @@
 #include <unordered_map>
 #include <memory>
 #include <string>
+#include <list>
 
 namespace mapbox {
 namespace sqlite {
@@ -24,6 +26,10 @@ namespace mbgl {
 class Response;
 class TileID;
 
+struct MapboxTileLimitExceededException :  util::Exception {
+    MapboxTileLimitExceededException() : util::Exception("Mapbox tile limit exceeded") {}
+};
+
 class OfflineDatabase : private util::noncopyable {
 public:
     // Limits affect ambient caching (put) only; resources required by offline
@@ -49,6 +55,7 @@ public:
     optional<std::pair<Response, uint64_t>> getRegionResource(int64_t regionID, const Resource&);
     optional<int64_t> hasRegionResource(int64_t regionID, const Resource&);
     uint64_t putRegionResource(int64_t regionID, const Resource&, const Response&);
+    void putRegionResources(int64_t regionID, const std::list<std::tuple<Resource, Response>>&, OfflineRegionStatus&);
 
     OfflineRegionDefinition getRegionDefinition(int64_t regionID);
     OfflineRegionStatus getRegionCompletedStatus(int64_t regionID);
@@ -57,6 +64,7 @@ public:
     uint64_t getOfflineMapboxTileCountLimit();
     bool offlineMapboxTileCountLimitExceeded();
     uint64_t getOfflineMapboxTileCount();
+    bool exceedsOfflineMapboxTileCountLimit(const Resource&);
 
 private:
     void connect(int flags);
@@ -79,6 +87,8 @@ private:
     bool putResource(const Resource&, const Response&,
                      const std::string&, bool compressed);
 
+    uint64_t putRegionResourceInternal(int64_t regionID, const Resource&, const Response&);
+
     optional<std::pair<Response, uint64_t>> getInternal(const Resource&);
     optional<int64_t> hasInternal(const Resource&);
     std::pair<bool, uint64_t> putInternal(const Resource&, const Response&, bool evict);
diff --git a/platform/default/mbgl/storage/offline_download.cpp b/platform/default/mbgl/storage/offline_download.cpp
index ba504c1f9b..1bd29f031c 100644
--- a/platform/default/mbgl/storage/offline_download.cpp
+++ b/platform/default/mbgl/storage/offline_download.cpp
@@ -330,7 +330,8 @@ void OfflineDownload::ensureResource(const Resource& resource,
             return;
         }
 
-        if (checkTileCountLimit(resource)) {
+        if (offlineDatabase.exceedsOfflineMapboxTileCountLimit(resource)) {
+            onMapboxTileCountLimitExceeded();
             return;
         }
 
@@ -347,17 +348,24 @@ void OfflineDownload::ensureResource(const Resource& resource,
                 callback(onlineResponse);
             }
 
-            status.completedResourceCount++;
-            uint64_t resourceSize = offlineDatabase.putRegionResource(id, resource, onlineResponse);
-            status.completedResourceSize += resourceSize;
-            if (resource.kind == Resource::Kind::Tile) {
-                status.completedTileCount += 1;
-                status.completedTileSize += resourceSize;
-            }
+            // Queue up for batched insertion
+            buffer.emplace_back(resource, onlineResponse);
 
-            observer->statusChanged(status);
+            // Flush buffer periodically
+            if (buffer.size() == 64 || resourcesRemaining.size() == 0) {
+                try {
+                    offlineDatabase.putRegionResources(id, buffer, status);
+                } catch (MapboxTileLimitExceededException) {
+                    onMapboxTileCountLimitExceeded();
+                    return;
+                }
 
-            if (checkTileCountLimit(resource)) {
+                buffer.clear();
+                observer->statusChanged(status);
+            }
+
+            if (offlineDatabase.exceedsOfflineMapboxTileCountLimit(resource)) {
+                onMapboxTileCountLimitExceeded();
                 return;
             }
 
@@ -366,15 +374,9 @@ void OfflineDownload::ensureResource(const Resource& resource,
     });
 }
 
-bool OfflineDownload::checkTileCountLimit(const Resource& resource) {
-    if (resource.kind == Resource::Kind::Tile && util::mapbox::isMapboxURL(resource.url) &&
-        offlineDatabase.offlineMapboxTileCountLimitExceeded()) {
-        observer->mapboxTileCountLimitExceeded(offlineDatabase.getOfflineMapboxTileCountLimit());
-        setState(OfflineRegionDownloadState::Inactive);
-        return true;
-    }
-
-    return false;
+void OfflineDownload::onMapboxTileCountLimitExceeded() {
+    observer->mapboxTileCountLimitExceeded(offlineDatabase.getOfflineMapboxTileCountLimit());
+    setState(OfflineRegionDownloadState::Inactive);
 }
 
 } // namespace mbgl
diff --git a/platform/default/mbgl/storage/offline_download.hpp b/platform/default/mbgl/storage/offline_download.hpp
index 437f221c11..cffac1665b 100644
--- a/platform/default/mbgl/storage/offline_download.hpp
+++ b/platform/default/mbgl/storage/offline_download.hpp
@@ -46,7 +46,8 @@ private:
      * is deactivated, all in progress requests are cancelled.
      */
     void ensureResource(const Resource&, std::function<void (Response)> = {});
-    bool checkTileCountLimit(const Resource& resource);
+
+    void onMapboxTileCountLimitExceeded();
 
     int64_t id;
     OfflineRegionDefinition definition;
@@ -58,6 +59,7 @@ private:
     std::list<std::unique_ptr<AsyncRequest>> requests;
     std::unordered_set<std::string> requiredSourceURLs;
     std::deque<Resource> resourcesRemaining;
+    std::list<std::tuple<Resource, Response>> buffer;
 
     void queueResource(Resource);
     void queueTiles(style::SourceType, uint16_t tileSize, const Tileset&);
diff --git a/test/storage/offline_database.test.cpp b/test/storage/offline_database.test.cpp
index 620e6eaa6d..5fd787f9dd 100644
--- a/test/storage/offline_database.test.cpp
+++ b/test/storage/offline_database.test.cpp
@@ -598,6 +598,60 @@ TEST(OfflineDatabase, OfflineMapboxTileCount) {
     EXPECT_EQ(0u, db.getOfflineMapboxTileCount());
 }
 
+
+TEST(OfflineDatabase, BatchInsertion) {
+    using namespace mbgl;
+
+    OfflineDatabase db(":memory:", 1024 * 100);
+    OfflineRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 };
+    OfflineRegion region = db.createRegion(definition, OfflineRegionMetadata());
+
+    Response response;
+    response.data = randomString(1024);
+    std::list<std::tuple<Resource, Response>> resources;
+
+    for (uint32_t i = 1; i <= 100; i++) {
+        resources.emplace_back(Resource::style("http://example.com/"s + util::toString(i)), response);
+    }
+
+    OfflineRegionStatus status;
+    db.putRegionResources(region.getID(), resources, status);
+
+    for (uint32_t i = 1; i <= 100; i++) {
+        EXPECT_TRUE(bool(db.get(Resource::style("http://example.com/"s + util::toString(i)))));
+    }
+}
+
+TEST(OfflineDatabase, BatchInsertionMapboxTileCountExceeded) {
+    using namespace mbgl;
+    
+    OfflineDatabase db(":memory:", 1024 * 100);
+    db.setOfflineMapboxTileCountLimit(1);
+    OfflineRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 };
+    OfflineRegion region = db.createRegion(definition, OfflineRegionMetadata());
+    
+    Response response;
+    response.data = randomString(1024);
+    std::list<std::tuple<Resource, Response>> resources;
+    
+    resources.emplace_back(Resource::style("http://example.com/"), response);
+    resources.emplace_back(Resource::tile("mapbox://tiles/1", 1.0, 0, 0, 0, Tileset::Scheme::XYZ), response);
+    resources.emplace_back(Resource::tile("mapbox://tiles/2", 1.0, 0, 0, 0, Tileset::Scheme::XYZ), response);
+    
+    OfflineRegionStatus status;
+    try {
+        db.putRegionResources(region.getID(), resources, status);
+        EXPECT_FALSE(true);
+    } catch (MapboxTileLimitExceededException) {
+        // Expected
+    }
+    
+    EXPECT_EQ(status.completedTileCount, 1u);
+    EXPECT_EQ(status.completedResourceCount, 2u);
+    EXPECT_EQ(db.getRegionCompletedStatus(region.getID()).completedTileCount, 1u);
+    EXPECT_EQ(db.getRegionCompletedStatus(region.getID()).completedResourceCount, 2u);
+}
+
 static int databasePageCount(const std::string& path) {
     mapbox::sqlite::Database db{ path, mapbox::sqlite::ReadOnly };
     mapbox::sqlite::Statement stmt{ db, "pragma page_count" };