Ticket #13275: 13275.patch

src/org/openstreetmap/josm/data/osm/MultipolygonBuilder.java

@@ -5 +5 @@
 
 import java.awt.Rectangle;
 import java.awt.geom.Area;
-import java.io.IOException;
-import java.io.ObjectInputStream;
-import java.io.ObjectOutputStream;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Objects;
 import java.util.Set;
-import java.util.concurrent.ForkJoinPool;
-import java.util.concurrent.ForkJoinTask;
-import java.util.concurrent.RecursiveTask;
 
 import org.openstreetmap.josm.Main;
 import org.openstreetmap.josm.tools.Geometry;
 import org.openstreetmap.josm.tools.Geometry.PolygonIntersection;
 import org.openstreetmap.josm.tools.MultiMap;
 import org.openstreetmap.josm.tools.Pair;
-import org.openstreetmap.josm.tools.Utils;
 
 /**
  * Helper class to build multipolygons from multiple ways.
@@ -33 +27 @@
  */
 public class MultipolygonBuilder {
 
-    private static final ForkJoinPool THREAD_POOL =
-            Utils.newForkJoinPool("multipolygon_creation.numberOfThreads", "multipolygon-builder-%d", Thread.NORM_PRIORITY);
-
     /**
      * Represents one polygon that consists of multiple ways.
      */
@@ -306 +297 @@
      * @return the outermostWay, or {@code null} if intersection found.
      */
     private static List<PolygonLevel> findOuterWaysMultiThread(List<JoinedPolygon> boundaryWays) {
-        return THREAD_POOL.invoke(new Worker(boundaryWays, 0, boundaryWays.size(), new ArrayList<PolygonLevel>(),
-                Math.max(32, boundaryWays.size() / THREAD_POOL.getParallelism() / 3)));
+        final List<PolygonLevel> output = new ArrayList<>();
+        return boundaryWays.parallelStream()
+                .map(way -> Worker.processOuterWay(0, boundaryWays, output, way))
+                .allMatch(Objects::nonNull) ? output : null;
     }
 
-    private static class Worker extends RecursiveTask<List<PolygonLevel>> {
-
-        // Needed for Findbugs / Coverity because parent class is serializable
-        private static final long serialVersionUID = 1L;
-
-        private final transient List<JoinedPolygon> input;
-        private final int from;
-        private final int to;
-        private final transient List<PolygonLevel> output;
-        private final int directExecutionTaskSize;
-
-        Worker(List<JoinedPolygon> input, int from, int to, List<PolygonLevel> output, int directExecutionTaskSize) {
-            this.input = input;
-            this.from = from;
-            this.to = to;
-            this.output = output;
-            this.directExecutionTaskSize = directExecutionTaskSize;
-        }
+    private static class Worker {
 
         /**
          * Collects outer way and corresponding inner ways from all boundaries.
@@ -380 +356 @@
             }
             return result;
         }
-
-        @Override
-        protected List<PolygonLevel> compute() {
-            if (to - from <= directExecutionTaskSize) {
-                return computeDirectly();
-            } else {
-                final Collection<ForkJoinTask<List<PolygonLevel>>> tasks = new ArrayList<>();
-                for (int fromIndex = from; fromIndex < to; fromIndex += directExecutionTaskSize) {
-                    tasks.add(new Worker(input, fromIndex, Math.min(fromIndex + directExecutionTaskSize, to),
-                            new ArrayList<PolygonLevel>(), directExecutionTaskSize));
-                }
-                for (ForkJoinTask<List<PolygonLevel>> task : ForkJoinTask.invokeAll(tasks)) {
-                    List<PolygonLevel> res = task.join();
-                    if (res == null) {
-                        return null;
-                    }
-                    output.addAll(res);
-                }
-                return output;
-            }
-        }
-
-        List<PolygonLevel> computeDirectly() {
-            for (int i = from; i < to; i++) {
-                if (processOuterWay(0, input, output, input.get(i)) == null) {
-                    return null;
-                }
-            }
-            return output;
-        }
-
-        private void readObject(ObjectInputStream ois) throws ClassNotFoundException, IOException {
-            // Needed for Findbugs / Coverity because parent class is serializable
-            ois.defaultReadObject();
-        }
-
-        private void writeObject(ObjectOutputStream oos) throws IOException {
-            // Needed for Findbugs / Coverity because parent class is serializable
-            oos.defaultWriteObject();
-        }
     }
 }

src/org/openstreetmap/josm/data/osm/visitor/paint/RenderBenchmarkCollector.java

@@ -2 +2 @@
 package org.openstreetmap.josm.data.osm.visitor.paint;
 
 import java.io.PrintStream;
-import java.util.List;
+import java.util.Collection;
 import java.util.function.Supplier;
 
 import org.openstreetmap.josm.Main;
@@ -39 +39 @@
      * @param allStyleElems All the elements that are painted.
      * @return <code>true</code> if the renderer should continue to render
      */
-    public boolean renderDraw(List<StyleRecord> allStyleElems) {
+    public boolean renderDraw(Collection<StyleRecord> allStyleElems) {
         // nop
         return true;
     }
@@ -74 +74 @@
         }
 
         @Override
-        public boolean renderDraw(List<StyleRecord> allStyleElems) {
+        public boolean renderDraw(Collection<StyleRecord> allStyleElems) {
             timeSortingDone = System.currentTimeMillis();
             return super.renderDraw(allStyleElems);
         }
@@ -126 +126 @@
         }
 
         @Override
-        public boolean renderDraw(List<StyleRecord> allStyleElems) {
+        public boolean renderDraw(Collection<StyleRecord> allStyleElems) {
             boolean res = super.renderDraw(allStyleElems);
             outStream.print("phase 1 (calculate styles): " + Utils.getDurationString(timeSortingDone - timeStart));
             return res;

src/org/openstreetmap/josm/data/osm/visitor/paint/StyledMapRenderer.java

@@ -27 +27 @@
 import java.awt.geom.Rectangle2D;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
+import java.util.TreeSet;
 import java.util.concurrent.ForkJoinPool;
-import java.util.concurrent.ForkJoinTask;
-import java.util.concurrent.RecursiveTask;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import javax.swing.AbstractButton;
 import javax.swing.FocusManager;
@@ -45 +45 @@
 import org.openstreetmap.josm.data.Bounds;
 import org.openstreetmap.josm.data.coor.EastNorth;
 import org.openstreetmap.josm.data.osm.BBox;
-import org.openstreetmap.josm.data.osm.Changeset;
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
@@ -54 +53 @@
 import org.openstreetmap.josm.data.osm.RelationMember;
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.data.osm.WaySegment;
-import org.openstreetmap.josm.data.osm.visitor.Visitor;
 import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon;
 import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon.PolyData;
 import org.openstreetmap.josm.data.osm.visitor.paint.relations.MultipolygonCache;
 import org.openstreetmap.josm.gui.NavigatableComponent;
 import org.openstreetmap.josm.gui.mappaint.ElemStyles;
 import org.openstreetmap.josm.gui.mappaint.MapPaintStyles;
-import org.openstreetmap.josm.gui.mappaint.StyleElementList;
 import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSStyleSource;
 import org.openstreetmap.josm.gui.mappaint.styleelement.AreaElement;
 import org.openstreetmap.josm.gui.mappaint.styleelement.BoxTextElement;
@@ -73 +70 @@
 import org.openstreetmap.josm.gui.mappaint.styleelement.RepeatImageElement.LineImageAlignment;
 import org.openstreetmap.josm.gui.mappaint.styleelement.StyleElement;
 import org.openstreetmap.josm.gui.mappaint.styleelement.TextLabel;
-import org.openstreetmap.josm.tools.CompositeList;
 import org.openstreetmap.josm.tools.Geometry;
 import org.openstreetmap.josm.tools.Geometry.AreaAndPerimeter;
 import org.openstreetmap.josm.tools.ImageProvider;
+import org.openstreetmap.josm.tools.StreamUtils;
 import org.openstreetmap.josm.tools.Utils;
 
 /**
@@ -1779 +1776 @@
         }
     }
 
-    private class ComputeStyleListWorker extends RecursiveTask<List<StyleRecord>> implements Visitor {
-        private final transient List<? extends OsmPrimitive> input;
-        private final transient List<StyleRecord> output;
-
+    private class ComputeStyleListWorker {
         private final transient ElemStyles styles = MapPaintStyles.getStyles();
-        private final int directExecutionTaskSize;
-
         private final boolean drawArea = circum <= Main.pref.getInteger("mappaint.fillareas", 10000000);
         private final boolean drawMultipolygon = drawArea && Main.pref.getBoolean("mappaint.multipolygon", true);
         private final boolean drawRestriction = Main.pref.getBoolean("mappaint.restriction", true);
 
         /**
          * Constructs a new {@code ComputeStyleListWorker}.
-         * @param input the primitives to process
-         * @param output the list of styles to which styles will be added
-         * @param directExecutionTaskSize the threshold deciding whether to subdivide the tasks
          */
-        ComputeStyleListWorker(final List<? extends OsmPrimitive> input, List<StyleRecord> output, int directExecutionTaskSize) {
-            this.input = input;
-            this.output = output;
-            this.directExecutionTaskSize = directExecutionTaskSize;
+        ComputeStyleListWorker() {
             this.styles.setDrawMultipolygon(drawMultipolygon);
         }
 
-        @Override
-        protected List<StyleRecord> compute() {
-            if (input.size() <= directExecutionTaskSize) {
-                return computeDirectly();
-            } else {
-                final Collection<ForkJoinTask<List<StyleRecord>>> tasks = new ArrayList<>();
-                for (int fromIndex = 0; fromIndex < input.size(); fromIndex += directExecutionTaskSize) {
-                    final int toIndex = Math.min(fromIndex + directExecutionTaskSize, input.size());
-                    final List<StyleRecord> output = new ArrayList<>(directExecutionTaskSize);
-                    tasks.add(new ComputeStyleListWorker(input.subList(fromIndex, toIndex), output, directExecutionTaskSize).fork());
-                }
-                for (ForkJoinTask<List<StyleRecord>> task : tasks) {
-                    output.addAll(task.join());
-                }
-                return output;
-            }
-        }
-
-        public List<StyleRecord> computeDirectly() {
-            MapCSSStyleSource.STYLE_SOURCE_LOCK.readLock().lock();
-            try {
-                for (final OsmPrimitive osm : input) {
-                    if (osm.isDrawable()) {
-                        osm.accept(this);
-                    }
-                }
-                return output;
-            } finally {
-                MapCSSStyleSource.STYLE_SOURCE_LOCK.readLock().unlock();
-            }
-        }
-
-        @Override
-        public void visit(Node n) {
-            add(n, computeFlags(n, false));
-        }
-
-        @Override
-        public void visit(Way w) {
-            add(w, computeFlags(w, true));
-        }
-
-        @Override
-        public void visit(Relation r) {
-            add(r, computeFlags(r, true));
-        }
-
-        @Override
-        public void visit(Changeset cs) {
-            throw new UnsupportedOperationException();
-        }
-
-        public void add(Node osm, int flags) {
-            StyleElementList sl = styles.get(osm, circum, nc);
-            for (StyleElement s : sl) {
-                output.add(new StyleRecord(s, osm, flags));
-            }
-        }
-
-        public void add(Relation osm, int flags) {
-            StyleElementList sl = styles.get(osm, circum, nc);
-            for (StyleElement s : sl) {
-                if (drawMultipolygon && drawArea && s instanceof AreaElement && (flags & FLAG_DISABLED) == 0) {
-                    output.add(new StyleRecord(s, osm, flags));
-                } else if (drawRestriction && s instanceof NodeElement) {
-                    output.add(new StyleRecord(s, osm, flags));
-                }
-            }
-        }
-
-        public void add(Way osm, int flags) {
-            StyleElementList sl = styles.get(osm, circum, nc);
-            for (StyleElement s : sl) {
-                if (!(drawArea && (flags & FLAG_DISABLED) == 0) && s instanceof AreaElement) {
-                    continue;
-                }
-                output.add(new StyleRecord(s, osm, flags));
-            }
+        Stream<StyleRecord> computeNodeStyle(Node osm) {
+            final int flags = computeFlags(osm, false);
+            return StreamUtils.toStream(styles.get(osm, circum, nc))
+                    .map(s -> new StyleRecord(s, osm, flags));
+        }
+
+        Stream<StyleRecord> computeRelationStyle(Relation osm) {
+            final int flags = computeFlags(osm, true);
+            return StreamUtils.toStream(styles.get(osm, circum, nc))
+                    .filter(s ->
+                            drawMultipolygon && drawArea && s instanceof AreaElement && (flags & FLAG_DISABLED) == 0
+                            || drawRestriction && s instanceof NodeElement
+                    )
+                    .map(s -> new StyleRecord(s, osm, flags));
+        }
+
+        Stream<StyleRecord> computeWayStyle(Way osm) {
+            final int flags = computeFlags(osm, true);
+            return StreamUtils.toStream(styles.get(osm, circum, nc))
+                    .filter(s -> !(!(drawArea && (flags & FLAG_DISABLED) == 0) && s instanceof AreaElement))
+                    .map(s -> new StyleRecord(s, osm, flags));
         }
     }
 
@@ -1909 +1838 @@
             List<Way> ways = data.searchWays(bbox);
             List<Relation> relations = data.searchRelations(bbox);
 
-            final List<StyleRecord> allStyleElems = new ArrayList<>(nodes.size()+ways.size()+relations.size());
+            final Collection<StyleRecord> allStyleElems;
 
             // Need to process all relations first.
             // Reason: Make sure, ElemStyles.getStyleCacheWithRange is
             // not called for the same primitive in parallel threads.
             // (Could be synchronized, but try to avoid this for
             // performance reasons.)
-            THREAD_POOL.invoke(new ComputeStyleListWorker(relations, allStyleElems,
-                    Math.max(20, relations.size() / THREAD_POOL.getParallelism() / 3)));
-            THREAD_POOL.invoke(new ComputeStyleListWorker(new CompositeList<>(nodes, ways), allStyleElems,
-                    Math.max(100, (nodes.size() + ways.size()) / THREAD_POOL.getParallelism() / 3)));
+            MapCSSStyleSource.STYLE_SOURCE_LOCK.readLock().lock();
+            try {
+                final ComputeStyleListWorker worker = new ComputeStyleListWorker();
+                // run parallel stream in THREAD_POOL, see https://stackoverflow.com/a/22269778/205629
+                allStyleElems = THREAD_POOL.submit(() ->
+                        relations.parallelStream().filter(OsmPrimitive::isDrawable).flatMap(worker::computeRelationStyle)
+                                .collect(Collectors.toCollection(TreeSet::new))).get();
+                allStyleElems.addAll(THREAD_POOL.submit(() -> Stream.concat(
+                        nodes.parallelStream().filter(OsmPrimitive::isDrawable).flatMap(worker::computeNodeStyle),
+                        ways.parallelStream().filter(OsmPrimitive::isDrawable).flatMap(worker::computeWayStyle)
+                                ).collect(Collectors.toList())).get());
+            } catch (Exception ex) {
+                throw new RuntimeException(ex);
+            } finally {
+                MapCSSStyleSource.STYLE_SOURCE_LOCK.readLock().unlock();
+            }
 
             if (!benchmark.renderSort()) {
                 return;
             }
 
-            Collections.sort(allStyleElems); // TODO: try parallel sort when switching to Java 8
-
             if (!benchmark.renderDraw(allStyleElems)) {
                 return;
             }

test/performance/org/openstreetmap/josm/gui/mappaint/MapRendererPerformanceTest.java

@@ -8 +8 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumMap;
 import java.util.HashMap;
@@ -331 +332 @@
 
     public static class BenchmarkData extends CapturingBenchmark {
 
-        private List<StyleRecord> allStyleElems;
+        private Collection<StyleRecord> allStyleElems;
 
         @Override
-        public boolean renderDraw(List<StyleRecord> allStyleElems) {
+        public boolean renderDraw(Collection<StyleRecord> allStyleElems) {
             this.allStyleElems = allStyleElems;
             return super.renderDraw(allStyleElems);
         }