Various performance related fixes

Signed-off-by: Taylor Smock <[email protected]>
JOSM · Apr 27, 2022 · 791ef63 · 791ef63
1 parent 666627c
commit 791ef63
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 19 deletions.
diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapillary/data/mapillary/MapillaryNode.java b/src/main/java/org/openstreetmap/josm/plugins/mapillary/data/mapillary/MapillaryNode.java
@@ -19,6 +19,16 @@ public class MapillaryNode extends MapillaryPrimitive implements INode, IPrimiti
     private double lon;
     private MapillarySequence referrer;
 
+    public MapillaryNode() {
+        // Do nothing
+    }
+
+    public MapillaryNode(MapillaryNode clone) {
+        this.lat = clone.lat;
+        this.lon = clone.lon;
+        // DO NOT CLONE THE REFERRER -- we _specifically_ only allow the referrer to be set _once_.
+    }
+
     @Override
     public double lon() {
         return this.lon;
@@ -52,6 +62,9 @@ public void setEastNorth(EastNorth eastNorth) {
 
     @Override
     public boolean isReferredByWays(int n) {
+        if (n == 1 && this.referrer != null) {
+            return true;
+        }
         return false;
     }
 

diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/layer/MapillaryLayer.java b/src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/layer/MapillaryLayer.java
@@ -466,18 +466,18 @@ private void drawImageMarker(final INode selectedImg, final Graphics2D g, final
         g.setColor(directionC);
         if (MapillaryImageUtils.IS_PANORAMIC.test(img)) {
             Composite currentComposite = g.getComposite();
+            AffineTransform scale = AffineTransform.getTranslateInstance(p.x, p.y);
+            scale.scale(2, 2);
+            g.setTransform(scale);
             g.setComposite(fadeComposite);
-            g.fillOval(p.x - CA_INDICATOR_RADIUS, p.y - CA_INDICATOR_RADIUS, 2 * CA_INDICATOR_RADIUS,
-                2 * CA_INDICATOR_RADIUS);
+            g.fill(IMAGE_CIRCLE);
             g.setComposite(currentComposite);
         }
-        AffineTransform backup = g.getTransform();
         g.setTransform(AffineTransform.getTranslateInstance(p.x, p.y));
         g.fill(IMAGE_CIRCLE);
-        g.setTransform(backup);
         if (i != null) {
-            // This _must_ be set after operations complete (see JOSM 19516 for more information)
-            backup = g.getTransform();
+            // This _must_ be set after operations complete (see JOSM #19516 for more information)
+            AffineTransform backup = g.getTransform();
             // convert the angle to radians from degrees
             double angle = MapillaryImageUtils.getAngle(img);
 

diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapillary/io/download/MapillaryDownloader.java b/src/main/java/org/openstreetmap/josm/plugins/mapillary/io/download/MapillaryDownloader.java
@@ -16,6 +16,8 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.concurrent.ForkJoinTask;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.stream.Collector;
 import java.util.stream.Collectors;
@@ -83,23 +85,33 @@ public static Map<String, Collection<MapillaryNode>> downloadImages(@Nullable Ma
         // This will be null if the image is null
         final long prioritizedId = MapillaryImageUtils.getKey(prioritizedImage);
 
+        Collection<ForkJoinTask<Map<String, List<MapillaryNode>>>> tasks = new ArrayList<>(images.length / step);
+        AtomicReference<ForkJoinTask<Map<String, List<MapillaryNode>>>> prioritizedTask = new AtomicReference<>();
         for (int i = 0; i <= images.length / step; i++) {
             final long[] imagesToGet = Arrays.copyOfRange(images, i * step, Math.min((i + 1) * step, images.length));
-            if (prioritizedImage == null
-                || prioritizedId > 0 && LongStream.of(imagesToGet).anyMatch(id -> id == prioritizedId)) {
-                realDownloadImages(imagesToGet).forEach((sequence, nodes) -> returnMap
-                    .computeIfAbsent(sequence, s -> new ArrayList<>(nodes.size())).addAll(nodes));
-                if (prioritizedImage != null && updater != null && returnMap.size() == 1) {
-                    final Map.Entry<String, Collection<MapillaryNode>> entry = returnMap.entrySet().iterator().next();
+            final ForkJoinTask<Map<String, List<MapillaryNode>>> task = ForkJoinTask.adapt(() -> {
+                Map<String, List<MapillaryNode>> map = realDownloadImages(imagesToGet);
+                if (prioritizedImage != null && updater != null && map.size() == 1) {
+                    final Map.Entry<String, List<MapillaryNode>> entry = map.entrySet().iterator().next();
                     final MapillarySequence sequence = new MapillarySequence(entry.getKey());
                     sequence.setNodes(new ArrayList<>(entry.getValue()));
                     updater.accept(Collections.singleton(sequence));
+                    return Collections.singletonMap(entry.getKey(),
+                        entry.getValue().stream().map(MapillaryNode::new).collect(Collectors.toList()));
                 }
+                return map;
+            });
+            tasks.add(task);
+            if (prioritizedImage == null
+                || (prioritizedId > 0 && LongStream.of(imagesToGet).anyMatch(id -> id == prioritizedId))) {
+                task.fork();
+                prioritizedTask.set(task);
             }
         }
-
-        if (prioritizedImage != null) {
-            return downloadImages(null, null, images);
+        tasks.stream().filter(task -> !Objects.equals(task, prioritizedTask.get())).forEach(ForkJoinTask::fork);
+        for (ForkJoinTask<Map<String, List<MapillaryNode>>> task : tasks) {
+            task.join().forEach((sequence, nodes) -> returnMap
+                .computeIfAbsent(sequence, s -> new ArrayList<>(nodes.size())).addAll(nodes));
         }
         return returnMap;
     }

diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapillary/utils/MapillaryImageUtils.java b/src/main/java/org/openstreetmap/josm/plugins/mapillary/utils/MapillaryImageUtils.java
@@ -61,8 +61,12 @@ public static <N extends INode> IWay<N> getSequence(@Nullable N image) {
         if (image == null) {
             return null;
         }
-        return image.getReferrers().stream().filter(IWay.class::isInstance).map(IWay.class::cast).findFirst()
-            .orElse(null);
+        for (IPrimitive p : image.getReferrers()) {
+            if (p instanceof IWay) {
+                return (IWay<N>) p;
+            }
+        }
+        return null;
     }
 
     /**
@@ -221,8 +225,9 @@ public static long getKey(@Nullable IPrimitive image, boolean ignoreId) {
             if (image.getUniqueId() > 0 && !ignoreId) {
                 return image.getId();
             }
-            if (image.hasKey(ImageProperties.ID.toString())
-                && NUMBERS.matcher(image.get(ImageProperties.ID.toString())).matches()) {
+            // This should always be a parseable integer according to API docs. By not checking that all characters are
+            // digits, we save 55.6 MB of allocations in the test area during download.
+            if (image.hasKey(ImageProperties.ID.toString())) {
                 final long id = Long.parseLong(image.get(ImageProperties.ID.toString()));
                 if (id > 0) {
                     image.setOsmId(id, 1);