From 6aec95193a99601f5ad02e2ec8aa3fb62bd734a1 Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Mon, 3 Feb 2025 18:20:14 +0000 Subject: [PATCH 1/5] Converted import operation to use chunking instead of streaming Improved efficiency of import operation --- CHANGELOG.md | 6 + .../src/screens/import/stages/progress.dart | 8 +- example/pubspec.yaml | 7 +- .../internal_workers/standard/worker.dart | 551 ++++++++++-------- pubspec.yaml | 2 +- windowsApplicationInstallerSetup.iss | 2 +- 6 files changed, 306 insertions(+), 270 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 433138db..a323a97a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,12 @@ Many thanks to my sponsors, no matter how much or how little they donated. Spons # Changelog +## [10.1.1] - 2025/02/03 + +* Fixed bug where import operation fatally crashed on some iOS devices + This appears to be an [ObjectBox issue](https://github.com/objectbox/objectbox-dart/issues/654) where streaming the results of a database query caused the crash. Instead, FMTC now uses a custom chunking system to avoid streaming and also avoid loading potentially many tiles into memory. +* Massive performance/efficiency improvements to importer + ## [10.1.0] - 2025/02/02 * Added support for flutter_map v8 diff --git a/example/lib/src/screens/import/stages/progress.dart b/example/lib/src/screens/import/stages/progress.dart index 74fc24fc..b3ae1497 100644 --- a/example/lib/src/screens/import/stages/progress.dart +++ b/example/lib/src/screens/import/stages/progress.dart @@ -62,13 +62,9 @@ class _ImportProgressStageState extends State { style: Theme.of(context).textTheme.titleLarge, textAlign: TextAlign.center, ), - const SizedBox(height: 16), + const SizedBox(height: 8), const Text( - 'This could take a while.\n' - "We don't recommend leaving this screen. The import will " - 'continue, but performance could be affected.\n' - 'Closing the app will stop the import operation in an ' - 'indeterminate (but stable) state.', + "This could take a while. Don't leave this screen.", textAlign: TextAlign.center, ), ], diff --git a/example/pubspec.yaml b/example/pubspec.yaml index e1facc1d..a6f87b13 100644 --- a/example/pubspec.yaml +++ b/example/pubspec.yaml @@ -1,7 +1,7 @@ name: fmtc_demo description: The demo app for 'flutter_map_tile_caching', showcasing its functionality and use-cases. publish_to: "none" -version: 10.1.0 +version: 10.1.1 environment: sdk: ">=3.6.0 <4.0.0" @@ -32,8 +32,9 @@ dependencies: stream_transform: ^2.1.0 dependency_overrides: - flutter_map: - git: https://github.com/fleaflet/flutter_map.git + flutter_map_animations: + git: + url: https://github.com/TesteurManiak/flutter_map_animations flutter_map_tile_caching: path: ../ diff --git a/lib/src/backend/impls/objectbox/backend/internal_workers/standard/worker.dart b/lib/src/backend/impls/objectbox/backend/internal_workers/standard/worker.dart index bcbdc0db..068e145d 100644 --- a/lib/src/backend/impls/objectbox/backend/internal_workers/standard/worker.dart +++ b/lib/src/backend/impls/objectbox/backend/internal_workers/standard/worker.dart @@ -99,10 +99,15 @@ Future _worker( final stores = root.box(); final tiles = root.box(); + final modifyStoreQuery = + stores.query(ObjectBoxStore_.name.equals('')).build(); + bool hadTilesToUpdate = false; - int rootDeltaSize = 0; final tilesToRemove = []; - final storesToUpdate = {}; + + int rootDeltaSize = 0; + final storeDeltaLength = {}; + final storeDeltaSize = {}; root.runInTransaction( TxMode.write, @@ -110,55 +115,53 @@ Future _worker( final queriedStores = storesQuery.property(ObjectBoxStore_.name).find(); if (queriedStores.isEmpty) return 0; - for (int offset = 0;; offset += tilesChunkSize) { - final limit = limitTiles == null - ? tilesChunkSize - : min(tilesChunkSize, limitTiles - offset); - - final tilesChunk = (tilesQuery - ..offset = offset - ..limit = limit) - .find(); - - // For each store, remove it from the tile if requested - // For each store & if removed, update that store's stats - for (final tile in tilesChunk) { - tile.stores.removeWhere((store) { - if (!queriedStores.contains(store.name)) return false; - - storesToUpdate[store.name] = (storesToUpdate[store.name] ?? store) - ..length -= 1 - ..size -= tile.bytes.lengthInBytes; - - return true; - }); - - if (tile.stores.isNotEmpty) { - tile.stores.applyToDb(mode: PutMode.update); - hadTilesToUpdate = true; - continue; - } - - rootDeltaSize -= tile.bytes.lengthInBytes; - tilesToRemove.add(tile.id); + // For each store, remove it from the tile if requested + // For each store & if removed, update that store's stats + for (final tile in tilesQuery.chunked( + chunkSize: tilesChunkSize, + limitTiles: limitTiles, + )) { + tile.stores.removeWhere((store) { + if (!queriedStores.contains(store.name)) return false; + + storeDeltaLength[store.name] = + (storeDeltaLength[store.name] ?? 0) - 1; + storeDeltaSize[store.name] = + (storeDeltaSize[store.name] ?? 0) - tile.bytes.lengthInBytes; + + return true; + }); + + if (tile.stores.isNotEmpty) { + tile.stores.applyToDb(mode: PutMode.update); + hadTilesToUpdate = true; + continue; } - if (tilesChunk.length < tilesChunkSize) break; + rootDeltaSize -= tile.bytes.lengthInBytes; + tilesToRemove.add(tile.id); } if (!hadTilesToUpdate && tilesToRemove.isEmpty) return 0; - tilesToRemove.forEach(tiles.remove); + tiles.removeMany(tilesToRemove); updateRootStatistics( deltaLength: -tilesToRemove.length, deltaSize: rootDeltaSize, ); - stores.putMany( - storesToUpdate.values.toList(), - mode: PutMode.update, - ); + for (final MapEntry(key: storeName, value: deltaSize) + in storeDeltaSize.entries) { + modifyStoreQuery.param(ObjectBoxStore_.name).value = storeName; + + stores.put( + modifyStoreQuery.findUnique()! + ..size += deltaSize + ..length += storeDeltaLength[storeName]!, + mode: PutMode.update, + ); + } }, ); @@ -1075,9 +1078,10 @@ Future _worker( } final StoresToStates storesToStates = {}; - (switch (strategy) { + + final compiledImportStores = switch (strategy) { ImportConflictStrategy.skip => importingStoresQuery - .stream() + .find() .where( (importingStore) { final name = importingStore.name; @@ -1110,7 +1114,7 @@ Future _worker( .map((s) => s.name) .toList(), ImportConflictStrategy.rename => - importingStoresQuery.stream().map((importingStore) { + importingStoresQuery.find().map((importingStore) { final name = importingStore.name; if ((specificStoresQuery @@ -1155,7 +1159,7 @@ Future _worker( }).toList(), ImportConflictStrategy.replace || ImportConflictStrategy.merge => - importingStoresQuery.stream().map( + importingStoresQuery.find().map( (importingStore) { final name = importingStore.name; @@ -1198,242 +1202,251 @@ Future _worker( return name; }, ).toList(), - }) - .then( - (storesToImport) async { - sendRes( - id: cmd.id, - data: { - 'expectStream': true, - 'storesToStates': storesToStates, - if (storesToImport.isEmpty) 'complete': 0, + }; + + sendRes( + id: cmd.id, + data: { + 'expectStream': true, + 'storesToStates': storesToStates, + if (compiledImportStores.isEmpty) 'complete': 0, + }, + ); + if (compiledImportStores.isEmpty) { + cleanup(); + break; + } + + // At this point: + // * storesToImport should contain only the required IMPORT stores + // * root's stores should be set so that every import store has an + // equivalent with the same name + // It is important never to 'copy' from the import root to the + // in-use root + + final importingTilesQuery = (importingRoot.box().query() + ..linkMany( + ObjectBoxTile_.stores, + ObjectBoxStore_.name.oneOf(compiledImportStores), + )) + .build(); + final importingTiles = importingTilesQuery.chunked(chunkSize: 300); + + final existingStoresQuery = root + .box() + .query(ObjectBoxStore_.name.equals('')) + .build(); + final existingTilesQuery = root + .box() + .query(ObjectBoxTile_.url.equals('')) + .build(); + + final storeDeltaLength = {}; + final storeDeltaSize = {}; + + int rootDeltaLength = 0; + int rootDeltaSize = 0; + + Iterable convertToExistingStores( + Iterable importingStores, + ) => + importingStores + .where((s) => compiledImportStores.contains(s.name)) + .map( + (s) { + final e = (existingStoresQuery + ..param(ObjectBoxStore_.name).value = s.name) + .findUnique()!; + storeDeltaLength[s.name] = 0; + storeDeltaSize[s.name] = 0; + return e; }, ); - if (storesToImport.isEmpty) { - cleanup(); - return; - } - // At this point: - // * storesToImport should contain only the required IMPORT stores - // * root's stores should be set so that every import store has an - // equivalent with the same name - // It is important never to 'copy' from the import root to the - // in-use root - - final importingTilesQuery = - (importingRoot.box().query() - ..linkMany( - ObjectBoxTile_.stores, - ObjectBoxStore_.name.oneOf(storesToImport), - )) - .build(); - final importingTiles = importingTilesQuery.stream(); - - final existingStoresQuery = root - .box() - .query(ObjectBoxStore_.name.equals('')) - .build(); - final existingTilesQuery = root - .box() - .query(ObjectBoxTile_.url.equals('')) - .build(); + if (strategy == ImportConflictStrategy.replace) { + final storesQuery = root + .box() + .query(ObjectBoxStore_.name.oneOf(compiledImportStores)) + .build(); + final tilesQuery = (root.box().query() + ..linkMany( + ObjectBoxTile_.stores, + ObjectBoxStore_.name.oneOf(compiledImportStores), + )) + .build(); + + deleteTiles( + storesQuery: storesQuery, + tilesQuery: tilesQuery, + ); - final storesToUpdate = {}; + final importingStoresQuery = importingRoot + .box() + .query(ObjectBoxStore_.name.oneOf(compiledImportStores)) + .build(); + + final importingStores = importingStoresQuery.find(); + + storesQuery.remove(); + + root.box().putMany( + List.generate( + importingStores.length, + (i) => ObjectBoxStore( + name: importingStores[i].name, + maxLength: importingStores[i].maxLength, + length: importingStores[i].length, + size: importingStores[i].size, + hits: importingStores[i].hits, + misses: importingStores[i].misses, + metadataJson: importingStores[i].metadataJson, + ), + growable: false, + ), + mode: PutMode.insert, + ); - int rootDeltaLength = 0; - int rootDeltaSize = 0; + storesQuery.close(); + tilesQuery.close(); + importingStoresQuery.close(); + } - Iterable convertToExistingStores( - Iterable importingStores, - ) => - importingStores - .where((s) => storesToImport.contains(s.name)) - .map( - (s) => storesToUpdate[s.name] ??= (existingStoresQuery - ..param(ObjectBoxStore_.name).value = s.name) - .findUnique()!, + final numImportedTiles = root.runInTransaction( + TxMode.write, + () { + int sum = 0; + + for (final tile in importingTiles) { + final convertedRelatedStores = + convertToExistingStores(tile.stores); + + final existingTile = (existingTilesQuery + ..param(ObjectBoxTile_.url).value = tile.url) + .findUnique(); + + if (existingTile == null) { + root.box().put( + ObjectBoxTile( + url: tile.url, + bytes: tile.bytes, + lastModified: tile.lastModified, + )..stores.addAll(convertedRelatedStores), + mode: PutMode.insert, ); - if (strategy == ImportConflictStrategy.replace) { - final storesQuery = root - .box() - .query(ObjectBoxStore_.name.oneOf(storesToImport)) - .build(); - final tilesQuery = (root.box().query() - ..linkMany( - ObjectBoxTile_.stores, - ObjectBoxStore_.name.oneOf(storesToImport), - )) - .build(); - - deleteTiles( - storesQuery: storesQuery, - tilesQuery: tilesQuery, - ); + // No need to modify store stats, because if tile didn't already + // exist, then was not present in an existing store that needs + // changing, and all importing stores are brand new and already + // contain accurate stats. EXCEPT in merge mode - importing + // stores may not be new. + if (strategy == ImportConflictStrategy.merge) { + // No need to worry if it was brand new, we use the same + // logic, treating it as an existing related store, because + // when we created it, we made it empty. + for (final convertedRelatedStore in convertedRelatedStores) { + storeDeltaLength[convertedRelatedStore.name] = + (storeDeltaLength[convertedRelatedStore.name] ?? 0) + 1; + storeDeltaSize[convertedRelatedStore.name] = + (storeDeltaSize[convertedRelatedStore.name] ?? 0) + + tile.bytes.lengthInBytes; + } + } - final importingStoresQuery = importingRoot - .box() - .query(ObjectBoxStore_.name.oneOf(storesToImport)) - .build(); - - final importingStores = importingStoresQuery.find(); - - storesQuery.remove(); - - root.box().putMany( - List.generate( - importingStores.length, - (i) => ObjectBoxStore( - name: importingStores[i].name, - maxLength: importingStores[i].maxLength, - length: importingStores[i].length, - size: importingStores[i].size, - hits: importingStores[i].hits, - misses: importingStores[i].misses, - metadataJson: importingStores[i].metadataJson, - ), - growable: false, - ), - mode: PutMode.insert, + rootDeltaLength++; + rootDeltaSize += tile.bytes.lengthInBytes; + + sum++; + continue; + } + + final existingTileIsNewer = + existingTile.lastModified.isAfter(tile.lastModified) || + existingTile.lastModified == tile.lastModified; + + final relations = { + ...existingTile.stores, + ...convertedRelatedStores, + }; + + root.box().put( + ObjectBoxTile( + url: tile.url, + bytes: + existingTileIsNewer ? existingTile.bytes : tile.bytes, + lastModified: existingTileIsNewer + ? existingTile.lastModified + : tile.lastModified, + ) + ..stores.addAll(relations) + ..id = existingTile.id, + mode: PutMode.update, ); - storesQuery.close(); - tilesQuery.close(); - importingStoresQuery.close(); - } + if (strategy == ImportConflictStrategy.merge) { + for (final newConvertedRelatedStore in convertedRelatedStores) { + if (existingTile.stores + .map((e) => e.name) + .contains(newConvertedRelatedStore.name)) { + continue; + } - final numImportedTiles = await root - .runInTransaction( - TxMode.write, - () => importingTiles.map((importingTile) { - final convertedRelatedStores = - convertToExistingStores(importingTile.stores); + storeDeltaLength[newConvertedRelatedStore.name] = + (storeDeltaLength[newConvertedRelatedStore.name] ?? 0) + + 1; + storeDeltaSize[newConvertedRelatedStore.name] = + (storeDeltaSize[newConvertedRelatedStore.name] ?? 0) + + (existingTileIsNewer ? existingTile : tile) + .bytes + .lengthInBytes; + } + } - final existingTile = (existingTilesQuery - ..param(ObjectBoxTile_.url).value = importingTile.url) - .findUnique(); + if (existingTileIsNewer) continue; - if (existingTile == null) { - root.box().put( - ObjectBoxTile( - url: importingTile.url, - bytes: importingTile.bytes, - lastModified: importingTile.lastModified, - )..stores.addAll(convertedRelatedStores), - mode: PutMode.insert, - ); + for (final existingTileStore in existingTile.stores) { + storeDeltaSize[existingTileStore.name] = + (storeDeltaSize[existingTileStore.name] ?? 0) - + existingTile.bytes.lengthInBytes + + tile.bytes.lengthInBytes; + } - // No need to modify store stats, because if tile didn't - // already exist, then was not present in an existing - // store that needs changing, and all importing stores - // are brand new and already contain accurate stats. - // EXCEPT in merge mode - importing stores may not be - // new. - if (strategy == ImportConflictStrategy.merge) { - // No need to worry if it was brand new, we use the - // same logic, treating it as an existing related - // store, because when we created it, we made it - // empty. - for (final convertedRelatedStore - in convertedRelatedStores) { - storesToUpdate[convertedRelatedStore.name] = - (storesToUpdate[convertedRelatedStore.name] ?? - convertedRelatedStore) - ..length += 1 - ..size += importingTile.bytes.lengthInBytes; - } - } - - rootDeltaLength++; - rootDeltaSize += importingTile.bytes.lengthInBytes; - - return 1; - } - - final existingTileIsNewer = existingTile.lastModified - .isAfter(importingTile.lastModified) || - existingTile.lastModified == importingTile.lastModified; - - final relations = { - ...existingTile.stores, - ...convertedRelatedStores, - }; - - root.box().put( - ObjectBoxTile( - url: importingTile.url, - bytes: existingTileIsNewer - ? existingTile.bytes - : importingTile.bytes, - lastModified: existingTileIsNewer - ? existingTile.lastModified - : importingTile.lastModified, - )..stores.addAll(relations), - ); + rootDeltaSize += + -existingTile.bytes.lengthInBytes + tile.bytes.lengthInBytes; - if (strategy == ImportConflictStrategy.merge) { - for (final newConvertedRelatedStore - in convertedRelatedStores) { - if (existingTile.stores - .map((e) => e.name) - .contains(newConvertedRelatedStore.name)) { - continue; - } - - storesToUpdate[newConvertedRelatedStore.name] = - (storesToUpdate[newConvertedRelatedStore.name] ?? - newConvertedRelatedStore) - ..length += 1 - ..size += (existingTileIsNewer - ? existingTile - : importingTile) - .bytes - .lengthInBytes; - } - } - - if (existingTileIsNewer) return null; - - for (final existingTileStore in existingTile.stores) { - storesToUpdate[existingTileStore.name] = - (storesToUpdate[existingTileStore.name] ?? - existingTileStore) - ..size += -existingTile.bytes.lengthInBytes + - importingTile.bytes.lengthInBytes; - } - - rootDeltaSize += -existingTile.bytes.lengthInBytes + - importingTile.bytes.lengthInBytes; - - return 1; - }), - ) - .where((e) => e != null) - .length; + sum++; + } - root.box().putMany( - storesToUpdate.values.toList(), - mode: PutMode.update, - ); + return sum; + }, + ); - updateRootStatistics( - deltaLength: rootDeltaLength, - deltaSize: rootDeltaSize, - ); + for (final MapEntry(key: storeName, value: deltaSize) + in storeDeltaSize.entries) { + specificStoresQuery.param(ObjectBoxStore_.name).value = storeName; - importingTilesQuery.close(); - existingStoresQuery.close(); - existingTilesQuery.close(); - cleanup(); + root.box().put( + specificStoresQuery.findUnique()! + ..size += deltaSize + ..length += storeDeltaLength[storeName] ?? 0, + mode: PutMode.update, + ); + } - sendRes( - id: cmd.id, - data: { - 'expectStream': true, - 'complete': numImportedTiles, - }, - ); + updateRootStatistics( + deltaLength: rootDeltaLength, + deltaSize: rootDeltaSize, + ); + + importingTilesQuery.close(); + existingStoresQuery.close(); + existingTilesQuery.close(); + cleanup(); + + sendRes( + id: cmd.id, + data: { + 'expectStream': true, + 'complete': numImportedTiles, }, ); case _CmdType.listImportableStores: @@ -1501,3 +1514,23 @@ Future _worker( } }).asFuture(); } + +extension _ChunkedFind on Query { + Iterable chunked({ + required int chunkSize, + int? limitTiles, + }) sync* { + for (int offset = 0;; offset += chunkSize) { + final chunk = (this + ..offset = offset + ..limit = (limitTiles == null + ? chunkSize + : min(chunkSize, limitTiles - offset))) + .find(); + + if (chunk.isEmpty) return; + yield* chunk; + if (chunk.length < chunkSize) return; + } + } +} diff --git a/pubspec.yaml b/pubspec.yaml index 8b2d8ec4..772a4c27 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,7 +1,7 @@ name: flutter_map_tile_caching description: Plugin for 'flutter_map' providing advanced caching functionality, with ability to download map regions for offline use. -version: 10.1.0 +version: 10.1.1 repository: https://github.com/JaffaKetchup/flutter_map_tile_caching issue_tracker: https://github.com/JaffaKetchup/flutter_map_tile_caching/issues diff --git a/windowsApplicationInstallerSetup.iss b/windowsApplicationInstallerSetup.iss index 53b7ccb7..3f7c58ad 100644 --- a/windowsApplicationInstallerSetup.iss +++ b/windowsApplicationInstallerSetup.iss @@ -1,7 +1,7 @@ ; Script generated by the Inno Setup Script Wizard #define MyAppName "FMTC Demo" -#define MyAppVersion "for 10.1.0" +#define MyAppVersion "for 10.1.1" #define MyAppPublisher "JaffaKetchup Development" #define MyAppURL "https://github.com/JaffaKetchup/flutter_map_tile_caching" #define MyAppSupportURL "https://github.com/JaffaKetchup/flutter_map_tile_caching/issues" From b0801b9be29b3dbdd4c60d10beb985e030acdf82 Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Wed, 12 Feb 2025 23:07:54 +0000 Subject: [PATCH 2/5] Use database transactions in new store delta updaters Remove unneccessary check on each iteration in `chunked` --- .../internal_workers/standard/worker.dart | 57 +++++++++++-------- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/lib/src/backend/impls/objectbox/backend/internal_workers/standard/worker.dart b/lib/src/backend/impls/objectbox/backend/internal_workers/standard/worker.dart index 068e145d..77c309d7 100644 --- a/lib/src/backend/impls/objectbox/backend/internal_workers/standard/worker.dart +++ b/lib/src/backend/impls/objectbox/backend/internal_workers/standard/worker.dart @@ -97,7 +97,6 @@ Future _worker( const tilesChunkSize = 200; final stores = root.box(); - final tiles = root.box(); final modifyStoreQuery = stores.query(ObjectBoxStore_.name.equals('')).build(); @@ -144,24 +143,28 @@ Future _worker( if (!hadTilesToUpdate && tilesToRemove.isEmpty) return 0; - tiles.removeMany(tilesToRemove); + root.box().removeMany(tilesToRemove); updateRootStatistics( deltaLength: -tilesToRemove.length, deltaSize: rootDeltaSize, ); - for (final MapEntry(key: storeName, value: deltaSize) - in storeDeltaSize.entries) { - modifyStoreQuery.param(ObjectBoxStore_.name).value = storeName; - - stores.put( - modifyStoreQuery.findUnique()! - ..size += deltaSize - ..length += storeDeltaLength[storeName]!, - mode: PutMode.update, - ); - } + stores.putMany( + storeDeltaSize.entries.map( + (entry) { + final storeName = entry.key; + final deltaSize = entry.value; + final deltaLength = storeDeltaLength[storeName]!; + + modifyStoreQuery.param(ObjectBoxStore_.name).value = storeName; + return modifyStoreQuery.findUnique()! + ..size += deltaSize + ..length += deltaLength; + }, + ).toList(growable: false), + mode: PutMode.update, + ); }, ); @@ -1420,17 +1423,22 @@ Future _worker( }, ); - for (final MapEntry(key: storeName, value: deltaSize) - in storeDeltaSize.entries) { - specificStoresQuery.param(ObjectBoxStore_.name).value = storeName; - - root.box().put( - specificStoresQuery.findUnique()! - ..size += deltaSize - ..length += storeDeltaLength[storeName] ?? 0, - mode: PutMode.update, - ); - } + root.box().putMany( + storeDeltaSize.entries.map( + (entry) { + final storeName = entry.key; + final deltaSize = entry.value; + final deltaLength = storeDeltaLength[storeName] ?? 0; + + specificStoresQuery.param(ObjectBoxStore_.name).value = + storeName; + return specificStoresQuery.findUnique()! + ..size += deltaSize + ..length += deltaLength; + }, + ).toList(growable: false), + mode: PutMode.update, + ); updateRootStatistics( deltaLength: rootDeltaLength, @@ -1528,7 +1536,6 @@ extension _ChunkedFind on Query { : min(chunkSize, limitTiles - offset))) .find(); - if (chunk.isEmpty) return; yield* chunk; if (chunk.length < chunkSize) return; } From 3d7ee169f210cb78830deef8903cbcf1aafb606a Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Sun, 9 Mar 2025 17:38:32 +0000 Subject: [PATCH 3/5] Fixed memory growth issues with internal `chunked` method --- .../internal_workers/standard/worker.dart | 252 +++++++++--------- 1 file changed, 128 insertions(+), 124 deletions(-) diff --git a/lib/src/backend/impls/objectbox/backend/internal_workers/standard/worker.dart b/lib/src/backend/impls/objectbox/backend/internal_workers/standard/worker.dart index 77c309d7..db16dc91 100644 --- a/lib/src/backend/impls/objectbox/backend/internal_workers/standard/worker.dart +++ b/lib/src/backend/impls/objectbox/backend/internal_workers/standard/worker.dart @@ -114,32 +114,35 @@ Future _worker( final queriedStores = storesQuery.property(ObjectBoxStore_.name).find(); if (queriedStores.isEmpty) return 0; - // For each store, remove it from the tile if requested - // For each store & if removed, update that store's stats - for (final tile in tilesQuery.chunked( + tilesQuery.chunkedMultiTransaction( chunkSize: tilesChunkSize, limitTiles: limitTiles, - )) { - tile.stores.removeWhere((store) { - if (!queriedStores.contains(store.name)) return false; - - storeDeltaLength[store.name] = - (storeDeltaLength[store.name] ?? 0) - 1; - storeDeltaSize[store.name] = - (storeDeltaSize[store.name] ?? 0) - tile.bytes.lengthInBytes; - - return true; - }); - - if (tile.stores.isNotEmpty) { - tile.stores.applyToDb(mode: PutMode.update); - hadTilesToUpdate = true; - continue; - } + root: root, - rootDeltaSize -= tile.bytes.lengthInBytes; - tilesToRemove.add(tile.id); - } + // For each store, remove it from the tile if requested + // For each store & if removed, update that store's stats + runInTransaction: (tile) { + tile.stores.removeWhere((store) { + if (!queriedStores.contains(store.name)) return false; + + storeDeltaLength[store.name] = + (storeDeltaLength[store.name] ?? 0) - 1; + storeDeltaSize[store.name] = + (storeDeltaSize[store.name] ?? 0) - tile.bytes.lengthInBytes; + + return true; + }); + + if (tile.stores.isNotEmpty) { + tile.stores.applyToDb(mode: PutMode.update); + hadTilesToUpdate = true; + return; + } + + rootDeltaSize -= tile.bytes.lengthInBytes; + tilesToRemove.add(tile.id); + }, + ); if (!hadTilesToUpdate && tilesToRemove.isEmpty) return 0; @@ -1233,7 +1236,6 @@ Future _worker( ObjectBoxStore_.name.oneOf(compiledImportStores), )) .build(); - final importingTiles = importingTilesQuery.chunked(chunkSize: 300); final existingStoresQuery = root .box() @@ -1314,112 +1316,106 @@ Future _worker( importingStoresQuery.close(); } - final numImportedTiles = root.runInTransaction( - TxMode.write, - () { - int sum = 0; - - for (final tile in importingTiles) { - final convertedRelatedStores = - convertToExistingStores(tile.stores); - - final existingTile = (existingTilesQuery - ..param(ObjectBoxTile_.url).value = tile.url) - .findUnique(); - - if (existingTile == null) { - root.box().put( - ObjectBoxTile( - url: tile.url, - bytes: tile.bytes, - lastModified: tile.lastModified, - )..stores.addAll(convertedRelatedStores), - mode: PutMode.insert, - ); - - // No need to modify store stats, because if tile didn't already - // exist, then was not present in an existing store that needs - // changing, and all importing stores are brand new and already - // contain accurate stats. EXCEPT in merge mode - importing - // stores may not be new. - if (strategy == ImportConflictStrategy.merge) { - // No need to worry if it was brand new, we use the same - // logic, treating it as an existing related store, because - // when we created it, we made it empty. - for (final convertedRelatedStore in convertedRelatedStores) { - storeDeltaLength[convertedRelatedStore.name] = - (storeDeltaLength[convertedRelatedStore.name] ?? 0) + 1; - storeDeltaSize[convertedRelatedStore.name] = - (storeDeltaSize[convertedRelatedStore.name] ?? 0) + - tile.bytes.lengthInBytes; - } - } - - rootDeltaLength++; - rootDeltaSize += tile.bytes.lengthInBytes; - - sum++; - continue; - } - - final existingTileIsNewer = - existingTile.lastModified.isAfter(tile.lastModified) || - existingTile.lastModified == tile.lastModified; + int numImportedTiles = 0; + importingTilesQuery.chunkedMultiTransaction( + chunkSize: 300, + root: root, + runInTransaction: (tile) { + final convertedRelatedStores = convertToExistingStores(tile.stores); - final relations = { - ...existingTile.stores, - ...convertedRelatedStores, - }; + final existingTile = (existingTilesQuery + ..param(ObjectBoxTile_.url).value = tile.url) + .findUnique(); + if (existingTile == null) { root.box().put( ObjectBoxTile( url: tile.url, - bytes: - existingTileIsNewer ? existingTile.bytes : tile.bytes, - lastModified: existingTileIsNewer - ? existingTile.lastModified - : tile.lastModified, - ) - ..stores.addAll(relations) - ..id = existingTile.id, - mode: PutMode.update, + bytes: tile.bytes, + lastModified: tile.lastModified, + )..stores.addAll(convertedRelatedStores), + mode: PutMode.insert, ); + // No need to modify store stats, because if tile didn't already + // exist, then was not present in an existing store that needs + // changing, and all importing stores are brand new and already + // contain accurate stats. EXCEPT in merge mode - importing + // stores may not be new. if (strategy == ImportConflictStrategy.merge) { - for (final newConvertedRelatedStore in convertedRelatedStores) { - if (existingTile.stores - .map((e) => e.name) - .contains(newConvertedRelatedStore.name)) { - continue; - } - - storeDeltaLength[newConvertedRelatedStore.name] = - (storeDeltaLength[newConvertedRelatedStore.name] ?? 0) + - 1; - storeDeltaSize[newConvertedRelatedStore.name] = - (storeDeltaSize[newConvertedRelatedStore.name] ?? 0) + - (existingTileIsNewer ? existingTile : tile) - .bytes - .lengthInBytes; + // No need to worry if it was brand new, we use the same + // logic, treating it as an existing related store, because + // when we created it, we made it empty. + for (final convertedRelatedStore in convertedRelatedStores) { + storeDeltaLength[convertedRelatedStore.name] = + (storeDeltaLength[convertedRelatedStore.name] ?? 0) + 1; + storeDeltaSize[convertedRelatedStore.name] = + (storeDeltaSize[convertedRelatedStore.name] ?? 0) + + tile.bytes.lengthInBytes; } } - if (existingTileIsNewer) continue; + rootDeltaLength++; + rootDeltaSize += tile.bytes.lengthInBytes; + + numImportedTiles++; + return; + } - for (final existingTileStore in existingTile.stores) { - storeDeltaSize[existingTileStore.name] = - (storeDeltaSize[existingTileStore.name] ?? 0) - - existingTile.bytes.lengthInBytes + - tile.bytes.lengthInBytes; + final existingTileIsNewer = + existingTile.lastModified.isAfter(tile.lastModified) || + existingTile.lastModified == tile.lastModified; + + final relations = { + ...existingTile.stores, + ...convertedRelatedStores, + }; + + root.box().put( + ObjectBoxTile( + url: tile.url, + bytes: + existingTileIsNewer ? existingTile.bytes : tile.bytes, + lastModified: existingTileIsNewer + ? existingTile.lastModified + : tile.lastModified, + ) + ..stores.addAll(relations) + ..id = existingTile.id, + mode: PutMode.update, + ); + + if (strategy == ImportConflictStrategy.merge) { + for (final newConvertedRelatedStore in convertedRelatedStores) { + if (existingTile.stores + .map((e) => e.name) + .contains(newConvertedRelatedStore.name)) { + continue; + } + + storeDeltaLength[newConvertedRelatedStore.name] = + (storeDeltaLength[newConvertedRelatedStore.name] ?? 0) + 1; + storeDeltaSize[newConvertedRelatedStore.name] = + (storeDeltaSize[newConvertedRelatedStore.name] ?? 0) + + (existingTileIsNewer ? existingTile : tile) + .bytes + .lengthInBytes; } + } - rootDeltaSize += - -existingTile.bytes.lengthInBytes + tile.bytes.lengthInBytes; + if (existingTileIsNewer) return; - sum++; + for (final existingTileStore in existingTile.stores) { + storeDeltaSize[existingTileStore.name] = + (storeDeltaSize[existingTileStore.name] ?? 0) - + existingTile.bytes.lengthInBytes + + tile.bytes.lengthInBytes; } - return sum; + rootDeltaSize += + -existingTile.bytes.lengthInBytes + tile.bytes.lengthInBytes; + + numImportedTiles++; }, ); @@ -1524,20 +1520,28 @@ Future _worker( } extension _ChunkedFind on Query { - Iterable chunked({ + void chunkedMultiTransaction({ + required Store root, + required void Function(T e) runInTransaction, required int chunkSize, int? limitTiles, - }) sync* { + TxMode transactionMode = TxMode.write, + }) { for (int offset = 0;; offset += chunkSize) { - final chunk = (this - ..offset = offset - ..limit = (limitTiles == null - ? chunkSize - : min(chunkSize, limitTiles - offset))) - .find(); - - yield* chunk; - if (chunk.length < chunkSize) return; + final exit = root.runInTransaction( + transactionMode, + () { + final chunk = (this + ..offset = offset + ..limit = (limitTiles == null + ? chunkSize + : min(chunkSize, limitTiles - offset))) + .find() + ..forEach(runInTransaction); + return chunk.length < chunkSize; + }, + ); + if (exit) return; } } } From 1c6c62389a7b4eb250e25e6346d75c29350269bd Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Sun, 9 Mar 2025 18:14:23 +0000 Subject: [PATCH 4/5] Remove streaming from database from export operation --- .../impls/objectbox/backend/backend.dart | 3 +- .../internal_workers/standard/worker.dart | 227 ++++++++---------- 2 files changed, 102 insertions(+), 128 deletions(-) diff --git a/lib/src/backend/impls/objectbox/backend/backend.dart b/lib/src/backend/impls/objectbox/backend/backend.dart index 5bb750cc..2851d559 100644 --- a/lib/src/backend/impls/objectbox/backend/backend.dart +++ b/lib/src/backend/impls/objectbox/backend/backend.dart @@ -9,9 +9,8 @@ import 'dart:isolate'; import 'dart:math'; import 'package:collection/collection.dart'; -import 'package:flutter/material.dart'; +import 'package:flutter/foundation.dart'; import 'package:flutter/services.dart'; -import 'package:meta/meta.dart'; import 'package:path/path.dart' as path; import 'package:path_provider/path_provider.dart'; diff --git a/lib/src/backend/impls/objectbox/backend/internal_workers/standard/worker.dart b/lib/src/backend/impls/objectbox/backend/internal_workers/standard/worker.dart index db16dc91..3737946d 100644 --- a/lib/src/backend/impls/objectbox/backend/internal_workers/standard/worker.dart +++ b/lib/src/backend/impls/objectbox/backend/internal_workers/standard/worker.dart @@ -900,142 +900,117 @@ Future _worker( rethrow; } - final storesQuery = root - .box() - .query(ObjectBoxStore_.name.oneOf(storeNames)) - .build(); + try { + final storesQuery = root + .box() + .query(ObjectBoxStore_.name.oneOf(storeNames)) + .build(); + final tilesQuery = (root.box().query() + ..linkMany( + ObjectBoxTile_.stores, + ObjectBoxStore_.name.oneOf(storeNames), + )) + .build(); - final tilesQuery = (root.box().query() - ..linkMany( - ObjectBoxTile_.stores, - ObjectBoxStore_.name.oneOf(storeNames), - )) - .build(); + // Copy all stores to external root + // Then, to make sure relations work 100%, we go through the stores + // just copied to the external root and add them to the map below + final storesToExport = storesQuery.find(); + if (!listEquals( + storesToExport.map((s) => s.name).toList(growable: false), + storeNames, + )) { + throw ArgumentError( + 'Specified stores did not match the resolved existing stores', + 'storeNames', + ); + } + final storesObjectsForRelations = + Map.fromEntries( + (exportingRoot.box() + ..putMany( + storesQuery + .find() + .map( + (store) => ObjectBoxStore( + name: store.name, + maxLength: store.maxLength, + length: store.length, + size: store.size, + hits: store.hits, + misses: store.misses, + metadataJson: store.metadataJson, + ), + ) + .toList(growable: false), + mode: PutMode.insert, + )) + .getAll() + .map((s) => MapEntry(s.name, s)), + ); - final storesObjectsForRelations = {}; + // Copy all tiles to external root + int numExportedTiles = 0; + tilesQuery.chunkedMultiTransaction( + chunkSize: 300, + root: root, + runInTransaction: (tile) { + exportingRoot.box().put( + ObjectBoxTile( + url: tile.url, + bytes: tile.bytes, + lastModified: tile.lastModified, + )..stores.addAll( + tile.stores + .map((s) => storesObjectsForRelations[s.name]) + .nonNulls, + ), + mode: PutMode.insert, + ); + numExportedTiles++; + }, + ); - final exportingStores = root.runInTransaction( - TxMode.read, - storesQuery.stream, - ); + storesQuery.close(); + tilesQuery.close(); + exportingRoot.close(); - exportingRoot - .runInTransaction( - TxMode.write, - () => exportingStores.map( - (exportingStore) { - exportingRoot.box().put( - storesObjectsForRelations[exportingStore.name] = - ObjectBoxStore( - name: exportingStore.name, - maxLength: exportingStore.maxLength, - length: exportingStore.length, - size: exportingStore.size, - hits: exportingStore.hits, - misses: exportingStore.misses, - metadataJson: exportingStore.metadataJson, - ), - mode: PutMode.insert, - ); - }, - ), - ) - .length - .then( - (numExportedStores) { - if (numExportedStores == 0) throw StateError('Unpossible'); - - final exportingTiles = root.runInTransaction( - TxMode.read, - tilesQuery.stream, - ); + final dbFile = File(path.join(workingDir.absolute.path, 'data.mdb')); - exportingRoot - .runInTransaction( - TxMode.write, - () => exportingTiles.map( - (exportingTile) { - exportingRoot.box().put( - ObjectBoxTile( - url: exportingTile.url, - bytes: exportingTile.bytes, - lastModified: exportingTile.lastModified, - )..stores.addAll( - exportingTile.stores - .map( - (s) => storesObjectsForRelations[s.name], - ) - .nonNulls, - ), - mode: PutMode.insert, - ); - }, - ), - ) - .length - .then( - (numExportedTiles) { - if (numExportedTiles == 0) { - throw ArgumentError( - 'Specified stores must include at least one tile total', - 'storeNames', - ); - } + final ram = dbFile.openSync(mode: FileMode.writeOnlyAppend); + try { + ram + ..writeFromSync(List.filled(4, 255)) + ..writeStringSync('ObjectBox') // Backend identifier + ..writeByteSync(255) + ..writeByteSync(255) + ..writeStringSync('FMTC'); // Signature + } finally { + ram.closeSync(); + } - storesQuery.close(); - tilesQuery.close(); - exportingRoot.close(); - - final dbFile = - File(path.join(workingDir.absolute.path, 'data.mdb')); - - final ram = dbFile.openSync(mode: FileMode.writeOnlyAppend); - try { - ram - ..writeFromSync(List.filled(4, 255)) - ..writeStringSync('ObjectBox') // Backend identifier - ..writeByteSync(255) - ..writeByteSync(255) - ..writeStringSync('FMTC'); // Signature - } finally { - ram.closeSync(); - } + try { + dbFile.renameSync(outputPath); + } on FileSystemException { + dbFile.copySync(outputPath); + } finally { + workingDir.deleteSync(recursive: true); + } - try { - dbFile.renameSync(outputPath); - } on FileSystemException { - dbFile.copySync(outputPath); - } finally { - workingDir.deleteSync(recursive: true); - } + sendRes( + id: cmd.id, + data: {'numExportedTiles': numExportedTiles}, + ); - sendRes( - id: cmd.id, - data: {'numExportedTiles': numExportedTiles}, - ); - }, - ).catchError((error, stackTrace) { - exportingRoot.close(); - try { - workingDir.deleteSync(recursive: true); - // If the working dir didn't exist, that's fine - // We don't want to spend time checking if exists, as it likely - // does - // ignore: empty_catches - } on FileSystemException {} - Error.throwWithStackTrace(error, stackTrace); - }); - }, - ).catchError((error, stackTrace) { + // We don't care what type, we always need to clean up and rethrow + // ignore: avoid_catches_without_on_clauses + } catch (e) { exportingRoot.close(); - try { + if (workingDir.existsSync()) { workingDir.deleteSync(recursive: true); - // If the working dir didn't exist, that's fine - // We don't want to spend time checking if exists, as it likely does - // ignore: empty_catches - } on FileSystemException {} - Error.throwWithStackTrace(error, stackTrace); - }); + } + rethrow; + } case _CmdType.importStores: final importPath = cmd.args['path']! as String; final strategy = cmd.args['strategy'] as ImportConflictStrategy; From a6e91d3b2ecaecdb01eb9d554b72ad38d88a2fef Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Sun, 9 Mar 2025 18:25:02 +0000 Subject: [PATCH 5/5] Updated dependencies & changelog --- CHANGELOG.md | 2 +- example/pubspec.yaml | 25 +++++++++++-------------- pubspec.yaml | 16 ++++++++-------- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a323a97a..8d4d04e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ Many thanks to my sponsors, no matter how much or how little they donated. Spons * Fixed bug where import operation fatally crashed on some iOS devices This appears to be an [ObjectBox issue](https://github.com/objectbox/objectbox-dart/issues/654) where streaming the results of a database query caused the crash. Instead, FMTC now uses a custom chunking system to avoid streaming and also avoid loading potentially many tiles into memory. -* Massive performance/efficiency improvements to importer +* Improved performance of import operation ## [10.1.0] - 2025/02/02 diff --git a/example/pubspec.yaml b/example/pubspec.yaml index a6f87b13..9d00b756 100644 --- a/example/pubspec.yaml +++ b/example/pubspec.yaml @@ -8,33 +8,30 @@ environment: flutter: ">=3.27.0" dependencies: - async: ^2.12.0 + async: ^2.13.0 auto_size_text: ^3.0.0 badges: ^3.1.2 - collection: ^1.18.0 - file_picker: 8.1.4 # Compatible with 3.27! + collection: ^1.19.1 + file_picker: ^9.0.2 flutter: sdk: flutter - flutter_map: - flutter_map_animations: ^0.8.0 + flutter_map: ^8.1.1 + flutter_map_animations: ^0.9.0 flutter_map_tile_caching: - flutter_slidable: ^3.1.2 + flutter_slidable: ^4.0.0 google_fonts: ^6.2.1 gpx: ^2.3.0 - http: ^1.2.2 - intl: ^0.19.0 + http: ^1.3.0 + intl: ^0.20.2 latlong2: ^0.9.1 path: ^1.9.1 path_provider: ^2.1.5 provider: ^6.1.2 - share_plus: ^10.1.3 - shared_preferences: ^2.3.3 - stream_transform: ^2.1.0 + share_plus: ^10.1.4 + shared_preferences: ^2.5.2 + stream_transform: ^2.1.1 dependency_overrides: - flutter_map_animations: - git: - url: https://github.com/TesteurManiak/flutter_map_animations flutter_map_tile_caching: path: ../ diff --git a/pubspec.yaml b/pubspec.yaml index 772a4c27..657345f0 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -33,19 +33,19 @@ dependencies: flat_buffers: ^23.5.26 flutter: sdk: flutter - flutter_map: ^8.0.0 + flutter_map: ^8.1.1 http: ^1.2.2 latlong2: ^0.9.1 meta: ^1.15.0 - objectbox: ^4.0.3 - objectbox_flutter_libs: ^4.0.3 - path: ^1.9.0 - path_provider: ^2.1.4 + objectbox: ^4.1.0 + objectbox_flutter_libs: ^4.1.0 + path: ^1.9.1 + path_provider: ^2.1.5 dev_dependencies: - build_runner: ^2.4.14 - objectbox_generator: ^4.0.3 - test: ^1.25.14 + build_runner: ^2.4.15 + objectbox_generator: ^4.1.0 + test: ^1.25.15 flutter: null