Changeset 18711 in josm for trunk/src

Timestamp:

2023-04-24T19:55:08+02:00 (12 months ago)

Author:

taylor.smock

Message:

Fix #22726: DownloadOsmChange task should not always get the history for each unknown object

In many cases, the latest version of an object will also be the version we will
get when iterating through all of its history. This will always be the case for
objects with only one version. As such, in most cases, getting the latest version
of an object will significantly reduce the number of individual history calls.

In a test, the amount of time taken to load an OSC went from 105 seconds to 14
seconds. This is an 86% improvement in terms of time taken.

File:

• trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadOsmChangeTask.java (modified) (8 diffs)

trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadOsmChangeTask.java

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
+import java.io.IOException;
 import java.time.Instant;
 import java.util.Arrays;
@@ -13 +14 @@
 import java.util.concurrent.Future;
 import java.util.concurrent.RejectedExecutionException;
+import java.util.function.Function;
 import java.util.regex.Matcher;
+import java.util.stream.Collectors;
 
 import org.openstreetmap.josm.data.Bounds;
@@ -24 +27 @@
 import org.openstreetmap.josm.data.osm.PrimitiveData;
 import org.openstreetmap.josm.data.osm.PrimitiveId;
+import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationData;
+import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.data.osm.WayData;
 import org.openstreetmap.josm.data.osm.history.History;
@@ -37 +42 @@
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
 import org.openstreetmap.josm.io.Compression;
+import org.openstreetmap.josm.io.MultiFetchServerObjectReader;
 import org.openstreetmap.josm.io.OsmApi;
 import org.openstreetmap.josm.io.OsmServerLocationReader;
@@ -43 +49 @@
 import org.openstreetmap.josm.io.UrlPatterns.OsmChangeUrlPattern;
 import org.openstreetmap.josm.tools.Logging;
+import org.xml.sax.SAXException;
 
 /**
@@ -105 +112 @@
 
         @Override
+        public void realRun() throws IOException, SAXException, OsmTransferException {
+            super.realRun();
+            final Map<OsmPrimitive, Instant> toLoadNext = new HashMap<>();
+            final Map<OsmPrimitive, Instant> toLoad = getToLoad(dataSet);
+            while (!toLoad.isEmpty()) {
+                loadLastVersions(toLoad, toLoadNext);
+                toLoad.putAll(toLoadNext);
+                toLoadNext.clear();
+            }
+        }
+
+        /**
+         * This gets the last versions of references primitives. This may enough for many of the primitives.
+         * @param toLoad The primitives to load
+         * @param toLoadNext The primitives to load next (filled by this method)
+         */
+        private void loadLastVersions(Map<OsmPrimitive, Instant> toLoad, Map<OsmPrimitive, Instant> toLoadNext) throws OsmTransferException {
+            final Map<OsmPrimitiveType, Map<OsmPrimitive, Instant>> typeMap = toLoad.entrySet().stream()
+                    .collect(Collectors.groupingBy(entry -> entry.getKey().getType(), Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)));
+            final Map<PrimitiveId, OsmPrimitive> idMap = toLoad.keySet().stream()
+                    .collect(Collectors.toMap(OsmPrimitive::getPrimitiveId, Function.identity()));
+            for (OsmPrimitiveType type : Arrays.asList(OsmPrimitiveType.NODE, OsmPrimitiveType.WAY, OsmPrimitiveType.RELATION)) {
+                if (!typeMap.containsKey(type)) {
+                    continue;
+                }
+                final MultiFetchServerObjectReader reader = MultiFetchServerObjectReader.create();
+                typeMap.get(type).forEach((primitive, instant) -> reader.append(primitive));
+                final DataSet ds = reader.parseOsm(this.progressMonitor.createSubTaskMonitor(1, false));
+                switch (type) {
+                    case NODE:
+                        for (Node node : ds.getNodes()) {
+                            Node original = (Node) idMap.get(node.getPrimitiveId());
+                            if (original != null && toLoad.get(original).isAfter(node.getInstant())) {
+                                original.load(node.save());
+                            }
+                            toLoad.remove(original);
+                        }
+                        break;
+                    case WAY:
+                        for (Way way : ds.getWays()) {
+                            Way original = (Way) idMap.get(way.getPrimitiveId());
+                            if (original != null && toLoad.get(original).isAfter(way.getInstant())) {
+                                Instant date = toLoad.get(original);
+                                original.load(way.save());
+                                for (Long nodeId : way.getNodeIds()) {
+                                    if (way.getDataSet().getPrimitiveById(nodeId, OsmPrimitiveType.NODE) == null) {
+                                        Node n = new Node(nodeId);
+                                        way.getDataSet().addPrimitive(n);
+                                        toLoadNext.put(n, date);
+                                    }
+                                }
+                            }
+                            toLoad.remove(original);
+                        }
+                        break;
+                    case RELATION:
+                        for (Relation relation : ds.getRelations()) {
+                            Relation original = (Relation) idMap.get(relation.getPrimitiveId());
+                            if (original != null && toLoad.get(original).isAfter(relation.getInstant())) {
+                                original.load(relation.save());
+                            }
+                            toLoad.remove(relation);
+                        }
+                        break;
+                    default:
+                        throw new IllegalStateException("Only Node, Ways, and Relations should be returned by the API");
+                }
+            }
+        }
+
+        @Override
         protected void finish() {
             super.finish();
@@ -112 +190 @@
                 // A changeset does not contain all referred primitives, this is the map of incomplete ones
                 // For each incomplete primitive, we'll have to get its state at date it was referred
-                Map<OsmPrimitive, Instant> toLoad = new HashMap<>();
-                for (OsmPrimitive p : downloadedData.allNonDeletedPrimitives()) {
-                    if (p.isIncomplete()) {
-                        Instant timestamp = p.getReferrers().stream()
-                                .filter(ref -> !ref.isTimestampEmpty())
-                                .findFirst()
-                                .map(AbstractPrimitive::getInstant)
-                                .orElse(null);
-                        toLoad.put(p, timestamp);
-                    }
-                }
+                Map<OsmPrimitive, Instant> toLoad = getToLoad(downloadedData);
                 if (isCanceled()) return;
                 // Let's load all required history
@@ -131 +199 @@
             }
         }
+    }
+
+    /**
+     * Get the primitives to load more information
+     * @param ds The dataset to look for incomplete primitives from
+     * @return The objects that still need to be loaded
+     */
+    private static Map<OsmPrimitive, Instant> getToLoad(DataSet ds) {
+        Map<OsmPrimitive, Instant> toLoad = new HashMap<>();
+        for (OsmPrimitive p : ds.allNonDeletedPrimitives()) {
+            if (p.isIncomplete()) {
+                Instant timestamp = p.getReferrers().stream()
+                        .filter(ref -> !ref.isTimestampEmpty())
+                        .findFirst()
+                        .map(AbstractPrimitive::getInstant)
+                        .orElse(null);
+                toLoad.put(p, timestamp);
+            }
+        }
+        return toLoad;
     }