[Patch] Poor performance in delete action

[GerdP]

What steps will reproduce the problem?

1. use overpass to download all ways with disused:building=*
2. use search to select all ways tagged building=*, this selects ~ 12.000 ways
3. press Delete key

What is the expected result?

Quick reaction showing a popup You are about to delete nodes which can have other referrers not yet downloaded ...

What happens instead?

No reaction for a long time. At least one CPU is 100% busy.

Please provide any additional information below. Attach a screenshot if possible.

Stumbled over this while testing the patch for #19652.

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-10-03 13:42:38 +0200 (Sat, 03 Oct 2020)
Build-Date:2020-10-04 01:30:47
Revision:17084
Relative:URL: ^/trunk

Identification: JOSM/1.5 (17084 en) Windows 10 64-Bit
OS Build number: Windows 10 Home 2004 (19041)
Memory Usage: 964 MB / 3641 MB (785 MB allocated, but free)
Java version: 1.8.0_221-b11, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel
Screen: \Display0 1920x1080 (scaling 1.0x1.0)
Maximum Screen Size: 1920x1080
Best cursor sizes: 16x16 -> 32x32, 32x32 -> 32x32
VM arguments: [-XX:StartFlightRecording=name=MyRecording2,settings=d:\dbg\gerd.jfc, -XX:FlightRecorderOptions=defaultrecording=true,dumponexit=true,dumponexitpath=e:\ld\perf_20201006_094358.jfr]

Plugins:
+ OpeningHoursEditor (35414)
+ apache-commons (35524)
+ buildings_tools (35563)
+ ejml (35313)
+ geotools (35169)
+ gridify (1588746833)
+ jaxb (35092)
+ jts (35122)
+ merge-overlap (35248)
+ o5m (35248)
+ opendata (35513)
+ pbf (35446)
+ poly (35248)
+ reverter (35556)
+ undelete (35521)
+ utilsplugin2 (35487)

[GerdP]

The patch avoids a repeated sequential search when the flag IS_OUTSIDE is already set. The search is sequential because in this scenario the collection in ignore is a SubclassFilteredCollection which always performs a sequential search when method contains() is used.

[GerdP]

There are more performance problems in case that you press OK in the warning dialog. The 2nd patch fixes them as well:

1. A memory leak in this part of the code:

           for (Way w : waysToBeChanged) {
               Way wnew = new Way(w);
               wnew.removeNodes(nodesToRemove);
               if (wnew.getNodesCount() < 2) {
                   primitivesToDelete.add(w);
               } else {
                   cmds.add(new ChangeNodesCommand(w, wnew.getNodes()));
               }
           }
   

Since wnew is referenced by the nodes it is never garbage collected until the data layer is closed. I fear there are lots of leaks like this in JOSM.

2. A mass delete should not be processed object by object. The usage of Dataset.update() method improves performance drastically, esp. because it avoids frequent costly evaluations of Dataset.requiresUploadToServer()

[GerdP]

In 17102/josm:

fix #19881: Poor performance in delete action

• avoids a repeated sequential search when the flag IS_OUTSIDE is already set. The search is sequential because in this scenario the collection in ignore is a SubclassFilteredCollection which always performs a sequential search when method contains() is used.
• avoids to create change commands for ways which are in the collection of primitives which should be deleted
• use Dataset.update() in DeleteCommand

[Don-vip]

Milestone renamed