Modify

Opened 14 months ago

Closed 14 months ago

Last modified 6 weeks ago

#16102 closed enhancement (fixed)

kml import is very slow compared to gpx import

Reported by: GerdP Owned by: Don-vip
Priority: normal Milestone:
Component: Plugin opendata Version:
Keywords: performance kml Cc:

Description

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.

Attachments (4)

kml.patch (3.0 KB) - added by GerdP 14 months ago.
kml-v2.patch (3.2 KB) - added by GerdP 14 months ago.
GpsiesTrack.kmz (310.7 KB) - added by GerdP 14 months ago.
simplifyWay-performance.patch (1.7 KB) - added by GerdP 14 months ago.

Download all attachments as: .zip

Change History (14)

Changed 14 months ago by GerdP

Attachment: kml.patch added

Changed 14 months ago by GerdP

Attachment: kml-v2.patch added

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

comment:2 Changed 14 months ago by 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.

comment:3 in reply to:  2 Changed 14 months ago by 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?

Changed 14 months ago by GerdP

Attachment: GpsiesTrack.kmz added

comment:4 Changed 14 months ago by 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.

Changed 14 months ago by GerdP

comment:5 Changed 14 months ago by 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.

comment:6 Changed 14 months ago by Don-vip

In 13540/josm:

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

comment:7 Changed 14 months ago by Don-vip

Resolution: fixed
Status: newclosed

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

comment:8 Changed 6 weeks ago by 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.

comment:9 Changed 6 weeks ago by Don-vip

Nice!

comment:10 Changed 6 weeks ago by 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)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Don-vip.
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.