Decrease allocations in various locations

With the changes from
325c169468bce987bd93e78da81b92c12e52d86f..2c11de9e3ca95e5ea26ba08b053692ff013eb728,
this reduces allocations due to MapWithAI cleanup methods
(on download) from 1.7 GB to 265 MB.

Signed-off-by: Taylor Smock <tsmock@fb.com>

src/main/java/org/openstreetmap/josm/plugins/mapwithai/actions/AddMapWithAILayerAction.java

@@ -4,12 +4,17 @@ package org.openstreetmap.josm.plugins.mapwithai.actions;
 import static org.openstreetmap.josm.gui.help.HelpUtil.ht;
 
 import java.awt.event.ActionEvent;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.ForkJoinTask;
 import java.util.concurrent.Future;
 
 import org.openstreetmap.josm.actions.AdaptableAction;
 import org.openstreetmap.josm.actions.AddImageryLayerAction;
 import org.openstreetmap.josm.actions.JosmAction;
+import org.openstreetmap.josm.data.Bounds;
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.OsmData;
 import org.openstreetmap.josm.gui.MainApplication;
@@ -17,8 +22,6 @@ import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 import org.openstreetmap.josm.gui.preferences.ToolbarPreferences;
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.gui.util.GuiHelper;
-import org.openstreetmap.josm.io.OsmTransferException;
-import org.openstreetmap.josm.plugins.mapwithai.backend.BoundingBoxMapWithAIDownloader;
 import org.openstreetmap.josm.plugins.mapwithai.backend.MapWithAIDataUtils;
 import org.openstreetmap.josm.plugins.mapwithai.backend.MapWithAILayer;
 import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo;
