From 791ef634290c64ae61169df8ff5191e82e3027b4 Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Mon, 25 Apr 2022 15:12:23 -0600 Subject: [PATCH] Various performance related fixes Signed-off-by: Taylor Smock --- .../data/mapillary/MapillaryNode.java | 13 ++++++++ .../mapillary/gui/layer/MapillaryLayer.java | 12 ++++---- .../io/download/MapillaryDownloader.java | 30 +++++++++++++------ .../mapillary/utils/MapillaryImageUtils.java | 13 +++++--- 4 files changed, 49 insertions(+), 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 index 39f9a88b9..17b7428a4 100644 --- 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 index 915924857..d4427f5ee 100644 --- 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 index a53f9d06f..f65c2d619 100644 --- 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> downloadImages(@Nullable Ma // This will be null if the image is null final long prioritizedId = MapillaryImageUtils.getKey(prioritizedImage); + Collection>>> tasks = new ArrayList<>(images.length / step); + AtomicReference>>> 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> entry = returnMap.entrySet().iterator().next(); + final ForkJoinTask>> task = ForkJoinTask.adapt(() -> { + Map> map = realDownloadImages(imagesToGet); + if (prioritizedImage != null && updater != null && map.size() == 1) { + final Map.Entry> 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>> 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 index f8cfaacb9..bd055a831 100644 --- 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 IWay 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) 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);