From 06cd3b2c0e6d7e0a8eb6f495f932068120cd3dab Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Thu, 24 Apr 2014 09:15:58 +0200 Subject: Revert "fix #3759: OOM while loading caches" This reverts commit 2f3e8ddd18303123a1b81995357f27461fa1d586, as the symptom of the out of memory error might not be its cause, and it slows down the loading of lists. --- main/src/cgeo/geocaching/DataStore.java | 114 +++++++++++++------------- main/src/cgeo/geocaching/utils/MiscUtils.java | 40 --------- 2 files changed, 56 insertions(+), 98 deletions(-) delete mode 100644 main/src/cgeo/geocaching/utils/MiscUtils.java (limited to 'main') diff --git a/main/src/cgeo/geocaching/DataStore.java b/main/src/cgeo/geocaching/DataStore.java index 623301b..302bfc3 100644 --- a/main/src/cgeo/geocaching/DataStore.java +++ b/main/src/cgeo/geocaching/DataStore.java @@ -20,7 +20,6 @@ import cgeo.geocaching.settings.Settings; import cgeo.geocaching.ui.dialog.Dialogs; import cgeo.geocaching.utils.FileUtils; import cgeo.geocaching.utils.Log; -import cgeo.geocaching.utils.MiscUtils; import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.collections4.MapUtils; @@ -1314,7 +1313,7 @@ public class DataStore { */ private static void removeOutdatedWaypointsOfCache(final @NonNull Geocache cache, final @NonNull Collection remainingWaypointIds) { final String idList = StringUtils.join(remainingWaypointIds, ','); - database.delete(dbTableWaypoints, "geocode = ? AND _id NOT in (" + idList + ")", new String[]{cache.getGeocode()}); + database.delete(dbTableWaypoints, "geocode = ? AND _id NOT in (" + idList + ")", new String[] { cache.getGeocode() }); } /** @@ -1559,8 +1558,8 @@ public class DataStore { return new HashSet(); } - final Set result = new HashSet(); - final List remaining = new LinkedList(geocodes); + Set result = new HashSet(); + Set remaining = new HashSet(geocodes); if (loadFlags.contains(LoadFlag.LOAD_CACHE_BEFORE)) { for (String geocode : new HashSet(remaining)) { @@ -1580,7 +1579,7 @@ public class DataStore { loadFlags.contains(LoadFlag.LOAD_INVENTORY) || loadFlags.contains(LoadFlag.LOAD_OFFLINE_LOG)) { - final Collection cachesFromDB = loadCachesFromGeocodes(remaining, loadFlags); + final Set cachesFromDB = loadCachesFromGeocodes(remaining, loadFlags); result.addAll(cachesFromDB); for (final Geocache cache : cachesFromDB) { remaining.remove(cache.getGeocode()); @@ -1606,11 +1605,15 @@ public class DataStore { /** * Load caches. * - * @param allGeocodes + * @param geocodes * @param loadFlags * @return Set of loaded caches. Never null. */ - private static Collection loadCachesFromGeocodes(final List allGeocodes, final EnumSet loadFlags) { + private static Set loadCachesFromGeocodes(final Set geocodes, final EnumSet loadFlags) { + if (CollectionUtils.isEmpty(geocodes)) { + return Collections.emptySet(); + } + // do not log the entire collection of geo codes to the debug log. This can be more than 100 KB of text for large lists! init(); @@ -1625,73 +1628,68 @@ public class DataStore { } query.append(" WHERE ").append(dbTableCaches).append('.'); + query.append(DataStore.whereGeocodeIn(geocodes)); - final String queryBegin = query.toString(); - final List result = new ArrayList(allGeocodes.size()); - - for (List geocodes: MiscUtils.buffer(allGeocodes, 50)) { - final Cursor cursor = database.rawQuery(queryBegin + DataStore.whereGeocodeIn(geocodes), null); - try { - final Set caches = new HashSet(); - int logIndex = -1; + Cursor cursor = database.rawQuery(query.toString(), null); + try { + final Set caches = new HashSet(); + int logIndex = -1; - while (cursor.moveToNext()) { - final Geocache cache = DataStore.createCacheFromDatabaseContent(cursor); + while (cursor.moveToNext()) { + Geocache cache = DataStore.createCacheFromDatabaseContent(cursor); - if (loadFlags.contains(LoadFlag.LOAD_ATTRIBUTES)) { - cache.setAttributes(loadAttributes(cache.getGeocode())); - } + if (loadFlags.contains(LoadFlag.LOAD_ATTRIBUTES)) { + cache.setAttributes(loadAttributes(cache.getGeocode())); + } - if (loadFlags.contains(LoadFlag.LOAD_WAYPOINTS)) { - final List waypoints = loadWaypoints(cache.getGeocode()); - if (CollectionUtils.isNotEmpty(waypoints)) { - cache.setWaypoints(waypoints, false); - } + if (loadFlags.contains(LoadFlag.LOAD_WAYPOINTS)) { + final List waypoints = loadWaypoints(cache.getGeocode()); + if (CollectionUtils.isNotEmpty(waypoints)) { + cache.setWaypoints(waypoints, false); } + } - if (loadFlags.contains(LoadFlag.LOAD_SPOILERS)) { - final List spoilers = loadSpoilers(cache.getGeocode()); - cache.setSpoilers(spoilers); - } + if (loadFlags.contains(LoadFlag.LOAD_SPOILERS)) { + final List spoilers = loadSpoilers(cache.getGeocode()); + cache.setSpoilers(spoilers); + } - if (loadFlags.contains(LoadFlag.LOAD_LOGS)) { - cache.setLogs(loadLogs(cache.getGeocode())); - final Map logCounts = loadLogCounts(cache.getGeocode()); - if (MapUtils.isNotEmpty(logCounts)) { - cache.getLogCounts().clear(); - cache.getLogCounts().putAll(logCounts); - } + if (loadFlags.contains(LoadFlag.LOAD_LOGS)) { + cache.setLogs(loadLogs(cache.getGeocode())); + final Map logCounts = loadLogCounts(cache.getGeocode()); + if (MapUtils.isNotEmpty(logCounts)) { + cache.getLogCounts().clear(); + cache.getLogCounts().putAll(logCounts); } + } - if (loadFlags.contains(LoadFlag.LOAD_INVENTORY)) { - final List inventory = loadInventory(cache.getGeocode()); - if (CollectionUtils.isNotEmpty(inventory)) { - if (cache.getInventory() == null) { - cache.setInventory(new ArrayList()); - } else { - cache.getInventory().clear(); - } - cache.getInventory().addAll(inventory); + if (loadFlags.contains(LoadFlag.LOAD_INVENTORY)) { + final List inventory = loadInventory(cache.getGeocode()); + if (CollectionUtils.isNotEmpty(inventory)) { + if (cache.getInventory() == null) { + cache.setInventory(new ArrayList()); + } else { + cache.getInventory().clear(); } + cache.getInventory().addAll(inventory); } + } - if (loadFlags.contains(LoadFlag.LOAD_OFFLINE_LOG)) { - if (logIndex < 0) { - logIndex = cursor.getColumnIndex("log"); - } - cache.setLogOffline(!cursor.isNull(logIndex)); + if (loadFlags.contains(LoadFlag.LOAD_OFFLINE_LOG)) { + if (logIndex < 0) { + logIndex = cursor.getColumnIndex("log"); } - cache.addStorageLocation(StorageLocation.DATABASE); - cacheCache.putCacheInCache(cache); - - caches.add(cache); + cache.setLogOffline(!cursor.isNull(logIndex)); } - result.addAll(caches); - } finally { - cursor.close(); + cache.addStorageLocation(StorageLocation.DATABASE); + cacheCache.putCacheInCache(cache); + + caches.add(cache); } + return caches; + } finally { + cursor.close(); } - return result; } diff --git a/main/src/cgeo/geocaching/utils/MiscUtils.java b/main/src/cgeo/geocaching/utils/MiscUtils.java deleted file mode 100644 index 122c4eb..0000000 --- a/main/src/cgeo/geocaching/utils/MiscUtils.java +++ /dev/null @@ -1,40 +0,0 @@ -package cgeo.geocaching.utils; - -import org.apache.commons.collections4.iterators.IteratorIterable; -import org.apache.commons.lang3.NotImplementedException; - -import java.util.Iterator; -import java.util.List; - -final public class MiscUtils { - - private MiscUtils() {} // Do not instantiate - - public static Iterable> buffer(final List original, final int n) { - if (n <= 0) { - throw new IllegalArgumentException("buffer size must be positive"); - } - return new IteratorIterable>(new Iterator>() { - final int size = original.size(); - int next = 0; - - @Override - public boolean hasNext() { - return next < size; - } - - @Override - public List next() { - final List result = original.subList(next, Math.min(next + n, size)); - next += n; - return result; - } - - @Override - public void remove() { - throw new NotImplementedException("remove"); - } - }); - } - -} -- cgit v1.1