kml import is very slow compared to gpx import

[GerdP]

I've got a kml file and a gpx file, both describe the same track.
I've noted that it takes much longer to load the kml file into JOSM (many seconds).
I think the reason is that KmlReader.java uses
way.addNode(node);
for each single node instead of adding all nodes to a List and calling
way.setNodes(list)
once the data is complete.
Each addNode() triggers the complex org.openstreetmap.josm.data.osm.Way.fireNodesChanged().

Attached is a patch which might work. I have problems compiling the plugin, so I cannot test the patch.

[GerdP]

I was finally able to compile the plugin and found some errors. 2nd version works, is much faster compared to the original code,
but still much slower compared to the gpx file import.
Numbers:
My files contain ~41000 points in a single way.
Loading the gpx takes ~ 1 sec, loading the kml with the patch takes 15 secs, without the patch ~60 secs.

I'll have a closer look at the code that imports gpx tomorrow.

[GerdP]

Was too curious...
The rest of the time is used for SimplifyWayAction. When I comment the lines in DataSetUpdater.java
the kml file is also imported in ~ 1 sec.
Why this is this method called? I think it should not be done or at least it should be optional.

[Don-vip]

Replying to GerdP:

Why this is this method called?

This is necessary for some files having way too much points... Can you please attach your file?

[GerdP]

Done. I also wonder why SimplifyWayAction needs so much time for this. It doesn't seem to be this specific data set that is so slow,
I see it with all kml / kmz files.

[GerdP]

I've attached simplifyWay-performance.patch which addresses two problems in SimplifyWayAction:
1) the method isRequiredNode always does the most complex action first (count the occurence of a node)
while the simple check for tags is done last. The patch reverses that order.
This has nearly no effect for nodes without tags but is much faster when the node has tags.
2) The case that SimplifyWay did not find a node to remove can be checked by comparing the number of nodes in the ways.
This is much faster than the addAll() + removeAll() combination that is done now.

With this additional patch the kml import is nearly as fast as gpx, and I see no negative impact when I use
Shift+Y on a large collection of ways.

[Don-vip]

In 13540/josm:

see #16102 - SimplifyWayAction performance improvements (patch by GerdP)

[Don-vip]

Fixed in [o34089:34090]. Thanks for the patches! :)

[GerdP]

In 14970/josm:

see #16102: Drastically improve performance for SimplifyWayAction when used with very complex ways.
For a kml file produced by brouter with ~50000 points the import took > 50 seconds, now it is done in 1.5 seconds.

[Don-vip]

Nice!

[GerdP]

I've tried to run the unit test but it doesn't seem to work on my machine :(
Three of four tests fail. A part of the log shows

Testcase: testMoreThanTenWays took 0,406 sec
	Caused an ERROR
java.lang.reflect.InvocationTargetException
ReportedException [thread=Thread[Timeout runner,5,], exception=java.lang.reflect.InvocationTargetException, methodWarningFrom=null]
	at org.openstreetmap.josm.tools.bugreport.BugReport.intercept(BugReport.java:173)
	at org.openstreetmap.josm.gui.util.GuiHelper.runInEDTAndWaitWithException(GuiHelper.java:243)
	at org.openstreetmap.josm.gui.layer.LayerManager.removeLayer(LayerManager.java:248)
	at org.openstreetmap.josm.actions.SimplifyWayActionTest.testMoreThanTenWays(SimplifyWayActionTest.java:115)
	at org.openstreetmap.josm.testutils.JOSMTestRules$TimeoutThread.run(JOSMTestRules.java:708)
Caused by: java.lang.reflect.InvocationTargetException
	at java.awt.EventQueue.invokeAndWait(EventQueue.java:1349)
	at java.awt.EventQueue.invokeAndWait(EventQueue.java:1324)
	at javax.swing.SwingUtilities.invokeAndWait(SwingUtilities.java:1353)
	at org.openstreetmap.josm.gui.util.GuiHelper.runInEDTAndWaitWithException(GuiHelper.java:241)
Caused by: java.lang.IllegalArgumentException: OsmDataLayer [name=, associatedFile=null] is not managed by us.
	at org.openstreetmap.josm.gui.layer.LayerManager.checkContainsLayer(LayerManager.java:393)
	at org.openstreetmap.josm.gui.layer.LayerManager.realRemoveLayer(LayerManager.java:264)
	at org.openstreetmap.josm.gui.layer.LayerManager.lambda$removeLayer$1(LayerManager.java:248)
	at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:301)
	at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:758)
	at java.awt.EventQueue.access$500(EventQueue.java:97)
	at java.awt.EventQueue$3.run(EventQueue.java:709)
	at java.awt.EventQueue$3.run(EventQueue.java:703)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:728)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:205)
	at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
	at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)