Modify

Opened 12 years ago

Closed 12 years ago

#6987 closed defect (fixed)

Crashing on osm file open

Reported by: alex-kostjukov@… Owned by: team
Priority: normal Milestone:
Component: Core Version:
Keywords: Cc:

Description (last modified by Don-vip)

Repository Root: http://josm.openstreetmap.de/svn
Build-Date: 2011-10-04 01:31:24
Last Changed Author: bastiK
Revision: 4487
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
URL: http://josm.openstreetmap.de/svn/trunk
Last Changed Date: 2011-10-03 22:52:38 +0200 (Mon, 03 Oct 2011)
Last Changed Rev: 4487

Identification: JOSM/1.5 (4487 en)
Memory Usage: 614 MB / 1820 MB (430 MB allocated, but free)
Java version: 1.7.0, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Operating system: Linux
Dataset consistency test: No problems found


java.lang.NullPointerException
	at org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon.joinWays(Multipolygon.java:384)
	at org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon.createPolygons(Multipolygon.java:292)
	at org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon.load(Multipolygon.java:276)
	at org.openstreetmap.josm.gui.mappaint.ElemStyles.getImpl(ElemStyles.java:163)
	at org.openstreetmap.josm.gui.mappaint.ElemStyles.getStyleCacheWithRange(ElemStyles.java:70)
	at org.openstreetmap.josm.gui.mappaint.ElemStyles.get(ElemStyles.java:53)
	at org.openstreetmap.josm.data.osm.visitor.paint.StyledMapRenderer$StyleCollector.add(StyledMapRenderer.java:117)
	at org.openstreetmap.josm.data.osm.visitor.paint.StyledMapRenderer.collectWayStyles(StyledMapRenderer.java:178)
	at org.openstreetmap.josm.data.osm.visitor.paint.StyledMapRenderer.render(StyledMapRenderer.java:230)
	at org.openstreetmap.josm.gui.layer.OsmDataLayer.paint(OsmDataLayer.java:259)
	at org.openstreetmap.josm.gui.MapView.paintLayer(MapView.java:451)
	at org.openstreetmap.josm.gui.MapView.paint(MapView.java:505)
	at javax.swing.JComponent.paintChildren(JComponent.java:887)
	at javax.swing.JSplitPane.paintChildren(JSplitPane.java:1047)
	at javax.swing.JComponent.paint(JComponent.java:1063)
	at javax.swing.JComponent.paintToOffscreen(JComponent.java:5221)
	at javax.swing.BufferStrategyPaintManager.paint(BufferStrategyPaintManager.java:295)
	at javax.swing.RepaintManager.paint(RepaintManager.java:1206)
	at javax.swing.JComponent._paintImmediately(JComponent.java:5169)
	at javax.swing.JComponent.paintImmediately(JComponent.java:4980)
	at javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:770)
	at javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:728)
	at javax.swing.RepaintManager.prePaintDirtyRegions(RepaintManager.java:677)
	at javax.swing.RepaintManager.access$700(RepaintManager.java:59)
	at javax.swing.RepaintManager$ProcessingRunnable.run(RepaintManager.java:1621)
	at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:251)
	at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:705)
	at java.awt.EventQueue.access$000(EventQueue.java:101)
	at java.awt.EventQueue$3.run(EventQueue.java:666)
	at java.awt.EventQueue$3.run(EventQueue.java:664)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:76)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:675)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:211)
	at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:128)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:117)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:113)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:105)
	at java.awt.EventDispatchThread.run(EventDispatchThread.java:90)

Attachments (0)

Change History (22)

comment:1 by bastiK, 12 years ago

I suppose it happens every time you open that file? Can you attach it?

comment:2 by Don-vip, 12 years ago

Description: modified (diff)

comment:3 by anonymous, 12 years ago

It doesn't happen when it doesn't try to render all the file.
After something is already open and positioned outside the problem file data area it opens file well. When you pan to the data of the problem file, on some step it throws that exception.
When it is 1st file being opened and it tries to render it all, it always throws.
JOSM-latest just hangs in infinite loop at that file. As near as I can tell, in the method that determines if something is within polygon.
File is 6mb bzipped. It doesn't allow me to attach.
It can be downloaded from http://narod.ru/disk/29115129001/lao.osm.bz2.html

comment:4 by Don-vip, 12 years ago

It's not really an infinite loop, but a huge performance problem when drawing members of relation 1391969.
This relation appears to be an enormous multipolygon of 917 members. The OSM website has trouble to display information about it: http://www.openstreetmap.org/browse/relation/1391969. The API itself is particularly slow to respond to api.openstreetmap.org/api/0.6/relation/1391969.

