Changeset 11627 in josm for trunk/src/org/openstreetmap/josm/data

Timestamp:

2017-02-26T00:59:32+01:00 (7 years ago)

Author:

Don-vip

Message:

fix #3346 - improve drastically the performance of fixing duplicate nodes by:

• caching data sources area computation
• moving layer invalidation from UndoRedoHandler.addNoRedraw to UndoRedoHandler.add
• avoiding any EDT call when building tag conflict dialog if it's not meant to be displayed

Location:

trunk/src/org/openstreetmap/josm/data

Files:

• UndoRedoHandler.java (modified) (2 diffs)
• osm/DataSet.java (modified) (6 diffs)
• validation/tests/DuplicateNode.java (modified) (1 diff)
• validation/tests/DuplicateRelation.java (modified) (2 diffs)
• validation/tests/DuplicateWay.java (modified) (4 diffs)

trunk/src/org/openstreetmap/josm/data/UndoRedoHandler.java

@@ -52 +52 @@
         CheckParameterUtil.ensureParameterNotNull(c, "c");
         c.executeCommand();
-        c.invalidateAffectedLayers();
         commands.add(c);
         // Limit the number of commands in the undo list.
@@ -81 +80 @@
         }
         addNoRedraw(c);
+        c.invalidateAffectedLayers();
         afterAdd();
 

trunk/src/org/openstreetmap/josm/data/osm/DataSet.java

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
+import java.awt.geom.Area;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -25 +26 @@
 
 import org.openstreetmap.josm.Main;
+import org.openstreetmap.josm.data.Bounds;
 import org.openstreetmap.josm.data.Data;
 import org.openstreetmap.josm.data.DataSource;
@@ -127 +129 @@
     private final Object selectionLock = new Object();
 
+    private Area cachedDataSourceArea;
+    private List<Bounds> cachedDataSourceBounds;
+
     /**
      * Constructs a new {@code DataSet}.
@@ -184 +189 @@
             copyFrom.getReadLock().unlock();
         }
+    }
+
+    /**
+     * Adds a new data source.
+     * @param source data source to add
+     * @return {@code true} if the collection changed as a result of the call
+     * @since 11626
+     */
+    public synchronized boolean addDataSource(DataSource source) {
+        return addDataSources(Collections.singleton(source));
+    }
+
+    /**
+     * Adds new data sources.
+     * @param sources data sources to add
+     * @return {@code true} if the collection changed as a result of the call
+     * @since 11626
+     */
+    public synchronized boolean addDataSources(Collection<DataSource> sources) {
+        boolean changed = dataSources.addAll(sources);
+        if (changed) {
+            cachedDataSourceArea = null;
+            cachedDataSourceBounds = null;
+        }
+        return changed;
     }
 
@@ -917 +947 @@
             fireSelectionChanged();
         }
+    }
+
+    @Override
+    public synchronized Area getDataSourceArea() {
+        if (cachedDataSourceArea == null) {
+            cachedDataSourceArea = Data.super.getDataSourceArea();
+        }
+        return cachedDataSourceArea;
+    }
+
+    @Override
+    public synchronized List<Bounds> getDataSourceBounds() {
+        if (cachedDataSourceBounds == null) {
+            cachedDataSourceBounds = Data.super.getDataSourceBounds();
+        }
+        return Collections.unmodifiableList(cachedDataSourceBounds);
     }
 
@@ -1334 +1380 @@
      * @param progressMonitor The progress monitor
      */