@@ -101,14 +104,17 @@ public class AddMapWithAILayerAction extends JosmAction implements AdaptableActi
             ds = null;
         }
         if (boundsSource != null && ds != null) {
-            boundsSource.getDataSourceBounds().forEach(b -> MainApplication.worker.execute(() -> {
+            ForkJoinPool pool = MapWithAIDataUtils.getForkJoinPool();
-                try {
+            List<ForkJoinTask<DataSet>> forkJoinTasks = new ArrayList<>(boundsSource.getDataSourceBounds().size());
-                    ds.mergeFrom(
+            for (Bounds b : boundsSource.getDataSourceBounds()) {
-                            new BoundingBoxMapWithAIDownloader(b, info, false).parseOsm(NullProgressMonitor.INSTANCE));
+                ForkJoinTask<DataSet> task = MapWithAIDataUtils.download(NullProgressMonitor.INSTANCE, b, info,
-                } catch (OsmTransferException error) {
+                        MapWithAIDataUtils.MAXIMUM_SIDE_DIMENSIONS);
-                    Logging.error(error);
+                forkJoinTasks.add(task);
-                }
+                pool.execute(task);
-            }));
+            }
+            for (ForkJoinTask<DataSet> task : forkJoinTasks) {
+                ds.mergeFrom(task.join());
+            }
         }
         if (layer != null) {
             layer.addDownloadedInfo(info);

src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/BoundingBoxMapWithAIDownloader.java

@@ -83,10 +83,7 @@ public class BoundingBoxMapWithAIDownloader extends BoundingBoxDownloader {
     public DataSet parseOsm(ProgressMonitor progressMonitor) throws OsmTransferException {
         long startTime = System.nanoTime();
         try {
-            DataSet externalData;
+            DataSet externalData = super.parseOsm(progressMonitor);
-            synchronized (BoundingBoxMapWithAIDownloader.class) {
-                externalData = super.parseOsm(progressMonitor);
-            }
             if (Boolean.TRUE.equals(MapWithAIInfo.THIRD_PARTY_CONFLATE.get()) && !this.info.isConflated()
                     && !MapWithAIConflationCategory.conflationUrlFor(this.info.getCategory()).isEmpty()) {
                 if (externalData.getDataSourceBounds().isEmpty()) {

src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/DownloadMapWithAITask.java

@@ -9,6 +9,8 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.ForkJoinTask;
 import java.util.concurrent.Future;
 import java.util.stream.Collectors;
 
@@ -18,7 +20,6 @@ import org.openstreetmap.josm.data.Bounds;
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.Notification;
-import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
 import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.io.OsmServerReader;
@@ -99,15 +100,10 @@ public class DownloadMapWithAITask extends DownloadOsmTask {
     }
 
     class DownloadTask extends AbstractInternalTask {
-        BoundingBoxMapWithAIDownloader downloader;
+        List<ForkJoinTask<DataSet>> downloader;
         final Bounds bounds;
         private List<MapWithAIInfo> relevantUrls;
 
-        DownloadTask(DownloadParams settings, String title, boolean ignoreException, boolean zoomAfterDownload,
-                Bounds bounds) {
-            this(settings, title, NullProgressMonitor.INSTANCE, ignoreException, zoomAfterDownload, bounds);
-        }
-
         public DownloadTask(DownloadParams settings, String title, ProgressMonitor progressMonitor,
                 boolean ignoreException, boolean zoomAfterDownload, Bounds bounds) {
             super(settings, title, progressMonitor, ignoreException, zoomAfterDownload);
@@ -118,7 +114,7 @@ public class DownloadMapWithAITask extends DownloadOsmTask {
         protected void cancel() {
             setCanceled(true);
             if (downloader != null) {
-                downloader.cancel();
+                downloader.forEach(task -> task.cancel(true));
             }
         }
 
@@ -137,14 +133,17 @@ public class DownloadMapWithAITask extends DownloadOsmTask {
                 monitor.setTicksCount(relevantUrls.size());
             }
             downloadedData = new DataSet();
+            final ForkJoinPool pool = MapWithAIDataUtils.getForkJoinPool();
+            this.downloader = new ArrayList<>(relevantUrls.size());
             for (MapWithAIInfo info : relevantUrls) {
                 if (isCanceled()) {
                     break;
                 }
-                downloader = new BoundingBoxMapWithAIDownloader(bounds, info, false);
+                this.downloader.add(pool.submit(MapWithAIDataUtils.download(this.progressMonitor, bounds, info,
-                DataSet ds = downloader.parseOsm(monitor.createSubTaskMonitor(1, true));
+                        MapWithAIDataUtils.MAXIMUM_SIDE_DIMENSIONS)));
-                downloadedData.mergeFrom(ds);
             }
+            this.downloader
+                    .forEach(task -> downloadedData.mergeFrom(task.join(), monitor.createSubTaskMonitor(1, false)));
         }
 
         @Override

src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/GetDataRunnable.java

@@ -4,6 +4,7 @@ package org.openstreetmap.josm.plugins.mapwithai.backend;
 import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
@@ -11,8 +12,12 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
 import java.util.TreeMap;
+import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.ForkJoinTask;
 import java.util.concurrent.RecursiveTask;
+import java.util.function.BiPredicate;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -28,6 +33,8 @@ import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.Hash;
 import org.openstreetmap.josm.data.osm.INode;
 import org.openstreetmap.josm.data.osm.IPrimitive;
+import org.openstreetmap.josm.data.osm.IRelation;
+import org.openstreetmap.josm.data.osm.IWay;
 import org.openstreetmap.josm.data.osm.IWaySegment;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
@@ -37,12 +44,12 @@ import org.openstreetmap.josm.data.osm.Tag;
 import org.openstreetmap.josm.data.osm.TagMap;
 import org.openstreetmap.josm.data.osm.UploadPolicy;
 import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.data.osm.visitor.PrimitiveVisitor;
 import org.openstreetmap.josm.data.projection.ProjectionRegistry;
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
-import org.openstreetmap.josm.io.OsmTransferException;
 import org.openstreetmap.josm.plugins.mapwithai.MapWithAIPlugin;
 import org.openstreetmap.josm.plugins.mapwithai.commands.MergeDuplicateWays;
 import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo;
@@ -50,7 +57,6 @@ import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAILayerInf
 import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.PreConflatedDataUtils;
 import org.openstreetmap.josm.tools.Geometry;
 import org.openstreetmap.josm.tools.JosmRuntimeException;
-import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.Pair;
 import org.openstreetmap.josm.tools.Utils;
 
@@ -111,6 +117,30 @@ public class GetDataRunnable extends RecursiveTask<DataSet> {
         }
     }
 
+    /**
+     * This checks that all visited objects are highways
+     */
+    private static final class HighwayVisitor implements PrimitiveVisitor {
+        boolean onlyHighways = true;
+
+        @Override
+        public void visit(INode n) {
+            onlyHighways = false;
+        }
+
+        @Override
+        public void visit(IWay<?> w) {
+            if (!w.isTagged() || !w.hasTag("highway")) {
+                onlyHighways = false;
+            }
+        }
+
+        @Override
+        public void visit(IRelation<?> r) {
+            onlyHighways = false;
+        }
+    }
+
     private static final long serialVersionUID = 258423685658089715L;
     private final transient List<Bounds> runnableBounds;
     private final transient DataSet dataSet;
@@ -416,11 +446,22 @@ public class GetDataRunnable extends RecursiveTask<DataSet> {
      * @param dataSet The dataset to remove tags from
      */
     public static void removeCommonTags(DataSet dataSet) {
-        dataSet.allPrimitives().stream().filter(prim -> prim.hasKey(MergeDuplicateWays.ORIG_ID))
+        final Set<Node> emptyNodes = new HashSet<>();
-                .forEach(prim -> prim.remove(MergeDuplicateWays.ORIG_ID));
+        for (OsmPrimitive tagged : dataSet.allPrimitives()) {
-        dataSet.getNodes().forEach(node -> node.remove(SERVER_ID_KEY));
+            if (!tagged.hasKeys()) {
-        final List<Node> emptyNodes = dataSet.getNodes().stream().distinct().filter(node -> !node.isDeleted())
+                continue;
-                .filter(node -> node.getReferrers().isEmpty() && !node.hasKeys()).collect(Collectors.toList());
+            }
+            if (tagged.hasKey(MergeDuplicateWays.ORIG_ID)) {
+                tagged.remove(MergeDuplicateWays.ORIG_ID);
+            }
+            if (tagged instanceof Node) {
+                tagged.remove(SERVER_ID_KEY);
+                final Node node = (Node) tagged;
+                if (!node.isDeleted() && !node.hasKeys() && node.getReferrers().isEmpty()) {
+                    emptyNodes.add(node);
+                }
+            }
+        }
         if (!emptyNodes.isEmpty()) {
             new DeleteCommand(emptyNodes).executeCommand();
         }
@@ -483,21 +524,22 @@ public class GetDataRunnable extends RecursiveTask<DataSet> {
         return basicNodeChecks(nearNode, node) && onlyHasHighwayParents(node)
                 && ((keyCheck(nearNode, node)
                         && distanceCheck(nearNode, node, MapWithAIPreferenceHelper.getMaxNodeDistance()))
-                        || (!nearNode.getKeys().isEmpty() && nearNode.getKeys().equals(node.getKeys())
+                        || (nearNode.hasKeys() && node.hasKeys() && nearNode.getKeys().equals(node.getKeys())
                                 && distanceCheck(nearNode, node, MapWithAIPreferenceHelper.getMaxNodeDistance() * 10)));
     }
 
-    private static boolean distanceCheck(INode nearNode, INode node, Double distance) {
+    private static boolean distanceCheck(Node nearNode, Node node, Double distance) {
-        return !(nearNode == null || node == null || nearNode.getCoor() == null || node.getCoor() == null)
+        return Geometry.getDistance(nearNode, node) < distance;
-                && nearNode.getCoor().greatCircleDistance(node.getCoor()) < distance;
     }
 
     private static boolean keyCheck(INode nearNode, INode node) {
-        return nearNode.getKeys().equals(node.getKeys()) || nearNode.getKeys().isEmpty() || node.getKeys().isEmpty();
+        return !nearNode.hasKeys() || !node.hasKeys() || nearNode.getKeys().equals(node.getKeys());
     }
 
     private static boolean onlyHasHighwayParents(Node node) {
-        return node.referrers(OsmPrimitive.class).allMatch(prim -> prim.hasKey("highway"));
+        HighwayVisitor highwayVisitor = new HighwayVisitor();
+        node.visitReferrers(highwayVisitor);
+        return highwayVisitor.onlyHighways;
     }
 
     private static boolean basicNodeChecks(INode nearNode, INode node) {
@@ -506,16 +548,21 @@ public class GetDataRunnable extends RecursiveTask<DataSet> {
     }
 
     private static void mergeWays(DataSet dataSet) {
-        final List<Way> ways = dataSet.getWays().stream().filter(way -> !way.isDeleted()).collect(Collectors.toList());
+        for (final Way way1 : dataSet.getWays()) {
-        for (final Way way1 : ways) {
+            if (way1.isDeleted()) {
+                continue;
+            }
             final BBox bbox = new BBox();
             bbox.addPrimitive(way1, DEGREE_BUFFER);
-            final List<Way> nearbyWays = dataSet.searchWays(bbox).stream()
+            for (Way nearbyWay : dataSet.searchWays(bbox)) {
-                    .filter(way -> way.getNodes().stream().filter(node -> way1.getNodes().contains(node)).count() > 1)
+                if (nearbyWay.getNodes().stream().filter(way1::containsNode).count() > 1) {
-                    .collect(Collectors.toList());
+                    // way1.getNodePairs(false);
-            way1.getNodePairs(false);
+                    for (Map.Entry<IWaySegment<Node, Way>, List<IWaySegment<Node, Way>>> entry : checkWayDuplications(
-            nearbyWays.stream().flatMap(way2 -> checkWayDuplications(way1, way2).entrySet().stream())
+                            way1, nearbyWay).entrySet()) {
-                    .forEach(GetDataRunnable::addMissingElement);
+                        GetDataRunnable.addMissingElement(entry);
+                    }
+                }
+            }
         }
     }
 
@@ -589,32 +636,26 @@ public class GetDataRunnable extends RecursiveTask<DataSet> {
         final List<IWaySegment<Node, Way>> waySegments2 = way2.getNodePairs(false).stream()
                 .map(pair -> IWaySegment.forNodePair(way2, pair.a, pair.b)).collect(Collectors.toList());
         final Map<IWaySegment<Node, Way>, List<IWaySegment<Node, Way>>> partials = new TreeMap<>();
+        final BiPredicate<IWaySegment<Node, Way>, IWaySegment<Node, Way>> connected = (segment1,
+                segment2) -> segment1.getFirstNode().equals(segment2.getFirstNode())
+                        || segment1.getSecondNode().equals(segment2.getFirstNode())
+                        || segment1.getFirstNode().equals(segment2.getSecondNode())
+                        || segment1.getSecondNode().equals(segment2.getSecondNode());
         for (final IWaySegment<Node, Way> segment1 : waySegments1) {
-            final Way waySegment1;
-            try {
-                waySegment1 = segment1.toWay();
-            } catch (ReflectiveOperationException e) {
-                throw new JosmRuntimeException(e);
-            }
             final List<IWaySegment<Node, Way>> replacements = waySegments2.stream()
-                    .filter(seg2 -> waySegment1.isFirstLastNode(seg2.getFirstNode())
+                    .filter(seg2 -> connected.test(segment1, seg2)).filter(seg -> {
-                            || waySegment1.isFirstLastNode(seg2.getSecondNode()))
+                        final Node node2 = segment1.getFirstNode().equals(seg.getFirstNode())
-                    .filter(seg -> {
+                                || segment1.getSecondNode().equals(seg.getFirstNode()) ? seg.getFirstNode()
-                        final Node node2 = waySegment1.isFirstLastNode(seg.getFirstNode()) ? seg.getFirstNode()
+                                        : seg.getSecondNode();
-                                : seg.getSecondNode();
                         final Node node1 = node2.equals(seg.getFirstNode()) ? seg.getSecondNode() : seg.getFirstNode();
-                        final Node node3 = waySegment1.getNode(0).equals(node2) ? waySegment1.getNode(1)
+                        final Node node3 = segment1.getFirstNode().equals(node2) ? segment1.getSecondNode()
-                                : waySegment1.getNode(0);
+                                : segment1.getFirstNode();
                         return Math.abs(Geometry.getCornerAngle(node1.getEastNorth(), node2.getEastNorth(),
                                 node3.getEastNorth())) < (Math.PI / 4);
                     }).collect(Collectors.toList());
-            if ((replacements.size() != 2) || replacements.stream().anyMatch(seg -> {
+            if ((replacements.size() != 2) || replacements.stream()
-                try {
+                    .anyMatch(seg -> Arrays.asList(segment1.getFirstNode(), segment1.getSecondNode())
-                    return new HashSet<>(waySegment1.getNodes()).containsAll(seg.toWay().getNodes());
+                            .containsAll(Arrays.asList(seg.getFirstNode(), seg.getSecondNode())))) {
-                } catch (ReflectiveOperationException e) {
-                    throw new JosmRuntimeException(e);
-                }
-            })) {
                 continue;
             }
             partials.put(segment1, replacements);
@@ -633,14 +674,14 @@ public class GetDataRunnable extends RecursiveTask<DataSet> {
         final DataSet dataSet = new DataSet();
         dataSet.setUploadPolicy(UploadPolicy.DISCOURAGED);
 
+        final List<ForkJoinTask<DataSet>> tasks = new ArrayList<>();
+        final ForkJoinPool pool = MapWithAIDataUtils.getForkJoinPool();
         for (MapWithAIInfo map : new ArrayList<>(MapWithAILayerInfo.getInstance().getLayers())) {
-            try {
+            tasks.add(pool.submit(
-                BoundingBoxMapWithAIDownloader downloader = new BoundingBoxMapWithAIDownloader(bounds, map,
+                    MapWithAIDataUtils.download(monitor, bounds, map, MapWithAIDataUtils.MAXIMUM_SIDE_DIMENSIONS)));
-                        DetectTaskingManagerUtils.hasTaskingManagerLayer());
+        }
-                dataSet.mergeFrom(downloader.parseOsm(monitor));
+        for (ForkJoinTask<DataSet> task : tasks) {
-            } catch (OsmTransferException e1) {
+            dataSet.mergeFrom(task.join());
-                Logging.debug(e1);
-            }
         }
         dataSet.setUploadPolicy(UploadPolicy.BLOCKED);
         return dataSet;

src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIDataUtils.java

@@ -33,6 +33,7 @@ import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil;
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.Notification;
 import org.openstreetmap.josm.gui.layer.OsmDataLayer;
+import org.openstreetmap.josm.gui.progress.ProgressMonitor;
 import org.openstreetmap.josm.gui.progress.swing.PleaseWaitProgressMonitor;
 import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.io.IllegalDataException;
@@ -160,8 +161,8 @@ public final class MapWithAIDataUtils {
      * @param maximumDimensions The maximum dimensions to download
      * @return A future that will have downloaded the data
      */
-    private static ForkJoinTask<DataSet> download(PleaseWaitProgressMonitor monitor, Bounds bound,
+    public static ForkJoinTask<DataSet> download(ProgressMonitor monitor, Bounds bound, MapWithAIInfo mapWithAIInfo,
-            MapWithAIInfo mapWithAIInfo, int maximumDimensions) {
+            int maximumDimensions) {
         return ForkJoinTask.adapt(() -> {
             BoundingBoxMapWithAIDownloader downloader = new BoundingBoxMapWithAIDownloader(bound, mapWithAIInfo,
                     DetectTaskingManagerUtils.hasTaskingManagerLayer());