[CP][core][android] Decouple vacuum from other operations on offline database

include/mbgl/storage/default_file_source.hpp

@@ -125,19 +125,22 @@ class DefaultFileSource : public FileSource {
      * region deletion is initiated, it is not legal to perform further actions with the
      * region.
      *
+     * Note that this operation can be potentially slow if packing the database occurs
+     * automatically (see runPackDatabaseAutomatically() and packDatabase()).
+     *
      * When the operation is complete or encounters an error, the given callback will be
      * executed on the database thread; it is the responsibility of the SDK bindings
      * to re-execute a user-provided callback on the main thread.
      */
-    void deleteOfflineRegion(OfflineRegion&&, std::function<void (std::exception_ptr)>);
+    void deleteOfflineRegion(OfflineRegion&&, std::function<void(std::exception_ptr)>);
 
     /*
      * Invalidate all the tiles from an offline region forcing Mapbox GL to revalidate
      * the tiles with the server before using. This is more efficient than deleting the
      * offline region and downloading it again because if the data on the cache matches
      * the server, no new data gets transmitted.
      */
-    void invalidateOfflineRegion(OfflineRegion&, std::function<void (std::exception_ptr)>);
+    void invalidateOfflineRegion(OfflineRegion&, std::function<void(std::exception_ptr)>);
 
     /*
      * Changing or bypassing this limit without permission from Mapbox is prohibited
@@ -179,7 +182,29 @@ class DefaultFileSource : public FileSource {
      * executed on the database thread; it is the responsibility of the SDK bindings
      * to re-execute a user-provided callback on the main thread.
      */
-    void resetDatabase(std::function<void (std::exception_ptr)>);
+    void resetDatabase(std::function<void(std::exception_ptr)>);
+
+    /*
+     * Packs the existing database file into a minimal amount of disk space.
+     *
+     * This operation has a performance impact as it will vacuum the database,
+     * forcing it to move pages on the filesystem.
+     *
+     * When the operation is complete or encounters an error, the given callback will be
+     * executed on the database thread; it is the responsibility of the SDK bindings
+     * to re-execute a user-provided callback on the main thread.
+     */
+    void packDatabase(std::function<void(std::exception_ptr)> callback);
+
+    /*
+     * Sets whether packing the database file occurs automatically after an offline
+     * region is deleted (deleteOfflineRegion()) or the ambient cache is cleared
+     * (clearAmbientCache()).
+     *
+     * By default, packing is enabled. If disabled, disk space will not be freed
+     * after resources are removed unless packDatabase() is explicitly called.
+     */
+    void runPackDatabaseAutomatically(bool);
 
     /*
      * Forces revalidation of the ambient cache.
@@ -198,9 +223,10 @@ class DefaultFileSource : public FileSource {
     /*
      * Erase resources from the ambient cache, freeing storage space.
      *
-     * 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.
+     * Erases the ambient cache, freeing resources.
+     *
+     * Note that this operation can be potentially slow if packing the database
+     * occurs automatically (see runPackDatabaseAutomatically() and packDatabase()).
      *
      * Resources overlapping with offline regions will not be affected
      * by this call.

platform/android/CHANGELOG.md

@@ -11,6 +11,8 @@ Mapbox welcomes participation and contributions from everyone.  If you'd like to
 
 ### Performance improvements
  - Enable incremental vacuum for the offline database in order to make data removal requests faster and to avoid the excessive disk space usage (creating a backup file on VACUUM call) [#15837](https://github.com/mapbox/mapbox-gl-native/pull/15837)
+ - Introduce `OfflineManager.packDatabase()` API, which explicitly packs the database file (i.e. runs incremental vacuum for the offline database) [#15899](https://github.com/mapbox/mapbox-gl-native/pull/15899) 
+ - Introduce `OfflineManager.runPackDatabaseAutomatically(boolean)`, which sets whether database file packing (i.e. incremental vacuum operation for the offline database) occurs automatically. [#15967](https://github.com/mapbox/mapbox-gl-native/pull/15967)
 
 ## 8.5.0-alpha.2 - October 10, 2019
 [Changes](https://github.com/mapbox/mapbox-gl-native/compare/android-v8.5.0-alpha.1...android-v8.5.0-alpha.2) since [Mapbox Maps SDK for Android v8.5.0-alpha.1](https://github.com/mapbox/mapbox-gl-native/releases/tag/android-v8.5.0-alpha.1):

platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineManager.java

@@ -306,6 +306,50 @@ public void run() {
     });
   }
 
+  /**
+   * Packs the existing database file into a minimal amount of disk space.
+   * <p>
+   * This operation has a performance impact as it will vacuum the database,
+   * forcing it to move pages on the filesystem.
+   * <p>
+   * When the operation is complete or encounters an error, the given callback will be
+   * executed on the database thread; it is the responsibility of the SDK bindings
+   * to re-execute a user-provided callback on the main thread.
+   * </p>
+   *
+   * @param callback the callback to be invoked when the database was reset or when the operation erred.
+   */
+  public void packDatabase(@Nullable final FileSourceCallback callback) {
+    fileSource.activate();
+    nativePackDatabase(new FileSourceCallback() {
+      @Override
+      public void onSuccess() {
+        handler.post(new Runnable() {
+          @Override
+          public void run() {
+            fileSource.deactivate();
+            if (callback != null) {
+              callback.onSuccess();
+            }
+          }
+        });
+      }
+
+      @Override
+      public void onError(@NonNull final String message) {
+        handler.post(new Runnable() {
+          @Override
+          public void run() {
+            fileSource.deactivate();
+            if (callback != null) {
+              callback.onError(message);
+            }
+          }
+        });
+      }
+    });
+  }
+
   /**
    * Forces re-validation of the ambient cache.
    * <p>
@@ -355,9 +399,10 @@ public void run() {
   /**
    * Erase resources from the ambient cache, freeing storage space.
    * <p>
-   * 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.
+   * Erases the ambient cache, freeing resources.
+   * <p>
+   * Note that this operation can be potentially slow if packing the database
+   * occurs automatically ({@link OfflineManager#runPackDatabaseAutomatically(boolean)}).
    * </p>
    * <p>
    * Resources overlapping with offline regions will not be affected
@@ -620,14 +665,32 @@ private boolean isValidOfflineRegionDefinition(OfflineRegionDefinition definitio
    * <p>
    * Once this limit is reached, {@link OfflineRegion.OfflineRegionObserver#mapboxTileCountLimitExceeded(long)}
    * fires every additional attempt to download additional tiles until already downloaded tiles are removed
-   * by calling {@link OfflineRegion#deleteOfflineRegion(OfflineRegion.OfflineRegionDeleteCallback)}.
+   * by calling {@link OfflineRegion#delete(OfflineRegion.OfflineRegionDeleteCallback)}.
    * </p>
    *
    * @param limit the maximum number of tiles allowed to be downloaded
    */
   @Keep
   public native void setOfflineMapboxTileCountLimit(long limit);
 
+  /**
+   * Sets whether database file packing occurs automatically.
+   * By default, the automatic database file packing is enabled.
+   * <p>
+   * If packing is enabled, database file packing occurs automatically
+   * after an offline region is deleted by calling
+   * {@link OfflineRegion#delete(OfflineRegion.OfflineRegionDeleteCallback)}
+   * or the ambient cache is cleared by calling {@link OfflineManager#clearAmbientCache()}.
+   *
+   * If packing is disabled, disk space will not be freed after
+   * resources are removed unless {@link OfflineManager#packDatabase()} is explicitly called.
+   * </p>
+   *
+   * @param autopack flag setting the automatic database file packing.
+   */
+  @Keep
+  public native void runPackDatabaseAutomatically(boolean autopack);
+
   @Keep
   private native void initialize(FileSource fileSource);
 
@@ -648,6 +711,9 @@ private native void createOfflineRegion(FileSource fileSource, OfflineRegionDefi
   @Keep
   private native void nativeResetDatabase(@Nullable FileSourceCallback callback);
 
+  @Keep
+  private native void nativePackDatabase(@Nullable FileSourceCallback callback);
+
   @Keep
   private native void nativeInvalidateAmbientCache(@Nullable FileSourceCallback callback);
 

platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineRegion.java

@@ -388,6 +388,10 @@ public void run() {
    * by other regions, until the database shrinks below a certain size.
    * </p>
    * <p>
+   * Note that this operation can be potentially slow if packing the database
+   * occurs automatically ({@link OfflineManager#runPackDatabaseAutomatically(boolean)}).
+   * </p>
+   * <p>
    * When the operation is complete or encounters an error, the given callback will be
    * executed on the main thread.
    * </p>

platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/offline/CacheTest.kt

@@ -85,4 +85,20 @@ class CacheTest {
     }
     countDownLatch.await()
   }
+
+  @Test
+  fun testSetPackDatabase() {
+    rule.activity.runOnUiThread {
+      OfflineManager.getInstance(context).packDatabase(object : OfflineManager.FileSourceCallback {
+        override fun onSuccess() {
+          countDownLatch.countDown()
+        }
+
+        override fun onError(message: String) {
+          Assert.assertNull("onError should not be called", message)
+        }
+      })
+    }
+    countDownLatch.await()
+  }
 }

platform/android/src/offline/offline_manager.cpp

@@ -7,8 +7,21 @@
 namespace mbgl {
 namespace android {
 
-// OfflineManager //
+namespace {
+// Reattach, the callback comes from a different thread
+void handleException(std::exception_ptr exception,
+                     const jni::Object<OfflineManager::FileSourceCallback>& callback,
+                     android::UniqueEnv env = android::AttachEnv()) {
+    if (exception) {
+        OfflineManager::FileSourceCallback::onError(
+            *env, callback, jni::Make<jni::String>(*env, mbgl::util::toString(exception)));
+    } else {
+        OfflineManager::FileSourceCallback::onSuccess(*env, callback);
+    }
+}
+} // namespace
 
+// OfflineManager //
 OfflineManager::OfflineManager(jni::JNIEnv& env, const jni::Object<FileSource>& jFileSource)
     : fileSource(std::static_pointer_cast<DefaultFileSource>(mbgl::FileSource::getSharedFileSource(FileSource::getSharedResourceOptions(env, jFileSource)))) {}
 
@@ -107,77 +120,56 @@ void OfflineManager::mergeOfflineRegions(jni::JNIEnv& env_, const jni::Object<Fi
 void OfflineManager::resetDatabase(jni::JNIEnv& env_, const jni::Object<FileSourceCallback>& callback_) {
     auto globalCallback = jni::NewGlobal<jni::EnvAttachingDeleter>(env_, callback_);
 
-    fileSource->resetDatabase([
-        //Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
-        callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))
-    ](std::exception_ptr exception) mutable {
+    fileSource->resetDatabase(
+        [
+            // Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
+            callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))](
+            std::exception_ptr exception) mutable { handleException(exception, *callback); });
+}
 
-        // Reattach, the callback comes from a different thread
-        android::UniqueEnv env = android::AttachEnv();
+void OfflineManager::packDatabase(jni::JNIEnv& env_, const jni::Object<FileSourceCallback>& callback_) {
+    auto globalCallback = jni::NewGlobal<jni::EnvAttachingDeleter>(env_, callback_);
 
-        if (exception) {
-            OfflineManager::FileSourceCallback::onError(*env, *callback, jni::Make<jni::String>(*env, mbgl::util::toString(exception)));
-        } else {
-            OfflineManager::FileSourceCallback::onSuccess(*env, *callback);
-        }
-    });
+    fileSource->packDatabase(
+        [
+            // Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
+            callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))](
+            std::exception_ptr exception) mutable { handleException(exception, *callback); });
 }
 
 void OfflineManager::invalidateAmbientCache(jni::JNIEnv& env_, const jni::Object<FileSourceCallback>& callback_) {
     auto globalCallback = jni::NewGlobal<jni::EnvAttachingDeleter>(env_, callback_);
 
-    fileSource->invalidateAmbientCache([
-        //Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
-        callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))
-    ](std::exception_ptr exception) mutable {
-
-        // Reattach, the callback comes from a different thread
-        android::UniqueEnv env = android::AttachEnv();
-
-        if (exception) {
-            OfflineManager::FileSourceCallback::onError(*env, *callback, jni::Make<jni::String>(*env, mbgl::util::toString(exception)));
-        } else {
-            OfflineManager::FileSourceCallback::onSuccess(*env, *callback);
-        }
-    });
+    fileSource->invalidateAmbientCache(
+        [
+            // Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
+            callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))](
+            std::exception_ptr exception) mutable { handleException(exception, *callback); });
 }
 
 void OfflineManager::clearAmbientCache(jni::JNIEnv& env_, const jni::Object<FileSourceCallback>& callback_) {
     auto globalCallback = jni::NewGlobal<jni::EnvAttachingDeleter>(env_, callback_);
 
-    fileSource->clearAmbientCache([
-        //Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
-        callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))
-    ](std::exception_ptr exception) mutable {
-
-        // Reattach, the callback comes from a different thread
-        android::UniqueEnv env = android::AttachEnv();
-
-        if (exception) {
-            OfflineManager::FileSourceCallback::onError(*env, *callback, jni::Make<jni::String>(*env, mbgl::util::toString(exception)));
-        } else {
-            OfflineManager::FileSourceCallback::onSuccess(*env, *callback);
-        }
-    });
+    fileSource->clearAmbientCache(
+        [
+            // Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
+            callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))](
+            std::exception_ptr exception) mutable { handleException(exception, *callback); });
 }
 
 void OfflineManager::setMaximumAmbientCacheSize(jni::JNIEnv& env_, const jni::jlong size_, const jni::Object<FileSourceCallback>& callback_) {
     auto globalCallback = jni::NewGlobal<jni::EnvAttachingDeleter>(env_, callback_);
 
-    fileSource->setMaximumAmbientCacheSize(size_, [
-        //Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
-        callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))
-    ](std::exception_ptr exception) mutable {
-
-        // Reattach, the callback comes from a different thread
-        android::UniqueEnv env = android::AttachEnv();
+    fileSource->setMaximumAmbientCacheSize(
+        size_,
+        [
+            // Keep a shared ptr to a global reference of the callback so they are not GC'd in the meanwhile
+            callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))](
+            std::exception_ptr exception) mutable { handleException(exception, *callback); });
+}
 
