Opened 21 months ago
Closed 19 months ago
#22808 closed enhancement (fixed)
[Patch] Undoing "Paste" for ways of a route relation is very slow
Reported by: | GerdP | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | 23.05 |
Component: | Core | Version: | |
Keywords: | template_report performance | Cc: |
Description
What steps will reproduce the problem?
- Download members of a route relation, e.g. 14974348 to an empty layer
- Select all way members, presss Ctrl+C to copy
- press Ctrl+N to create new data layer
- press Ctrl+V to paste the ways as new objects
- undo
What is the expected result?
The action is undone within a fraction of a second
What happens instead?
CPU usage for one core is 100% for some seconds. The more ways the longer it takes.
Please provide any additional information below. Attach a screenshot if possible.
Noticed that while trying to create a simplified GPX for the relation.
Relative:URL: ^/trunk Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b Last:Changed Date: 2023-03-01 15:17:42 +0100 (Wed, 01 Mar 2023) Revision:18678 Build-Date:2023-03-02 02:30:57 URL:https://josm.openstreetmap.de/svn/trunk Identification: JOSM/1.5 (18678 en) Windows 10 64-Bit OS Build number: Windows 10 Home 2009 (19045) Memory Usage: 554 MB / 1972 MB (287 MB allocated, but free) Java version: 17.0.4+8-LTS, Azul Systems, Inc., OpenJDK 64-Bit Server VM Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel Screen: \Display0 1920×1080 (scaling 1.00×1.00) Maximum Screen Size: 1920×1080 Best cursor sizes: 16×16→32×32, 32×32→32×32 System property file.encoding: Cp1252 System property sun.jnu.encoding: Cp1252 Locale info: en_DE Numbers with default locale: 1234567890 -> 1234567890 VM arguments: [-Djpackage.app-version=1.5.18531, --add-modules=java.scripting,java.sql,javafx.controls,javafx.media,javafx.swing,javafx.web, --add-exports=java.base/sun.security.action=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.plugins.jpeg=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.spi=ALL-UNNAMED, --add-opens=java.base/java.lang=ALL-UNNAMED, --add-opens=java.base/java.nio=ALL-UNNAMED, --add-opens=java.base/jdk.internal.loader=ALL-UNNAMED, --add-opens=java.base/jdk.internal.ref=ALL-UNNAMED, --add-opens=java.desktop/javax.imageio.spi=ALL-UNNAMED, --add-opens=java.desktop/javax.swing.text.html=ALL-UNNAMED, --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED, -Djpackage.app-path=%UserProfile%\AppData\Local\JOSM\HWConsole.exe] Plugins: + OpeningHoursEditor (35924) + RoadSigns (36038) + apache-commons (36034) + buildings_tools (36011) + contourmerge (v0.1.9) + ejml (35924) + geotools (36028) + jackson (36034) + jaxb (35952) + jts (36004) + o5m (35893) + opendata (36025) + pbf (36034) + poly (35976) + reltoolbox (35976) + reverter (36043) + undelete (36011) + utilsplugin2 (36011) Validator rules: + c:\josm\core\resources\data\validator\combinations.mapcss + c:\josm\core\resources\data\validator\geometry.mapcss + c:\josm\core\resources\data\validator\unnecessary.mapcss + d:\java_tools\JOSM\mygeometry.mapcss + https://josm.openstreetmap.de/josmfile?page=Rules/GermanySpecific&zip=1 Last errors/warnings: - 00000.667 W: extended font config - overriding 'filename.Myanmar_Text=mmrtext.ttf' with 'MMRTEXT.TTF' - 00000.669 W: extended font config - overriding 'filename.Mongolian_Baiti=monbaiti.ttf' with 'MONBAITI.TTF' - 00001.251 E: java.security.KeyStoreException: Windows-ROOT not found. Cause: java.security.NoSuchAlgorithmException: Windows-ROOT KeyStore not available
Attachments (5)
Change History (14)
by , 21 months ago
Attachment: | 22808-screen.JPG added |
---|
comment:1 by , 21 months ago
Obviously the problem is that the just pasted ways are selected and the undo action modifies this selection for each pasted object.
When you clear the selction before using undo the performacnce is good. Maybe org.openstreetmap.josm.command.AddPrimitivesCommand.undoCommand()
could be changed to clear the selection first and calculate a new selction at the end?
by , 21 months ago
Attachment: | 22808.patch added |
---|
quick hack to work around the update selection problem
comment:2 by , 21 months ago
Summary: | Undoing "Paste" for ways of a route relation is very slow → [Patch] Undoing "Paste" for ways of a route relation is very slow |
---|
The simple patch fixes the problem in the described scenario. No idea how many other actions suffer from the same problem when lots of objects are removed with a serious of calls of org.openstreetmap.josm.data.osm.DataSet.removePrimitive(PrimitiveId primitiveId)
.
comment:3 by , 20 months ago
I'd rather add a bulk removal call, e.g. removePrimitives(Collection<PrimitiveId> primitiveIds)
.
by , 20 months ago
Attachment: | 22808.2.patch added |
---|
follow-up: 5 comment:4 by , 20 months ago
Patch looks wrong to me:
- sounds like ds.beginUpdate() and ds.endUpdate() should be used instead
also sounds like a SequenceCommand() should be used to create only one Undo/Redo entry (which probably makes point 1 unnecessary :-)
comment:5 by , 20 months ago
Replying to stoecker:
Patch looks wrong to me:
- sounds like ds.beginUpdate() and ds.endUpdate() should be used instead
Unfortunately, no. The problem lies in the removePrimitiveImpl
-- it calls clearSelection
, which fires an event immediately.
My patch is still wrong -- I cannot use the removePrimitiveImpl
since it does call clearSelection
.
by , 20 months ago
Attachment: | 22808.3.patch added |
---|
Avoid calling clearSelection
for each removed primitive in bulk removal methods
comment:8 by , 20 months ago
Milestone: | → 23.05 |
---|
Replying to stoecker:
Multiple typos "from"/"form"... ;-)
That is what I get for copying and pasting the docs from removePrimitive
...
Replying to GerdP
The patch works well for me.
Excellent. It looks like other locations in the code that removes objects in loops do the same thing you suggested in attachment:22808.patch.
screenshot from visual-vm monitoring the mentioned action