Modify

Opened 6 years ago

Closed 6 years ago

Last modified 5 years 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 6 years ago.
kml-v2.patch (3.2 KB ) - added by GerdP 6 years ago.
GpsiesTrack.kmz (310.7 KB ) - added by GerdP 6 years ago.
simplifyWay-performance.patch (1.7 KB ) - added by GerdP 6 years ago.

Download all attachments as: .zip

Change History (14)

by GerdP, 6 years ago

Attachment: kml.patch added

by GerdP, 6 years ago

Attachment: kml-v2.patch added

comment:1 by GerdP, 6 years ago

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

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.

in reply to:  2 comment:3 by Don-vip, 6 years ago

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?

by GerdP, 6 years ago

Attachment: GpsiesTrack.kmz added

comment:4 by GerdP, 6 years ago

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.

by GerdP, 6 years ago

comment:5 by GerdP, 6 years ago

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

In 13540/josm:

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

comment:7 by Don-vip, 6 years ago

Resolution: fixed
Status: newclosed

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

comment:8 by GerdP, 5 years ago

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

Nice!

comment:10 by GerdP, 5 years ago

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. 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.