-        if (exception) {
-            OfflineManager::FileSourceCallback::onError(*env, *callback, jni::Make<jni::String>(*env, mbgl::util::toString(exception)));
-        } else {
-            OfflineManager::FileSourceCallback::onSuccess(*env, *callback);
-        }
-    });
+void OfflineManager::runPackDatabaseAutomatically(jni::JNIEnv&, jboolean autopack) {
+    fileSource->runPackDatabaseAutomatically(autopack);
 }
 
 // FileSource::FileSourceCallback //
@@ -207,7 +199,10 @@ void OfflineManager::registerNative(jni::JNIEnv& env) {
 
     #define METHOD(MethodPtr, name) jni::MakeNativePeerMethod<decltype(MethodPtr), (MethodPtr)>(name)
 
-    jni::RegisterNativePeer<OfflineManager>( env, javaClass, "nativePtr",
+    jni::RegisterNativePeer<OfflineManager>(
+        env,
+        javaClass,
+        "nativePtr",
         jni::MakePeer<OfflineManager, const jni::Object<FileSource>&>,
         "initialize",
         "finalize",
@@ -216,9 +211,11 @@ void OfflineManager::registerNative(jni::JNIEnv& env) {
         METHOD(&OfflineManager::createOfflineRegion, "createOfflineRegion"),
         METHOD(&OfflineManager::mergeOfflineRegions, "mergeOfflineRegions"),
         METHOD(&OfflineManager::resetDatabase, "nativeResetDatabase"),
+        METHOD(&OfflineManager::packDatabase, "nativePackDatabase"),
         METHOD(&OfflineManager::invalidateAmbientCache, "nativeInvalidateAmbientCache"),
         METHOD(&OfflineManager::clearAmbientCache, "nativeClearAmbientCache"),
         METHOD(&OfflineManager::setMaximumAmbientCacheSize, "nativeSetMaximumAmbientCacheSize"),
+        METHOD(&OfflineManager::runPackDatabaseAutomatically, "runPackDatabaseAutomatically"),
         METHOD(&OfflineManager::putResourceWithUrl, "putResourceWithUrl"));
 }
 

platform/android/src/offline/offline_manager.hpp

@@ -95,12 +95,16 @@ class OfflineManager {
 
     void resetDatabase(jni::JNIEnv&, const jni::Object<FileSourceCallback>& callback_);
 
+    void packDatabase(jni::JNIEnv&, const jni::Object<FileSourceCallback>& callback_);
+
     void invalidateAmbientCache(jni::JNIEnv&, const jni::Object<FileSourceCallback>& callback_);
 
     void clearAmbientCache(jni::JNIEnv&, const jni::Object<FileSourceCallback>& callback_);
 
     void setMaximumAmbientCacheSize(jni::JNIEnv&, const jni::jlong size, const jni::Object<FileSourceCallback>& callback_);
 
+    void runPackDatabaseAutomatically(jni::JNIEnv&, jboolean autopack);
+
 private:
     std::shared_ptr<mbgl::DefaultFileSource> fileSource;
 };