Index: src/org/openstreetmap/josm/data/osm/MultipolygonBuilder.java =================================================================== --- src/org/openstreetmap/josm/data/osm/MultipolygonBuilder.java (revision 10743) +++ src/org/openstreetmap/josm/data/osm/MultipolygonBuilder.java (working copy) @@ -5,18 +5,13 @@ 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; @@ -23,7 +18,6 @@ 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,9 +27,54 @@ */ public class MultipolygonBuilder { - private static final ForkJoinPool THREAD_POOL = - Utils.newForkJoinPool("multipolygon_creation.numberOfThreads", "multipolygon-builder-%d", Thread.NORM_PRIORITY); + private static class ResultCache { + Geometry.PolygonIntersection[] results; + private final int dim; + private long countCheck; + private long countMiss; + public ResultCache(Collection polygons) { + int id = 0; + for (JoinedPolygon p : polygons) + p.setCacheId(id++); + + this.dim = id; + results = new Geometry.PolygonIntersection[dim*dim]; + } + + + private synchronized PolygonIntersection getCachedResult(JoinedPolygon pa, JoinedPolygon pb){ + if (pa.id < 0 || pa.id >= dim ){ + throw new JoinedPolygonCreationException(tr("Internal error: unexpected id in 1st polygon", pa.id)); + } + if (pb.id < 0 || pb.id >= dim ){ + throw new JoinedPolygonCreationException(tr("Internal error: unexpected id in 2nd polygon", pb.id)); + } + countCheck++; + int posAB = pa.id * dim + pb.id; + if (results[posAB] == null){ + countMiss++; + PolygonIntersection intersection = Geometry.polygonIntersection(pa.area, pb.area); + results[posAB] = intersection; + // set the results for exchanged parameters (a is outer b also means b is outer a etc.) + int posBA = pb.id * dim + pa.id; + if (intersection == PolygonIntersection.OUTSIDE || intersection == PolygonIntersection.CROSSING){ + results[posBA] = intersection; + } else if (intersection == PolygonIntersection.FIRST_INSIDE_SECOND){ + results[posBA] = PolygonIntersection.SECOND_INSIDE_FIRST; + } else if (intersection == PolygonIntersection.SECOND_INSIDE_FIRST) + results[posBA] = PolygonIntersection.FIRST_INSIDE_SECOND; + + } + return results[posAB]; + } + + @Override + public String toString() { + return "Tests: " + countCheck + " hit/miss " + (countCheck - countMiss) + "/" + countMiss; + } + } + /** * Represents one polygon that consists of multiple ways. */ @@ -45,6 +84,7 @@ public final List nodes; public final Area area; public final Rectangle bounds; + private int id; /** * Constructs a new {@code JoinedPolygon} from given list of ways. @@ -57,6 +97,7 @@ this.nodes = this.getNodes(); this.area = Geometry.getArea(nodes); this.bounds = area.getBounds(); + this.id = -1; } /** @@ -91,6 +132,14 @@ return nodes; } + + /** + * Set id that is used in ResultCache + * @param id + */ + public void setCacheId(int id) { + this.id = id; + } } /** @@ -271,7 +320,7 @@ return null; } - private static Pair> findInnerWaysCandidates(JoinedPolygon outerWay, Collection boundaryWays) { + private static Pair> findInnerWaysCandidates(ResultCache cache, JoinedPolygon outerWay, Collection boundaryWays) { boolean outerGood = true; List innerCandidates = new ArrayList<>(); @@ -283,7 +332,8 @@ // Preliminary computation on bounds. If bounds do not intersect, no need to do a costly area intersection if (outerWay.bounds.intersects(innerWay.bounds)) { // Bounds intersection, let's see in detail - PolygonIntersection intersection = Geometry.polygonIntersection(outerWay.area, innerWay.area); + PolygonIntersection intersection = cache.getCachedResult(outerWay, innerWay); +// PolygonIntersection intersection = Geometry.polygonIntersection(outerWay.area, innerWay.area); if (intersection == PolygonIntersection.FIRST_INSIDE_SECOND) { outerGood = false; // outer is inside another polygon @@ -306,41 +356,31 @@ * @return the outermostWay, or {@code null} if intersection found. */ private static List findOuterWaysMultiThread(List boundaryWays) { - return THREAD_POOL.invoke(new Worker(boundaryWays, 0, boundaryWays.size(), new ArrayList(), - Math.max(32, boundaryWays.size() / THREAD_POOL.getParallelism() / 3))); + ResultCache cache = new ResultCache(boundaryWays); + final List output = new ArrayList<>(); + List res = boundaryWays.parallelStream() + .map(way -> Worker.processOuterWay(0, cache, boundaryWays, output, way)) + .allMatch(Objects::nonNull) ? output : null; + if (!boundaryWays.isEmpty()) + Main.debug("mp cache: " + cache.toString()); + return res; } - private static class Worker extends RecursiveTask> { + private static class Worker { - // Needed for Findbugs / Coverity because parent class is serializable - private static final long serialVersionUID = 1L; - - private final transient List input; - private final int from; - private final int to; - private final transient List output; - private final int directExecutionTaskSize; - - Worker(List input, int from, int to, List output, int directExecutionTaskSize) { - this.input = input; - this.from = from; - this.to = to; - this.output = output; - this.directExecutionTaskSize = directExecutionTaskSize; - } - /** * Collects outer way and corresponding inner ways from all boundaries. * @param level nesting level + * @param cache cache that tracks previously calculated results * @param boundaryWays boundary ways * @return the outermostWay, or {@code null} if intersection found. */ - private static List findOuterWaysRecursive(int level, List boundaryWays) { + private static List findOuterWaysRecursive(int level, ResultCache cache, List boundaryWays) { final List result = new ArrayList<>(); for (JoinedPolygon outerWay : boundaryWays) { - if (processOuterWay(level, boundaryWays, result, outerWay) == null) { + if (processOuterWay(level, cache, boundaryWays, result, outerWay) == null) { return null; } } @@ -348,9 +388,9 @@ return result; } - private static List processOuterWay(int level, List boundaryWays, + private static List processOuterWay(int level, ResultCache cache, List boundaryWays, final List result, JoinedPolygon outerWay) { - Pair> p = findInnerWaysCandidates(outerWay, boundaryWays); + Pair> p = findInnerWaysCandidates(cache, outerWay, boundaryWays); if (p == null) { // ways intersect return null; @@ -362,7 +402,7 @@ //process inner ways if (!p.b.isEmpty()) { - List innerList = findOuterWaysRecursive(level + 1, p.b); + List innerList = findOuterWaysRecursive(level + 1, cache, p.b); if (innerList == null) { return null; //intersection found } @@ -380,45 +420,5 @@ } return result; } - - @Override - protected List compute() { - if (to - from <= directExecutionTaskSize) { - return computeDirectly(); - } else { - final Collection>> tasks = new ArrayList<>(); - for (int fromIndex = from; fromIndex < to; fromIndex += directExecutionTaskSize) { - tasks.add(new Worker(input, fromIndex, Math.min(fromIndex + directExecutionTaskSize, to), - new ArrayList(), directExecutionTaskSize)); - } - for (ForkJoinTask> task : ForkJoinTask.invokeAll(tasks)) { - List res = task.join(); - if (res == null) { - return null; - } - output.addAll(res); - } - return output; - } - } - - List 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(); - } } } Index: src/org/openstreetmap/josm/data/osm/visitor/paint/RenderBenchmarkCollector.java =================================================================== --- src/org/openstreetmap/josm/data/osm/visitor/paint/RenderBenchmarkCollector.java (revision 10743) +++ src/org/openstreetmap/josm/data/osm/visitor/paint/RenderBenchmarkCollector.java (working copy) @@ -2,7 +2,7 @@ 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,7 +39,7 @@ * @param allStyleElems All the elements that are painted. * @return true if the renderer should continue to render */ - public boolean renderDraw(List allStyleElems) { + public boolean renderDraw(Collection allStyleElems) { // nop return true; } @@ -74,7 +74,7 @@ } @Override - public boolean renderDraw(List allStyleElems) { + public boolean renderDraw(Collection allStyleElems) { timeSortingDone = System.currentTimeMillis(); return super.renderDraw(allStyleElems); } @@ -126,7 +126,7 @@ } @Override - public boolean renderDraw(List allStyleElems) { + public boolean renderDraw(Collection allStyleElems) { boolean res = super.renderDraw(allStyleElems); outStream.print("phase 1 (calculate styles): " + Utils.getDurationString(timeSortingDone - timeStart)); return res; Index: src/org/openstreetmap/josm/data/osm/visitor/paint/StyledMapRenderer.java =================================================================== --- src/org/openstreetmap/josm/data/osm/visitor/paint/StyledMapRenderer.java (revision 10743) +++ src/org/openstreetmap/josm/data/osm/visitor/paint/StyledMapRenderer.java (working copy) @@ -27,16 +27,16 @@ 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,7 +45,6 @@ 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,7 +53,6 @@ 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; @@ -61,7 +59,6 @@ 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,10 +70,10 @@ 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,13 +1776,8 @@ } } - private class ComputeStyleListWorker extends RecursiveTask> implements Visitor { - private final transient List input; - private final transient List 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); @@ -1792,96 +1784,33 @@ /** * 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 input, List output, int directExecutionTaskSize) { - this.input = input; - this.output = output; - this.directExecutionTaskSize = directExecutionTaskSize; + ComputeStyleListWorker() { this.styles.setDrawMultipolygon(drawMultipolygon); } - @Override - protected List compute() { - if (input.size() <= directExecutionTaskSize) { - return computeDirectly(); - } else { - final Collection>> tasks = new ArrayList<>(); - for (int fromIndex = 0; fromIndex < input.size(); fromIndex += directExecutionTaskSize) { - final int toIndex = Math.min(fromIndex + directExecutionTaskSize, input.size()); - final List output = new ArrayList<>(directExecutionTaskSize); - tasks.add(new ComputeStyleListWorker(input.subList(fromIndex, toIndex), output, directExecutionTaskSize).fork()); - } - for (ForkJoinTask> task : tasks) { - output.addAll(task.join()); - } - return output; - } + Stream computeNodeStyle(Node osm) { + final int flags = computeFlags(osm, false); + return StreamUtils.toStream(styles.get(osm, circum, nc)) + .map(s -> new StyleRecord(s, osm, flags)); } - public List 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(); - } + Stream 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)); } - @Override - public void visit(Node n) { - add(n, computeFlags(n, false)); + Stream 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)); } - - @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)); - } - } } /** @@ -1909,7 +1838,7 @@ List ways = data.searchWays(bbox); List relations = data.searchRelations(bbox); - final List allStyleElems = new ArrayList<>(nodes.size()+ways.size()+relations.size()); + final Collection allStyleElems; // Need to process all relations first. // Reason: Make sure, ElemStyles.getStyleCacheWithRange is @@ -1916,17 +1845,27 @@ // 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; } Index: test/performance/org/openstreetmap/josm/gui/mappaint/MapRendererPerformanceTest.java =================================================================== --- test/performance/org/openstreetmap/josm/gui/mappaint/MapRendererPerformanceTest.java (revision 10743) +++ test/performance/org/openstreetmap/josm/gui/mappaint/MapRendererPerformanceTest.java (working copy) @@ -8,6 +8,7 @@ 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,10 +332,10 @@ public static class BenchmarkData extends CapturingBenchmark { - private List allStyleElems; + private Collection allStyleElems; @Override - public boolean renderDraw(List allStyleElems) { + public boolean renderDraw(Collection allStyleElems) { this.allStyleElems = allStyleElems; return super.renderDraw(allStyleElems); }