Modify

Opened 3 years ago

Closed 3 years ago

Last modified 3 years 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 3 years ago.
19881.2.patch (6.1 KB ) - added by GerdP 3 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 by GerdP, 3 years ago

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 by GerdP, 3 years ago

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

by GerdP, 3 years ago

Attachment: 19881.patch added

by GerdP, 3 years ago

Attachment: 19881.2.patch added

comment:3 by GerdP, 3 years ago

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 by GerdP, 3 years ago

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 by Don-vip, 3 years ago

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. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.