-    public void mergeFrom(DataSet from, ProgressMonitor progressMonitor) {
+    public synchronized void mergeFrom(DataSet from, ProgressMonitor progressMonitor) {
         if (from != null) {
             new DataSetMerger(this, from).merge(progressMonitor);
-            dataSources.addAll(from.dataSources);
-            from.dataSources.clear();
+            if (!from.dataSources.isEmpty()) {
+                if (dataSources.addAll(from.dataSources)) {
+                    cachedDataSourceArea = null;
+                    cachedDataSourceBounds = null;
+                }
+                from.dataSources.clear();
+                from.cachedDataSourceArea = null;
+                from.cachedDataSourceBounds = null;
+            }
         }
     }

trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateNode.java

@@ -309 +309 @@
     @Override
     public Command fixError(TestError testError) {
-        if (!isFixable(testError)) return null;
         Collection<OsmPrimitive> sel = new LinkedList<>(testError.getPrimitives());
         Set<Node> nodes = new LinkedHashSet<>(OsmPrimitive.getFilteredList(sel, Node.class));

trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateRelation.java

@@ -293 +293 @@
         }
 
-        //Delete all relations in the list
+        // Delete all relations in the list
         relFix.remove(relationToKeep);
         commands.add(new DeleteCommand(relFix));
@@ -306 +306 @@
         // We fix it only if there is no more than one relation that is relation member.
         Collection<? extends OsmPrimitive> sel = testError.getPrimitives();
-        Set<Relation> relations = new HashSet<>();
+        Set<Relation> rels = new HashSet<>();
 
         for (OsmPrimitive osm : sel) {
             if (osm instanceof Relation) {
-                relations.add((Relation) osm);
-            }
-        }
-
-        if (relations.size() < 2)
+                rels.add((Relation) osm);
+            }
+        }
+
+        if (rels.size() < 2)
             return false;
 
         int relationsWithRelations = 0;
-        for (Relation w : relations) {
+        for (Relation w : rels) {
             List<Relation> rel = OsmPrimitive.getFilteredList(w.getReferrers(), Relation.class);
             if (!rel.isEmpty()) {

trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateWay.java

@@ -249 +249 @@
     public Command fixError(TestError testError) {
         Collection<? extends OsmPrimitive> sel = testError.getPrimitives();
-        Set<Way> ways = new HashSet<>();
+        Set<Way> wayz = new HashSet<>();
 
         for (OsmPrimitive osm : sel) {
             if (osm instanceof Way && !osm.isDeleted()) {
-                ways.add((Way) osm);
-            }
-        }
-
-        if (ways.size() < 2)
+                wayz.add((Way) osm);
+            }
+        }
+
+        if (wayz.size() < 2)
             return null;
 
         long idToKeep = 0;
-        Way wayToKeep = ways.iterator().next();
+        Way wayToKeep = wayz.iterator().next();
         // Find the way that is member of one or more relations. (If any)
         Way wayWithRelations = null;
         List<Relation> relations = null;
-        for (Way w : ways) {
+        for (Way w : wayz) {
             List<Relation> rel = OsmPrimitive.getFilteredList(w.getReferrers(), Relation.class);
             if (!rel.isEmpty()) {
@@ -284 +284 @@
 
         // Fix relations.
-        if (wayWithRelations != null && wayToKeep != wayWithRelations) {
+        if (wayWithRelations != null && relations != null && wayToKeep != wayWithRelations) {
             for (Relation rel : relations) {
                 Relation newRel = new Relation(rel);
@@ -297 +297 @@
         }
 
-        //Delete all ways in the list
-        //Note: nodes are not deleted, these can be detected and deleted at next pass
-        ways.remove(wayToKeep);
-        commands.add(new DeleteCommand(ways));
+        // Delete all ways in the list
+        // Note: nodes are not deleted, these can be detected and deleted at next pass
+        wayz.remove(wayToKeep);
+        commands.add(new DeleteCommand(wayz));
         return new SequenceCommand(tr("Delete duplicate ways"), commands);
     }
@@ -309 +309 @@
             return false;
 
-        //Do not automatically fix same ways with different tags
+        // Do not automatically fix same ways with different tags
         if (testError.getCode() != DUPLICATE_WAY) return false;
 
         // We fix it only if there is no more than one way that is relation member.
         Collection<? extends OsmPrimitive> sel = testError.getPrimitives();
-        Set<Way> ways = new HashSet<>();
+        Set<Way> wayz = new HashSet<>();
 
         for (OsmPrimitive osm : sel) {
             if (osm instanceof Way) {
-                ways.add((Way) osm);
-            }
-        }
-
-        if (ways.size() < 2)
+                wayz.add((Way) osm);
+            }
+        }
+
+        if (wayz.size() < 2)
             return false;
 
         int waysWithRelations = 0;
-        for (Way w : ways) {
+        for (Way w : wayz) {
             List<Relation> rel = OsmPrimitive.getFilteredList(w.getReferrers(), Relation.class);
             if (!rel.isEmpty()) {