[core] Batch transactions for region downloads

3 files changed, 32 insertions, 13 deletions

diff --git a/platform/default/mbgl/storage/offline_database.cpp b/platform/default/mbgl/storage/offline_database.cpp
index 65c2097182..00fb75f374 100644
--- a/platform/default/mbgl/storage/offline_database.cpp
+++ b/platform/default/mbgl/storage/offline_database.cpp
@@ -25,6 +25,7 @@ OfflineDatabase::~OfflineDatabase() {
     // Deleting these SQLite objects may result in exceptions, but we're in a destructor, so we
     // can't throw anything.
     try {
+        endRegionDownload();
         statements.clear();
         db.reset();
     } catch (mapbox::sqlite::Exception& ex) {
@@ -291,10 +292,6 @@ bool OfflineDatabase::putResource(const Resource& resource,
 
     // We can't use REPLACE because it would change the id value.
 
-    // Begin an immediate-mode transaction to ensure that two writers do not attempt
-    // to INSERT a resource at the same moment.
-    mapbox::sqlite::Transaction transaction(*db, mapbox::sqlite::Transaction::Immediate);
-
     // clang-format off
     Statement update = getStatement(
         "UPDATE resources "
@@ -327,7 +324,6 @@ bool OfflineDatabase::putResource(const Resource& resource,
 
     update->run();
     if (update->changes() != 0) {
-        transaction.commit();
         return false;
     }
 
@@ -354,7 +350,6 @@ bool OfflineDatabase::putResource(const Resource& resource,
     }
 
     insert->run();
-    transaction.commit();
 
     return true;
 }
@@ -480,10 +475,6 @@ bool OfflineDatabase::putTile(const Resource::TileData& tile,
 
     // We can't use REPLACE because it would change the id value.
 
-    // Begin an immediate-mode transaction to ensure that two writers do not attempt
-    // to INSERT a resource at the same moment.
-    mapbox::sqlite::Transaction transaction(*db, mapbox::sqlite::Transaction::Immediate);
-
     // clang-format off
     Statement update = getStatement(
         "UPDATE tiles "
@@ -522,7 +513,6 @@ bool OfflineDatabase::putTile(const Resource::TileData& tile,
 
     update->run();
     if (update->changes() != 0) {
-        transaction.commit();
         return false;
     }
 
@@ -551,12 +541,26 @@ bool OfflineDatabase::putTile(const Resource::TileData& tile,
         insert->bind(12, compressed);
     }
 
+
     insert->run();
-    transaction.commit();
 
     return true;
 }
 
+void OfflineDatabase::beginRegionDownload() {
+    if (!regionTransaction) {
+        regionTransaction = std::make_unique<mapbox::sqlite::Transaction>(*db, mapbox::sqlite::Transaction::Immediate);
+    }
+}
+
+void OfflineDatabase::endRegionDownload() {
+    if (regionTransaction) {
+        regionTransaction->commit();
+        regionTransaction.reset();
+        pendingRegionResources = 0;
+    }
+}
+
 std::vector<OfflineRegion> OfflineDatabase::listRegions() {
     // clang-format off
     Statement stmt = getStatement(
@@ -640,15 +644,21 @@ optional<int64_t> OfflineDatabase::hasRegionResource(int64_t regionID, const Res
 }
 
 uint64_t OfflineDatabase::putRegionResource(int64_t regionID, const Resource& resource, const Response& response) {
+    pendingRegionResources++;
     uint64_t size = putInternal(resource, response, false).second;
     bool previouslyUnused = markUsed(regionID, resource);
+    
+    if (pendingRegionResources >= 128) {
+        endRegionDownload();
+        beginRegionDownload();
+    }
 
     if (offlineMapboxTileCount
         && resource.kind == Resource::Kind::Tile
         && util::mapbox::isMapboxURL(resource.url)
         && previouslyUnused) {
         *offlineMapboxTileCount += 1;
-    }
+    }    
 
     return size;
 }
diff --git a/platform/default/mbgl/storage/offline_database.hpp b/platform/default/mbgl/storage/offline_database.hpp
index 91b544a9e0..fc57e58bc3 100644
--- a/platform/default/mbgl/storage/offline_database.hpp
+++ b/platform/default/mbgl/storage/offline_database.hpp
@@ -15,6 +15,7 @@ namespace mapbox {
 namespace sqlite {
 class Database;
 class Statement;
+class Transaction;
 } // namespace sqlite
 } // namespace mapbox
 
@@ -35,6 +36,9 @@ public:
     // Return value is (inserted, stored size)
     std::pair<bool, uint64_t> put(const Resource&, const Response&);
 
+    void beginRegionDownload();
+    void endRegionDownload();
+
     std::vector<OfflineRegion> listRegions();
 
     OfflineRegion createRegion(const OfflineRegionDefinition&,
@@ -104,6 +108,8 @@ private:
     const std::string path;
     std::unique_ptr<::mapbox::sqlite::Database> db;
     std::unordered_map<const char *, std::unique_ptr<::mapbox::sqlite::Statement>> statements;
+    std::unique_ptr<::mapbox::sqlite::Transaction> regionTransaction;
+    int64_t pendingRegionResources = 0;
 
     template <class T>
     T getPragma(const char *);
diff --git a/platform/default/mbgl/storage/offline_download.cpp b/platform/default/mbgl/storage/offline_download.cpp
index ba504c1f9b..03374deeb7 100644
--- a/platform/default/mbgl/storage/offline_download.cpp
+++ b/platform/default/mbgl/storage/offline_download.cpp
@@ -156,6 +156,7 @@ void OfflineDownload::activateDownload() {
     status = OfflineRegionStatus();
     status.downloadState = OfflineRegionDownloadState::Active;
     status.requiredResourceCount++;
+    offlineDatabase.beginRegionDownload();
     ensureResource(Resource::style(definition.styleURL), [&](Response styleResponse) {
         status.requiredResourceCountIsPrecise = true;
 
@@ -269,6 +270,7 @@ void OfflineDownload::activateDownload() {
 */
 void OfflineDownload::continueDownload() {
     if (resourcesRemaining.empty() && status.complete()) {
+        offlineDatabase.endRegionDownload();
         setState(OfflineRegionDownloadState::Inactive);
         return;
     }
@@ -283,6 +285,7 @@ void OfflineDownload::deactivateDownload() {
     requiredSourceURLs.clear();
     resourcesRemaining.clear();
     requests.clear();
+    offlineDatabase.endRegionDownload();
 }
 
 void OfflineDownload::queueResource(Resource resource) {