The main JOSM statement causing this performance problem seems to be http://josm.openstreetmap.de/browser/josm/trunk/src/org/openstreetmap/josm/data/osm/visitor/paint/StyledMapRenderer.java?rev=4327#L117

comment:5 by Don-vip, 12 years ago

Resolution: fixed
Status: newclosed

In [4621/josm]:

fix #6987 - Crashing on osm file open -> Fix a performance problem with very large multipolygons loaded thousands of times

comment:6 by bastiK, 12 years ago

Resolution: fixed
Status: closedreopened

Great, I always wondered, why this would not be a performance killer.

Do you care for updates, when MP gets changed? Also, I suppose there can be 2 (different) relations with same id on 2 data layers. Currently they'd share a cache value.

comment:7 by Don-vip, 12 years ago

Currently not, I have to check that tonight.

comment:8 by Don-vip, 12 years ago

In [4623/josm]:

see #6987 - multipolygon cache aware of events requiring cache refresh (data change, zoom, layer removing)

comment:9 by bastiK, 12 years ago

Why clear for zoom change, but not for projection change? Why not use cache for MapPainter?

Last edited 12 years ago by bastiK (previous) (diff)

comment:10 by Don-vip, 12 years ago

In [4624/josm]:

see #6987 - multipolygon cache aware of projection change

in reply to:  9 ; comment:11 by Don-vip, 12 years ago

Replying to bastiK:

Why clear for zoom change, but not for projection change? Why not use cache for MapPainter?

Because I didn't think of the projection change, fixed :) MapPainter is already using the cache in r4623, is there a specific thing I haven't seen ?

in reply to:  11 ; comment:12 by bastiK, 12 years ago

Replying to Don-vip:

Replying to bastiK:

Why clear for zoom change, but not for projection change? Why not use cache for MapPainter?

MapPainter is already using the cache in r4623, is there a specific thing I haven't seen ?

Yes, my bad.

Is it really necessary to clear the cache on zoom?

Multipolygon.java is quite a memory hog with all the node coordinates duplicated in PolyData. In principle it should be enough to save the identification of inner and outer ways. But it's probably no problem, because the average OSM data set has relatively few MPs; so I think this optimization is low priority.

comment:13 by Don-vip, 12 years ago

In [4626/josm]:

see #7101 and #6987 - Avoid Data consistency error in multipolygon cache after undo/redo

in reply to:  12 comment:14 by Don-vip, 12 years ago

Replying to bastiK:

Is it really necessary to clear the cache on zoom?

I had to do something after letting MapPainter use the MP cache. If MapPainter uses MP cache without clearing it on zoom change, the MP is not drawn correctly. If it's a problem, I can always go back and remove the call cache in MapPainter, after all, I do not have seen specific performance gains with it.

comment:15 by bastiK, 12 years ago

You could remove the dependency of Multipolygon class on NavigatableComponent by doing all calculations in EastNorth coords. Then, in MapPainter copy the Polygons and convert to screen coordinates.

But I think it's alright the way it's now, not much to gain here.

Btw., ZoomChangeListener from NavigatableComponent is a misnomer, should be called ViewportChangeListener or something like this.

in reply to:  15 comment:16 by Don-vip, 12 years ago

Replying to bastiK:

You could remove the dependency of Multipolygon class on NavigatableComponent by doing all calculations in EastNorth coords.

I think there's a real performance gain in this idea. I'm checking it.

in reply to:  15 comment:17 by bastiK, 12 years ago

Replying to bastiK:

[...] Then, in MapPainter copy the Polygons and convert to screen coordinates.

Or use the Graphics' transform to convert EastNorth to Screen coords. :)

comment:18 by Don-vip, 12 years ago

In [4630/josm]:

see #6987 - final (?) performance tweaks on multipolygon cache

comment:19 by Don-vip, 12 years ago

In [4631/josm]:

see #7110 and #6987 - Use getUniqueId() to handle newly created primitive objects

comment:20 by Don-vip, 12 years ago

In [4639/josm]:

see #7110 and #6987 - Create a new ArrayList to hold Nodes in PolyData

comment:21 by Don-vip, 12 years ago

In [4649/josm]:

see #7131 and #6987 - Let Multipolygon cache handle incomplete relations

comment:22 by Don-vip, 12 years ago

Resolution: fixed
Status: reopenedclosed

Modify Ticket

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