Skip to content

Commit

Permalink
Fix #23908: Significantly improve the performance of copy/paste when …
Browse files Browse the repository at this point in the history
…dealing with large amounts of data

From #23908, at least one valid workflow involves copy/pasting large amounts of
data (specifically updating boundaries). The relation for `Terwolde` in the
sample data had 2206 objects. This took >5 minutes to copy between layers. With
this change, it takes <1 second.

This is a performance regression from r19176. The primary culprit from r19176 is
when we check the size of the dataset. The problem is that we check to see if the
size of the primitives being changed is greater than or equal to the non-deleted
complete primitives in the dataset. We get a  new filtered collection each time
we get those primitives, and therefore the size of that is not cached. The size
calculation for the filtered collection is where almost all the expense is. We
fix that by wrapping the work from AddPrimitivesCommand in `DataSet#update` to
have a single large update at the end of the copy operation. This ensures that
we do not have many spurious fired event calls when a mass operation is going on.

git-svn-id: https://josm.openstreetmap.de/svn/trunk@19214 0c6e7542-c601-0410-84e7-c038aed88b3b
  • Loading branch information
taylor.smock committed Sep 9, 2024
1 parent 8d42e3f commit 85b28a3
Showing 1 changed file with 8 additions and 11 deletions.
19 changes: 8 additions & 11 deletions src/org/openstreetmap/josm/command/AddPrimitivesCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
import java.util.Optional;
import java.util.stream.Collectors;

import javax.swing.Icon;

import org.openstreetmap.josm.data.osm.DataSet;
import org.openstreetmap.josm.data.osm.Node;
import org.openstreetmap.josm.data.osm.NodeData;
Expand Down Expand Up @@ -68,6 +66,14 @@ private void init(List<PrimitiveData> data, List<PrimitiveData> toSelect) {
@Override
public boolean executeCommand() {
DataSet ds = getAffectedDataSet();
ds.update(() -> this.executeRealCommand(ds));
if (toSelect != null) {
ds.setSelected(toSelect.stream().map(ds::getPrimitiveById).collect(Collectors.toList()));
}
return true;
}

private void executeRealCommand(DataSet ds) {
if (createdPrimitives == null) { // first time execution
List<OsmPrimitive> newPrimitives = new ArrayList<>(data.size());
preExistingData = new ArrayList<>();
Expand Down Expand Up @@ -109,10 +115,6 @@ public boolean executeCommand() {
}
}
}
if (toSelect != null) {
ds.setSelected(toSelect.stream().map(ds::getPrimitiveById).collect(Collectors.toList()));
}
return true;
}

@Override public void undoCommand() {
Expand Down Expand Up @@ -148,11 +150,6 @@ public String getDescriptionText() {
return trn("Added {0} object", "Added {0} objects", size, size);
}

@Override
public Icon getDescriptionIcon() {
return null;
}

@Override
public void fillModifiedData(Collection<OsmPrimitive> modified, Collection<OsmPrimitive> deleted,
Collection<OsmPrimitive> added) {
Expand Down

0 comments on commit 85b28a3

Please sign in to comment.