replace unnecessary Map with a List (#352)

* replace unnecessary Map with a List see #332 (comment) for more background * append changelog * add some missing javadoc (copy-paste error)
GIScience · Feb 16, 2021 · 7454255 · 7454255
1 parent 48c40e2
commit 7454255
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,10 @@ import org.heigit.ohsome.oshdb.api.db.OSHDBDatabase;
 import org.heigit.ohsome.oshdb.api.db.OSHDBH2;
 ```
 
+### performance improvements
+
+* replace an unnecessarily used Map with a more lightweight implementation using a List. ([#352])
+
 ### other changes
 
 * integrate [ohsome-filter](https://gitlab.gistools.geog.uni-heidelberg.de/giscience/big-data/ohsome/libs/ohsome-filter) module fully into this repository, renaming it to `oshdb-filter`. ([#306])
@@ -29,6 +33,7 @@ import org.heigit.ohsome.oshdb.api.db.OSHDBH2;
 [#306]: https://github.com/GIScience/oshdb/pull/306
 [#327]: https://github.com/GIScience/oshdb/issues/327
 [#338]: https://github.com/GIScience/oshdb/issues/338
+[#352]: https://github.com/GIScience/oshdb/pull/352
 
 
 ## 0.6.3

diff --git a/oshdb-util/src/main/java/org/heigit/ohsome/oshdb/util/celliterator/CellIterator.java b/oshdb-util/src/main/java/org/heigit/ohsome/oshdb/util/celliterator/CellIterator.java
@@ -1,5 +1,6 @@
 package org.heigit.ohsome.oshdb.util.celliterator;
 
+import com.google.common.collect.Lists;
 import com.google.common.collect.Streams;
 import java.io.Serializable;
 import java.util.ArrayList;
@@ -310,14 +311,13 @@ public Stream<IterateByTimestampEntry> iterateByTimestamps(GridOSHEntity cell) {
         }
       }
 
-      SortedMap<OSHDBTimestamp, OSMEntity> osmEntityByTimestamps =
-          getVersionsByTimestamps(oshEntity, new ArrayList<>(queryTs.keySet()));
+      List<OSHDBTimestamp> timestamps = new ArrayList<>(queryTs.keySet());
+      List<OSMEntity> osmEntityAtTimestamps = getVersionsByTimestamps(oshEntity, timestamps);
 
       List<IterateByTimestampEntry> results = new LinkedList<>();
-      osmEntityLoop: for (Map.Entry<OSHDBTimestamp, OSMEntity> entity :
-          osmEntityByTimestamps.entrySet()) {
-        OSHDBTimestamp timestamp = entity.getKey();
-        OSMEntity osmEntity = entity.getValue();
+      osmEntityLoop: for (int j = 0; j < osmEntityAtTimestamps.size(); j++) {
+        OSHDBTimestamp timestamp = timestamps.get(j);
+        OSMEntity osmEntity = osmEntityAtTimestamps.get(j);
 
         if (!osmEntity.isVisible()) {
           // skip because this entity is deleted at this timestamp
@@ -584,17 +584,16 @@ public Stream<IterateAllEntry> iterateByContribution(GridOSHEntity cell) {
         return Stream.empty();
       }
 
-      SortedMap<OSHDBTimestamp, OSMEntity> osmEntityByTimestamps =
-          getVersionsByTimestamps(oshEntity, modTs);
+      List<OSMEntity> osmEntityAtTimestamps = getVersionsByTimestamps(oshEntity, modTs);
 
       List<IterateAllEntry> results = new LinkedList<>();
 
       IterateAllEntry prev = null;
 
       osmEntityLoop:
-      for (Map.Entry<OSHDBTimestamp, OSMEntity> entity : osmEntityByTimestamps.entrySet()) {
-        OSHDBTimestamp timestamp = entity.getKey();
-        OSMEntity osmEntity = entity.getValue();
+      for (int j = 0; j < osmEntityAtTimestamps.size(); j++) {
+        OSHDBTimestamp timestamp = modTs.get(j);
+        OSMEntity osmEntity = osmEntityAtTimestamps.get(j);
 
         // prev = results.size() > 0 ? results.get(results.size()-1) : null;
         // todo: replace with variable outside of osmEntitiyLoop (than we can also get rid of
@@ -805,22 +804,23 @@ public Stream<IterateAllEntry> iterateByContribution(GridOSHEntity cell) {
   }
 
   /**
-   * Returns the corresponding OSMEntity "versions" of the given OSHEntity which are valid at the
-   * given timestamps.
+   * Returns a list of corresponding osm OSMEntity "versions" of the given OSHEntity which are
+   * valid at the given timestamps. The resulting list will contain the same number of entries
+   * as the supplied list of timestamps.
    */
-  private static SortedMap<OSHDBTimestamp, OSMEntity> getVersionsByTimestamps(
+  private static List<OSMEntity> getVersionsByTimestamps(
       OSHEntity osh, List<OSHDBTimestamp> timestamps) {
-    SortedMap<OSHDBTimestamp, OSMEntity> result = new TreeMap<>();
+    List<OSMEntity> result = new ArrayList<>(timestamps.size());
 
     int i = timestamps.size() - 1;
     Iterator<? extends OSMEntity> itr = osh.getVersions().iterator();
     while (itr.hasNext() && i >= 0) {
       OSMEntity osm = itr.next();
       while (i >= 0 && osm.getTimestamp().compareTo(timestamps.get(i)) <= 0) {
-        result.put(timestamps.get(i), osm);
+        result.add(osm);
         i--;
       }
     }
-    return result;
+    return Lists.reverse(result);
   }
 }
diff --git a/oshdb/src/main/java/org/heigit/ohsome/oshdb/osh/OSHEntities.java b/oshdb/src/main/java/org/heigit/ohsome/oshdb/osh/OSHEntities.java
@@ -29,12 +29,14 @@ public static <T extends OSMEntity> List<T> toList(Iterable<T> versions) {
 
   /**
    * Collects all versions of an OSH entity from an iterable ({@link OSHEntity#getVersions()})
-   * into a list.
+   * into a list after applying a transformation function.
    *
    * @param versions the versions of an OSH entity, as returned from {@link OSHEntity#getVersions()}
+   * @param transformer a function which is called for each version
    * @param <T> the type of the OSM entities: {@link org.heigit.ohsome.oshdb.osm.OSMNode},
    *            {@link org.heigit.ohsome.oshdb.osm.OSMWay} or
    *            {@link org.heigit.ohsome.oshdb.osm.OSMRelation}
+   * @param <R> the type of the returned list's items
    * @return all versions of the OSH entity as a list, with the most recent version first.
    */
   public static <T extends OSMEntity, R> List<R> toList(