Index: src/org/openstreetmap/josm/actions/CreateCircleAction.java =================================================================== --- src/org/openstreetmap/josm/actions/CreateCircleAction.java (revision 17090) +++ src/org/openstreetmap/josm/actions/CreateCircleAction.java (working copy) @@ -15,7 +15,7 @@ import javax.swing.JOptionPane; import org.openstreetmap.josm.command.AddCommand; -import org.openstreetmap.josm.command.ChangeCommand; +import org.openstreetmap.josm.command.ChangeNodesCommand; import org.openstreetmap.josm.command.Command; import org.openstreetmap.josm.command.SequenceCommand; import org.openstreetmap.josm.data.UndoRedoHandler; @@ -233,9 +233,7 @@ newWay.setNodes(nodesToAdd); cmds.add(new AddCommand(ds, newWay)); } else { - Way newWay = new Way(existingWay); - newWay.setNodes(nodesToAdd); - cmds.add(new ChangeCommand(ds, existingWay, newWay)); + cmds.add(new ChangeNodesCommand(ds, existingWay, nodesToAdd)); } UndoRedoHandler.getInstance().add(new SequenceCommand(tr("Create Circle"), cmds)); Index: src/org/openstreetmap/josm/actions/SimplifyWayAction.java =================================================================== --- src/org/openstreetmap/josm/actions/SimplifyWayAction.java (revision 17090) +++ src/org/openstreetmap/josm/actions/SimplifyWayAction.java (working copy) @@ -30,7 +30,7 @@ import javax.swing.event.ChangeEvent; import javax.swing.event.ChangeListener; -import org.openstreetmap.josm.command.ChangeCommand; +import org.openstreetmap.josm.command.ChangeNodesCommand; import org.openstreetmap.josm.command.Command; import org.openstreetmap.josm.command.DeleteCommand; import org.openstreetmap.josm.command.SequenceCommand; @@ -410,9 +410,7 @@ if (delNodes.isEmpty()) return null; Collection cmds = new LinkedList<>(); - Way newWay = new Way(w); - newWay.setNodes(newNodes); - cmds.add(new ChangeCommand(w, newWay)); + cmds.add(new ChangeNodesCommand(w, newNodes)); cmds.add(new DeleteCommand(w.getDataSet(), delNodes)); w.getDataSet().clearSelection(delNodes); return new SequenceCommand( Index: src/org/openstreetmap/josm/actions/corrector/TagCorrector.java =================================================================== --- src/org/openstreetmap/josm/actions/corrector/TagCorrector.java (revision 17090) +++ src/org/openstreetmap/josm/actions/corrector/TagCorrector.java (working copy) @@ -194,6 +194,8 @@ // save the clone if (!keysChanged.isEmpty()) { commands.add(new ChangeCommand(dataSet, primitive, clone)); + } else { + clone.removeFromChildren(); } } for (Entry> entry : roleCorrectionMap.entrySet()) { Index: src/org/openstreetmap/josm/actions/mapmode/ImproveWayAccuracyAction.java =================================================================== --- src/org/openstreetmap/josm/actions/mapmode/ImproveWayAccuracyAction.java (revision 17090) +++ src/org/openstreetmap/josm/actions/mapmode/ImproveWayAccuracyAction.java (working copy) @@ -489,6 +489,7 @@ if (deleteCmd != null) { UndoRedoHandler.getInstance().add(deleteCmd); } + newWay.setNodes(null); } else { UndoRedoHandler.getInstance().add(new ChangeCommand(targetWay, newWay)); } Index: src/org/openstreetmap/josm/actions/mapmode/SelectAction.java =================================================================== --- src/org/openstreetmap/josm/actions/mapmode/SelectAction.java (revision 17090) +++ src/org/openstreetmap/josm/actions/mapmode/SelectAction.java (working copy) @@ -22,7 +22,7 @@ import org.openstreetmap.josm.actions.MergeNodesAction; import org.openstreetmap.josm.command.AddCommand; -import org.openstreetmap.josm.command.ChangeCommand; +import org.openstreetmap.josm.command.ChangeNodesCommand; import org.openstreetmap.josm.command.Command; import org.openstreetmap.josm.command.MoveCommand; import org.openstreetmap.josm.command.RotateCommand; @@ -1243,7 +1243,8 @@ Way w = virtualWay.way; Way wnew = new Way(w); wnew.addNode(virtualWay.lowerIndex + 1, virtualNode); - virtualCmds.add(new ChangeCommand(ds, w, wnew)); + virtualCmds.add(new ChangeNodesCommand(ds, w, wnew.getNodes())); + wnew.removeFromChildren(); } virtualCmds.add(new MoveCommand(ds, virtualNode, startEN, currentEN)); String text = trn("Add and move a virtual new node to way", Index: src/org/openstreetmap/josm/command/Command.java =================================================================== --- src/org/openstreetmap/josm/command/Command.java (revision 17090) +++ src/org/openstreetmap/josm/command/Command.java (working copy) @@ -227,7 +227,7 @@ for (OsmPrimitive osm : primitives) { if (osm.isIncomplete()) { res |= IS_INCOMPLETE; - } else if ((osm.isOutsideDownloadArea() + } else if ((res & IS_OUTSIDE) == 0 && (osm.isOutsideDownloadArea() || (osm instanceof Node && !osm.isNew() && osm.getDataSet() != null && osm.getDataSet().getDataSourceBounds().isEmpty())) && (ignore == null || !ignore.contains(osm))) { res |= IS_OUTSIDE; Index: src/org/openstreetmap/josm/command/DeleteCommand.java =================================================================== --- src/org/openstreetmap/josm/command/DeleteCommand.java (revision 17090) +++ src/org/openstreetmap/josm/command/DeleteCommand.java (working copy) @@ -184,23 +184,25 @@ @Override public boolean executeCommand() { ensurePrimitivesAreInDataset(); - // Make copy and remove all references (to prevent inconsistent dataset (delete referenced) while command is executed) - for (OsmPrimitive osm: toDelete) { - if (osm.isDeleted()) - throw new IllegalArgumentException(osm + " is already deleted"); - clonedPrimitives.put(osm, osm.save()); - if (osm instanceof Way) { - ((Way) osm).setNodes(null); - } else if (osm instanceof Relation) { - ((Relation) osm).setMembers(null); + getAffectedDataSet().update(() -> { + // Make copy and remove all references (to prevent inconsistent dataset (delete referenced) while command is executed) + for (OsmPrimitive osm : toDelete) { + if (osm.isDeleted()) + throw new IllegalArgumentException(osm + " is already deleted"); + clonedPrimitives.put(osm, osm.save()); + + if (osm instanceof Way) { + ((Way) osm).setNodes(null); + } else if (osm instanceof Relation) { + ((Relation) osm).setMembers(null); + } } - } - for (OsmPrimitive osm: toDelete) { - osm.setDeleted(true); - } - + for (OsmPrimitive osm : toDelete) { + osm.setDeleted(true); + } + }); return true; } @@ -208,13 +210,15 @@ public void undoCommand() { ensurePrimitivesAreInDataset(); - for (OsmPrimitive osm: toDelete) { - osm.setDeleted(false); - } + getAffectedDataSet().update(() -> { + for (OsmPrimitive osm : toDelete) { + osm.setDeleted(false); + } - for (Entry entry: clonedPrimitives.entrySet()) { - entry.getKey().load(entry.getValue()); - } + for (Entry entry : clonedPrimitives.entrySet()) { + entry.getKey().load(entry.getValue()); + } + }); } @Override @@ -426,12 +430,13 @@ Collection cmds = new LinkedList<>(); Set nodesToRemove = new HashSet<>(Utils.filteredCollection(primitivesToDelete, Node.class)); for (Way w : waysToBeChanged) { - Way wnew = new Way(w); - wnew.removeNodes(nodesToRemove); - if (wnew.getNodesCount() < 2) { + if (primitivesToDelete.contains(w)) + continue; + List remainingNodes = w.calculateRemoveNodes(nodesToRemove); + if (remainingNodes.size() < 2) { primitivesToDelete.add(w); } else { - cmds.add(new ChangeNodesCommand(w, wnew.getNodes())); + cmds.add(new ChangeNodesCommand(w, remainingNodes)); } } Index: src/org/openstreetmap/josm/data/osm/DatasetConsistencyTest.java =================================================================== --- src/org/openstreetmap/josm/data/osm/DatasetConsistencyTest.java (revision 17090) +++ src/org/openstreetmap/josm/data/osm/DatasetConsistencyTest.java (working copy) @@ -68,7 +68,7 @@ } /** - * Checks for womplete ways with incomplete nodes. + * Checks for complete ways with incomplete nodes. */ public void checkCompleteWaysWithIncompleteNodes() { final Stopwatch stopwatch = Stopwatch.createStarted(); @@ -182,6 +182,20 @@ printElapsedTime(stopwatch); } + private void searchParentsWithoutDataset() { + final Stopwatch stopwatch = Stopwatch.createStarted(); + long dubiousParents = 0; + for (OsmPrimitive p : dataSet.allPrimitives()) { + dubiousParents += p.checkReferrersWithoutDataset(false); + } + if (dubiousParents > 0) { + printError("WARN - Memory leak", + "Dataset contains %d pointers to parents which do not belong to the dataset", dubiousParents); + } + printElapsedTime(stopwatch); + + } + private void printElapsedTime(Stopwatch stopwatch) { if (Logging.isDebugEnabled()) { StackTraceElement item = Thread.currentThread().getStackTrace()[2]; @@ -203,6 +217,7 @@ searchNodes(); searchWays(); checkZeroNodesWays(); + searchParentsWithoutDataset(); printElapsedTime(stopwatch); if (errorCount > MAX_ERRORS) { writer.println((errorCount - MAX_ERRORS) + " more..."); Index: src/org/openstreetmap/josm/data/osm/Node.java =================================================================== --- src/org/openstreetmap/josm/data/osm/Node.java (revision 17090) +++ src/org/openstreetmap/josm/data/osm/Node.java (working copy) @@ -394,4 +394,9 @@ protected void updateDirectionFlags() { // Nodes do not need/have a direction, greatly improves performance, see #18886 } + + @Override + public void removeFromChildren() { + // nothing to do + } } Index: src/org/openstreetmap/josm/data/osm/OsmPrimitive.java =================================================================== --- src/org/openstreetmap/josm/data/osm/OsmPrimitive.java (revision 17090) +++ src/org/openstreetmap/josm/data/osm/OsmPrimitive.java (working copy) @@ -787,6 +787,44 @@ } /** + * Find all referrers which have no dataset. + * @param doRemove if {@code true} remove the found referrers + * @return number of (possibly removed) referrers without a dataset + */ + final int checkReferrersWithoutDataset(boolean doRemove) { + if (dataSet == null) { + return 0; + } + if (doRemove) + checkDatasetNotReadOnly(); + + if (referrers == null) { + return 0; + } + if (referrers instanceof OsmPrimitive) { + if (((OsmPrimitive) referrers).getDataSet() == null) { + if (doRemove) + referrers = null; + return 1; + } + return 0; + } + OsmPrimitive[] array = ((OsmPrimitive[]) referrers); + long oldCount = array.length; + long badCount = Arrays.stream(array).filter(p -> p.getDataSet() == null).count(); + if (badCount > 0 && doRemove) { + OsmPrimitive[] smaller = new OsmPrimitive[(int) (oldCount - badCount)]; + int k = 0; + for (int i = 0; i < oldCount; i++) { + if (array[i].getDataSet() != null) + smaller[k++] = array[i]; + } + referrers = smaller; + } + return (int) badCount; + } + + /** *

Visits {@code visitor} for all referrers.

* * @param visitor the visitor. Ignored, if null. @@ -1146,4 +1184,10 @@ * @since 11269 */ protected abstract void addToBBox(BBox box, Set visited); + + /** + * If this primitive has children, + * @since xxx + */ + public abstract void removeFromChildren(); } Index: src/org/openstreetmap/josm/data/osm/Relation.java =================================================================== --- src/org/openstreetmap/josm/data/osm/Relation.java (revision 17090) +++ src/org/openstreetmap/josm/data/osm/Relation.java (working copy) @@ -563,4 +563,10 @@ public UniqueIdGenerator getIdGenerator() { return idGenerator; } + + @Override + public void removeFromChildren() { + setMembers(null); + } + } Index: src/org/openstreetmap/josm/data/osm/Way.java =================================================================== --- src/org/openstreetmap/josm/data/osm/Way.java (revision 17090) +++ src/org/openstreetmap/josm/data/osm/Way.java (working copy) @@ -363,18 +363,7 @@ if (selection == null || isIncomplete()) return; boolean locked = writeLock(); try { - boolean closed = isClosed() && selection.contains(lastNode()); - List copy = Arrays.stream(nodes) - .filter(n -> !selection.contains(n)) - .collect(Collectors.toList()); - - int i = copy.size(); - if (closed && i > 2) { - copy.add(copy.get(0)); - } else if (i >= 2 && i <= 3 && copy.get(0) == copy.get(i-1)) { - copy.remove(i-1); - } - setNodes(removeDouble(copy)); + setNodes(calculateRemoveNodes(selection)); for (Node n : selection) { n.clearCachedStyle(); } @@ -384,6 +373,29 @@ } /** + * Calculate the remaining nodes after a removal of the given set of {@link Node nodes} from this way. + * @param selection The selection of nodes to remove. Ignored, if null + * @return result of the removal, can be empty + * @since xxx + */ + public List calculateRemoveNodes(Set selection) { + if (selection == null || isIncomplete()) + return getNodes(); + boolean closed = isClosed() && selection.contains(lastNode()); + List copy = Arrays.stream(nodes) + .filter(n -> !selection.contains(n)) + .collect(Collectors.toList()); + + int i = copy.size(); + if (closed && i > 2) { + copy.add(copy.get(0)); + } else if (i >= 2 && i <= 3 && copy.get(0) == copy.get(i-1)) { + copy.remove(i-1); + } + return removeDouble(copy); + } + + /** * Adds a node to the end of the list of nodes. Ignored, if n is null. * * @param n the node. Ignored, if null @@ -756,4 +768,10 @@ public UniqueIdGenerator getIdGenerator() { return idGenerator; } + + @Override + public void removeFromChildren() { + setNodes(null); + } + } Index: src/org/openstreetmap/josm/data/osm/visitor/paint/AbstractMapRenderer.java =================================================================== --- src/org/openstreetmap/josm/data/osm/visitor/paint/AbstractMapRenderer.java (revision 17090) +++ src/org/openstreetmap/josm/data/osm/visitor/paint/AbstractMapRenderer.java (working copy) @@ -12,6 +12,7 @@ import org.openstreetmap.josm.data.osm.INode; import org.openstreetmap.josm.data.osm.IWay; import org.openstreetmap.josm.data.osm.OsmData; +import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.data.osm.WaySegment; import org.openstreetmap.josm.gui.MapViewState; import org.openstreetmap.josm.gui.MapViewState.MapViewPoint; @@ -148,7 +149,9 @@ path = new GeneralPath(); for (WaySegment wseg: data.getHighlightedVirtualNodes()) { if (wseg.way.isUsable() && !wseg.way.isDisabled()) { - visitVirtual(path, wseg.toWay()); + Way tmpWay = wseg.toWay(); + visitVirtual(path, tmpWay); + tmpWay.removeFromChildren(); } } g.setColor(highlightColor); Index: src/org/openstreetmap/josm/data/validation/tests/Coastlines.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/Coastlines.java (revision 17090) +++ src/org/openstreetmap/josm/data/validation/tests/Coastlines.java (working copy) @@ -13,7 +13,7 @@ import java.util.List; import java.util.Set; -import org.openstreetmap.josm.command.ChangeCommand; +import org.openstreetmap.josm.command.ChangeNodesCommand; import org.openstreetmap.josm.command.Command; import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.OsmPrimitive; @@ -260,13 +260,11 @@ Iterator it = testError.getPrimitives().iterator(); if (it.hasNext()) { Way way = (Way) it.next(); - Way newWay = new Way(way); - List nodesCopy = newWay.getNodes(); + List nodesCopy = way.getNodes(); Collections.reverse(nodesCopy); - newWay.setNodes(nodesCopy); - return new ChangeCommand(way, newWay); + return new ChangeNodesCommand(way, nodesCopy); } } return null; Index: src/org/openstreetmap/josm/data/validation/tests/TagChecker.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/TagChecker.java (revision 17090) +++ src/org/openstreetmap/josm/data/validation/tests/TagChecker.java (working copy) @@ -987,6 +987,7 @@ if (countDeprecated(clone) > unchangedDeprecated) iter.remove(); } + clone.removeFromChildren(); } private int countDeprecated(OsmPrimitive p) { Index: src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java =================================================================== --- src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java (revision 17090) +++ src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java (working copy) @@ -950,7 +950,11 @@ relation.addMember(new RelationMember(roles.size() == 1 ? roles.iterator().next() : "", p)); modified = true; } - return modified ? new ChangeCommand(orig, relation) : null; + if (!modified) { + relation.removeFromChildren(); + return null; + } + return new ChangeCommand(orig, relation); } catch (AddAbortException ign) { Logging.trace(ign); return null;