Modify

Opened 21 months ago

Closed 21 months ago

Last modified 19 months ago

#19881 closed enhancement (fixed)

[Patch] Poor performance in delete action

Reported by: GerdP Owned by: GerdP
Priority: normal Milestone: 20.11
Component: Core Version:
Keywords: template_report performance Cc:

Description

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)

Attachments (2)

19881.patch (798 bytes) - added by GerdP 21 months ago.
19881.2.patch (6.1 KB) - added by GerdP 21 months ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 21 months ago by 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.

comment:2 Changed 21 months ago by GerdP

Milestone: 20.10
Owner: changed from team to GerdP
Status: newassigned

Changed 21 months ago by GerdP

Attachment: 19881.patch added

Changed 21 months ago by GerdP

Attachment: 19881.2.patch added

comment:3 Changed 21 months ago by 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.

  1. 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()

comment:4 Changed 21 months ago by GerdP

Resolution: fixed
Status: assignedclosed

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

comment:5 Changed 19 months ago by Don-vip

Milestone: 20.1020.11

Milestone renamed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain GerdP.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.