Modify

Opened 12 months ago

Closed 12 months ago

Last modified 11 months ago

#13570 closed defect (fixed)

[Patch] Coastline validator doesn't report some problems and is slow

Reported by: GerdP Owned by: team
Priority: normal Milestone: 16.10
Component: Core validator Version:
Keywords: coastline validator, performance Cc:

Description

What steps will reproduce the problem?

  1. open file coastlines-samples.osm.zip
  2. press Shift+V to run validator

What is the expected result?

All but the two ways in the top right box should be flagged

What happens instead?

Some ways are not flagged.

Please provide any additional information below. Attach a screenshot if possible.

The unpatched code is also rather complex and slow when the data contains lots of coastline ways
as the example for #13385 (y.osm.bz2)
The attached patch fixes the problems and makes the test much faster.

URL:http://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2016-09-06 00:16:07 +0200 (Tue, 06 Sep 2016)
Build-Date:2016-09-05 22:21:00
Revision:10966
Relative:URL: ^/trunk

Identification: JOSM/1.5 (10966 en) Windows 10 64-Bit
Memory Usage: 2301 MB / 5461 MB (2032 MB allocated, but free)
Java version: 1.8.0_102-b14, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: \Display0 1920x1080
Maximum Screen Size: 1920x1080
VM arguments: [-XX:StartFlightRecording=name=MyRecording2,settings=d:\dbg\gerd.jfc, -XX:FlightRecorderOptions=defaultrecording=true,dumponexit=true,dumponexitpath=e:\ld\perf_20160907_163056.jfr]
Dataset consistency test: No problems found

Plugins:
+ FastDraw (32930)
+ apache-commons (32699)
+ buildings_tools (32796)
+ ejml (32680)
+ geotools (32813)
+ jts (32699)
+ o5m (32857)
+ opendata (32898)
+ pbf (32865)
+ poly (32699)
+ reverter (32796)
+ utilsplugin2 (32815)

Map paint styles:
+ https://josm.openstreetmap.de/josmfile?page=Styles/Lane_and_Road_Attributes&zip=1

Attachments (4)

coastlines-samples.osm.zip (4.4 KB) - added by GerdP 12 months ago.
13570.patch (13.5 KB) - added by GerdP 12 months ago.
13570_v2.patch (54.5 KB) - added by GerdP 12 months ago.
13570_area.patch (1.4 KB) - added by GerdP 12 months ago.

Download all attachments as: .zip

Change History (12)

Changed 12 months ago by GerdP

Attachment: coastlines-samples.osm.zip added

Changed 12 months ago by GerdP

Attachment: 13570.patch added

comment:1 Changed 12 months ago by Don-vip

Milestone: 16.09

comment:2 Changed 12 months ago by Don-vip

Thanks for the patch! It would be great to add a unit test on the same model as MultipolygonTestTest.testMultipolygonFile with your sample file.

comment:3 Changed 12 months ago by GerdP

OK, I also thought about that.

Changed 12 months ago by GerdP

Attachment: 13570_v2.patch added

comment:4 Changed 12 months ago by GerdP

Please review. I am not sure about the unit test setup. The coastline test uses Main.getLayerManager().getEditLayer() so
it needs an edit layer, but I have no idea why I see the messages below (similar problem in MultipolygonTestTest):

Sep 10, 2016 4:30:22 PM org.openstreetmap.josm.Main info
INFO: GET http://api06.dev.openstreetmap.org/api/capabilities -> 200 (391 B)
Sep 10, 2016 4:30:22 PM org.openstreetmap.josm.Main info
INFO: OK
Sep 10, 2016 4:30:23 PM org.openstreetmap.josm.Main info
INFO: GET https://josm.openstreetmap.de/maps -> 200
Sep 10, 2016 4:30:23 PM org.openstreetmap.josm.Main info
INFO: GET https://josm.openstreetmap.de/wiki/StartupPage -> 200
Sep 10, 2016 4:30:26 PM org.openstreetmap.josm.Main info
INFO: Keystroke shift pressed ESCAPE is already assigned to org.openstreetmap.josm.actions.relation.RecentRelationsAction$1@c3d81ef, will be overridden by org.openstreetmap.josm.actions.relation.RecentRelationsAction$1@6a47a37a

comment:5 Changed 12 months ago by Don-vip

Resolution: fixed
Status: newclosed

In 10991/josm:

fix #13570 - Coastline validator doesn't report some problems and is slow (patch by Gerd Petermann, modified)

comment:6 Changed 12 months ago by Don-vip

thank you! I removed the need for a layer, and refactored the code using Java 8 features to avoid copy&paste

comment:7 Changed 12 months ago by GerdP

Fine!
I am not happy with the solution reg. downloadedArea, see also #10638.
My understanding is that it can be done once in visit(), see new patch. The effect on performance is near zero, it just looks clearer to me.

Besides that I still see the delay when I start the unit test in Eclipse. Do you see a chance to change that? On my machine the delay is ~ 5 seconds, and the time is added to the first executed unit test, so it somehow messes up the reported duration times.

Changed 12 months ago by GerdP

Attachment: 13570_area.patch added

comment:8 Changed 11 months ago by simon04

Milestone: 16.0916.10

Milestone renamed

Modify Ticket

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