[core] Introduce OfflineDatabase::runPackDatabaseAutomatically() API

- added a unit test
- Updated inline comments in default_file_source.hpp
This commit is contained in:
Mikhail Pozdnyakov
2019-11-25 12:18:32 +02:00
parent 0ec01356d8
commit 4c49f0c9db
6 changed files with 76 additions and 52 deletions

View File

@@ -121,20 +121,18 @@ public:
* Eviction works by removing the least-recently requested resources not also required
* by other regions, until the database shrinks below a certain size.
*
* If the |pack| argument is `false` the database file packing is skipped and the
* database file does not shrink after this operation completes. Database file
* packing can be done later with `packDatabase()`. It is a useful optimization
* e.g. when several regions should be deleted in a row.
*
* Note that this method takes ownership of the input, reflecting the fact that once
* 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)>, bool pack = true);
void deleteOfflineRegion(OfflineRegion&&, std::function<void(std::exception_ptr)>);
/*
* Invalidate all the tiles from an offline region forcing Mapbox GL to revalidate
@@ -189,12 +187,25 @@ public:
/*
* 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.
*
@@ -212,9 +223,10 @@ public:
/*
* 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.

View File

@@ -104,37 +104,40 @@ void OfflineRegion::getOfflineRegionStatus(jni::JNIEnv& env_, const jni::Object<
void OfflineRegion::deleteOfflineRegion(jni::JNIEnv& env_, const jni::Object<OfflineRegionDeleteCallback>& callback_) {
auto globalCallback = jni::NewGlobal<jni::EnvAttachingDeleter>(env_, callback_);
fileSource->deleteOfflineRegion(std::move(*region), [
//Ensure the object is not gc'd in the meanwhile
callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))
](std::exception_ptr error) mutable {
// Reattach, the callback comes from a different thread
android::UniqueEnv env = android::AttachEnv();
fileSource->deleteOfflineRegion(std::move(*region),
[
// Ensure the object is not gc'd in the meanwhile
callback = std::make_shared<decltype(globalCallback)>(
std::move(globalCallback))](std::exception_ptr error) mutable {
// Reattach, the callback comes from a different thread
android::UniqueEnv env = android::AttachEnv();
if (error) {
OfflineRegionDeleteCallback::onError(*env, *callback, error);
} else {
OfflineRegionDeleteCallback::onDelete(*env, *callback);
}
});
if (error) {
OfflineRegionDeleteCallback::onError(*env, *callback, error);
} else {
OfflineRegionDeleteCallback::onDelete(*env, *callback);
}
});
}
void OfflineRegion::invalidateOfflineRegion(jni::JNIEnv& env_, const jni::Object<OfflineRegionInvalidateCallback>& callback_) {
void OfflineRegion::invalidateOfflineRegion(jni::JNIEnv& env_,
const jni::Object<OfflineRegionInvalidateCallback>& callback_) {
auto globalCallback = jni::NewGlobal<jni::EnvAttachingDeleter>(env_, callback_);
fileSource->invalidateOfflineRegion(*region, [
//Ensure the object is not gc'd in the meanwhile
callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))
](std::exception_ptr error) mutable {
// Reattach, the callback comes from a different thread
android::UniqueEnv env = android::AttachEnv();
fileSource->invalidateOfflineRegion(*region,
[
// Ensure the object is not gc'd in the meanwhile
callback = std::make_shared<decltype(globalCallback)>(
std::move(globalCallback))](std::exception_ptr error) mutable {
// Reattach, the callback comes from a different thread
android::UniqueEnv env = android::AttachEnv();
if (error) {
OfflineRegionInvalidateCallback::onError(*env, *callback, error);
} else {
OfflineRegionInvalidateCallback::onInvalidate(*env, *callback);
}
});
if (error) {
OfflineRegionInvalidateCallback::onError(*env, *callback, error);
} else {
OfflineRegionInvalidateCallback::onInvalidate(*env, *callback);
}
});
}
void OfflineRegion::updateOfflineRegionMetadata(jni::JNIEnv& env_, const jni::Array<jni::jbyte>& jMetadata, const jni::Object<OfflineRegionUpdateMetadataCallback>& callback_) {
@@ -206,7 +209,10 @@ void OfflineRegion::registerNative(jni::JNIEnv& env) {
#define METHOD(MethodPtr, name) jni::MakeNativePeerMethod<decltype(MethodPtr), (MethodPtr)>(name)
jni::RegisterNativePeer<OfflineRegion>( env, javaClass, "nativePtr",
jni::RegisterNativePeer<OfflineRegion>(
env,
javaClass,
"nativePtr",
jni::MakePeer<OfflineRegion, jni::jlong, const jni::Object<FileSource>&>,
"initialize",
"finalize",
@@ -215,8 +221,7 @@ void OfflineRegion::registerNative(jni::JNIEnv& env) {
METHOD(&OfflineRegion::getOfflineRegionStatus, "getOfflineRegionStatus"),
METHOD(&OfflineRegion::deleteOfflineRegion, "deleteOfflineRegion"),
METHOD(&OfflineRegion::invalidateOfflineRegion, "invalidateOfflineRegion"),
METHOD(&OfflineRegion::updateOfflineRegionMetadata, "updateOfflineRegionMetadata")
);
METHOD(&OfflineRegion::updateOfflineRegionMetadata, "updateOfflineRegionMetadata"));
}
// OfflineRegionObserver //
@@ -227,7 +232,7 @@ void OfflineRegion::OfflineRegionStatusCallback::onError(jni::JNIEnv& env,
const jni::Object<OfflineRegion::OfflineRegionStatusCallback>& callback,
std::exception_ptr error) {
static auto& javaClass = jni::Class<OfflineRegion::OfflineRegionStatusCallback>::Singleton(env);
static auto method = javaClass.GetMethod<void (jni::String)>(env, "onError");
static auto method = javaClass.GetMethod<void(jni::String)>(env, "onError");
callback.Call(env, method, jni::Make<jni::String>(env, mbgl::util::toString(error)));
}

View File

@@ -74,7 +74,7 @@ public:
expected<OfflineRegionMetadata, std::exception_ptr>
updateMetadata(const int64_t regionID, const OfflineRegionMetadata&);
std::exception_ptr deleteRegion(OfflineRegion&&, bool pack = true);
std::exception_ptr deleteRegion(OfflineRegion&&);
std::exception_ptr invalidateRegion(int64_t regionID);
// Return value is (response, stored size)
@@ -94,6 +94,7 @@ public:
bool exceedsOfflineMapboxTileCountLimit(const Resource&);
void markUsedResources(int64_t regionID, const std::list<Resource>&);
std::exception_ptr pack();
void runPackDatabaseAutomatically(bool autopack_) { autopack = autopack_; }
private:
void initialize();
@@ -149,6 +150,7 @@ private:
optional<uint64_t> offlineMapboxTileCount;
bool evict(uint64_t neededFreeSize);
bool autopack = true;
};
} // namespace mbgl

View File

@@ -82,9 +82,9 @@ public:
}
}
void deleteRegion(OfflineRegion&& region, std::function<void(std::exception_ptr)> callback, bool pack) {
void deleteRegion(OfflineRegion&& region, std::function<void(std::exception_ptr)> callback) {
downloads.erase(region.getID());
callback(offlineDatabase->deleteRegion(std::move(region), pack));
callback(offlineDatabase->deleteRegion(std::move(region)));
}
void invalidateRegion(int64_t regionID, std::function<void (std::exception_ptr)> callback) {
@@ -208,6 +208,8 @@ public:
void packDatabase(std::function<void(std::exception_ptr)> callback) { callback(offlineDatabase->pack()); }
void runPackDatabaseAutomatically(bool autopack) { offlineDatabase->runPackDatabaseAutomatically(autopack); }
private:
expected<OfflineDownload*, std::exception_ptr> getDownload(int64_t regionID) {
auto it = downloads.find(regionID);
@@ -316,10 +318,8 @@ void DefaultFileSource::updateOfflineMetadata(const int64_t regionID,
impl->actor().invoke(&Impl::updateMetadata, regionID, metadata, callback);
}
void DefaultFileSource::deleteOfflineRegion(OfflineRegion&& region,
std::function<void(std::exception_ptr)> callback,
bool pack) {
impl->actor().invoke(&Impl::deleteRegion, std::move(region), callback, pack);
void DefaultFileSource::deleteOfflineRegion(OfflineRegion&& region, std::function<void(std::exception_ptr)> callback) {
impl->actor().invoke(&Impl::deleteRegion, std::move(region), callback);
}
void DefaultFileSource::invalidateOfflineRegion(OfflineRegion& region,
@@ -363,6 +363,10 @@ void DefaultFileSource::packDatabase(std::function<void(std::exception_ptr)> cal
impl->actor().invoke(&Impl::packDatabase, std::move(callback));
}
void DefaultFileSource::runPackDatabaseAutomatically(bool autopack) {
impl->actor().invoke(&Impl::runPackDatabaseAutomatically, autopack);
}
void DefaultFileSource::invalidateAmbientCache(std::function<void (std::exception_ptr)> callback) {
impl->actor().invoke(&Impl::invalidateAmbientCache, std::move(callback));
}

View File

@@ -154,7 +154,7 @@ void OfflineDatabase::removeExisting() {
void OfflineDatabase::removeOldCacheTable() {
assert(db);
db->exec("DROP TABLE IF EXISTS http_cache");
vacuum();
if (autopack) vacuum();
}
void OfflineDatabase::createSchema() {
@@ -705,7 +705,7 @@ std::exception_ptr OfflineDatabase::clearAmbientCache() try {
resourceQuery.run();
vacuum();
if (autopack) vacuum();
return nullptr;
} catch (...) {
@@ -884,7 +884,7 @@ OfflineDatabase::updateMetadata(const int64_t regionID, const OfflineRegionMetad
return unexpected<std::exception_ptr>(std::current_exception());
}
std::exception_ptr OfflineDatabase::deleteRegion(OfflineRegion&& region, bool pack) try {
std::exception_ptr OfflineDatabase::deleteRegion(OfflineRegion&& region) try {
{
mapbox::sqlite::Query query{ getStatement("DELETE FROM regions WHERE id = ?") };
query.bind(1, region.getID());
@@ -893,7 +893,7 @@ std::exception_ptr OfflineDatabase::deleteRegion(OfflineRegion&& region, bool pa
evict(0);
assert(db);
if (pack) vacuum();
if (autopack) vacuum();
// Ensure that the cached offlineTileCount value is recalculated.
offlineMapboxTileCount = nullopt;
@@ -1235,7 +1235,7 @@ std::exception_ptr OfflineDatabase::setMaximumAmbientCacheSize(uint64_t size) {
if (databaseSize > maximumAmbientCacheSize) {
evict(0);
vacuum();
if (autopack) vacuum();
}
return nullptr;

View File

@@ -701,7 +701,8 @@ TEST(OfflineDatabase, TEST_REQUIRES_WRITE(DeleteRegion)) {
db.putRegionResources(region2->getID(), generateResources("mapbox://tile_2", "mapbox://style_2"), status);
const size_t sizeWithTwoRegions = util::read_file(filename).size();
db.deleteRegion(std::move(*region1), false /*pack*/);
db.runPackDatabaseAutomatically(false);
db.deleteRegion(std::move(*region1));
ASSERT_EQ(1u, db.listRegions().value().size());
// Region is removed but the size of the database is the same.
@@ -711,7 +712,7 @@ TEST(OfflineDatabase, TEST_REQUIRES_WRITE(DeleteRegion)) {
// The size of the database has shrunk after pack().
const size_t sizeWithOneRegion = util::read_file(filename).size();
EXPECT_LT(sizeWithOneRegion, sizeWithTwoRegions);
db.runPackDatabaseAutomatically(true);
db.deleteRegion(std::move(*region2));
// The size of the database has shrunk right away.
const size_t sizeWithoutRegions = util::read_file(filename).size();