Modify

Opened 5 years ago

Closed 5 years ago

#7914 closed enhancement (fixed)

[PATCH] Concurrent fetching of primitives

Reported by: Don-vip Owned by: team
Priority: major Milestone:
Component: Core Version:
Keywords: performance, multithread, multicore Cc:

Description (last modified by Don-vip)

The OSM API usage policy allows us to use 2 download threads:

http://wiki.openstreetmap.org/wiki/API_usage_policy#Technical_Usage_Requirements

I have achieved to improve dramatically the performances in fetching a large number of primitives (I used as example the multipolygon 1155338 which contains more than 12k nodes). With this patch, download time fell from 3min50s to 1min42s on my machine !

However, I'd like to have some code review and some testing before committing.
I am particularly afraid of the handling of ProgressMonitor. I was expecting to get some ConcurrentModificationExceptions. I did not get any problem, but I don't know if I was lucky or if the code is OK :)

Attachments (1)

7914.patch (15.7 KB) - added by Don-vip 5 years ago.

Download all attachments as: .zip

Change History (8)

Changed 5 years ago by Don-vip

Attachment: 7914.patch added

comment:1 Changed 5 years ago by Don-vip

Description: modified (diff)

comment:2 Changed 5 years ago by bastiK

I like this... :)

Even users with a single processor machine would benefit, see e.g. this explanation.

ProgressMonitor should be safe for multi-threading, it uses synchronized methods throughout.

I noticed, after cancel it closes the dialog window, but the downloads go on in the background.

We should keep in mind, that the API usage policy might change at any time. Let's hope, if that happens, they implement it in such a way that the second connection gets queued and not rejected. Still, it might be a good idea to make the number of connections user configurable.

comment:3 Changed 5 years ago by stoecker

Please, as we now have online Javadoc - new code should be completely documented and doc-error free. And in the same run doc errors in modified files can be fixed :-)

BTW: Improving download time is good...

comment:4 Changed 5 years ago by Don-vip

Replying to bastiK:

I like this... :)

Thanks :)

Even users with a single processor machine would benefit, see e.g. this explanation.

OK so I'll test on my old laptop, with a single core processor, to see it's better. If yes, I'll remove the check on the number of processors.

ProgressMonitor should be safe for multi-threading, it uses synchronized methods throughout.

Great ! :)

I noticed, after cancel it closes the dialog window, but the downloads go on in the background.

Yep. Fix on the way.

We should keep in mind, that the API usage policy might change at any time. Let's hope, if that happens, they implement it in such a way that the second connection gets queued and not rejected.

I have a better idea on this: https://trac.openstreetmap.org/ticket/4501

Still, it might be a good idea to make the number of connections user configurable.

Agreed.

Replying to stoecker:

new code should be completely documented and doc-error free. And in the same run doc errors in modified files can be fixed :-)

Sir, yes sir ! :)

comment:5 in reply to:  2 Changed 5 years ago by skyper

Replying to bastiK:

Even users with a single processor machine would benefit, see e.g. this explanation.

Sure, I did always compile my kernels with gcc option "-j 2" on single cpus. On a quad-core I would add "-j 6".

comment:6 Changed 5 years ago by Don-vip

In 5386/josm:

see #7914 - Introduce OsmApi.MAX_DOWNLOAD_THREADS and improve javadoc in some classes of josm.io package

comment:7 Changed 5 years ago by Don-vip

Resolution: fixed
Status: newclosed

In 5387/josm:

fix #7914 - Concurrent fetching of primitives

The default and maximum number of threads is set to 2, to comply with OSM API usage policy:
http://wiki.openstreetmap.org/wiki/API_usage_policy#Technical_Usage_Requirements

The property osm.download.threads`can be set to 1 do restore single-thread download (it may still be faster than before as we have a download thread and a thread that merges the primitives in the data set)

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.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.