Modify

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#10902 closed defect (fixed)

[PATCH] TMS simultaneous connections

Reported by: bastiK Owned by: team
Priority: normal Milestone: 15.05
Component: Core imagery Version:
Keywords: Cc:

Description

TMS layer sends 25 requests at the same time. This is quite excessive, it should be 2-3 simultaneous connections per domain at most.

Attachments (2)

TMS_simultaneus_connections.patch (9.2 KB) - added by wiktorn 4 years ago.
TMS_simultaneous_connections_v02.patch (12.3 KB) - added by wiktorn 4 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 5 years ago by stoecker

Hmm, I think this is in sync with what a web browser does. TMS normally is a simple take and deliver. Usually no advanced operation is needed.

comment:2 Changed 5 years ago by bastiK

Normal browser limit seems to be at about 6 connections: http://stackoverflow.com/questions/985431/max-parallel-http-connections-in-a-browser

comment:3 Changed 5 years ago by stoecker

My feeling always was that a web-browser is a little more demanding to my tile servers than JOSM. So there must be other differences as well.

comment:4 Changed 4 years ago by wiktorn

Owner: changed from team to bastiK
Status: newneedinfo

After 8168/josm we have a default limit of 25 concurrent downloads (but not constrained by hosts), which is further tweakable from preferences menu.

If we are to do better here, then we need to decide, whether:

  • limit the number per host (many TMS providers has multiple hosts, but one host might also provide different TMS layers)
  • limit the number per TMS source

I can take this one.

comment:5 in reply to:  4 Changed 4 years ago by bastiK

Replying to wiktorn:

After 8168/josm we have a default limit of 25 concurrent downloads (but not constrained by hosts), which is further tweakable from preferences menu.

If we are to do better here, then we need to decide, whether:

  • limit the number per host (many TMS providers has multiple hosts, but one host might also provide different TMS layers)
  • limit the number per TMS source

I can take this one.

Please go ahead. 6-8 concurrent downloads per host sounds reasonable to me.

comment:6 Changed 4 years ago by wiktorn

Owner: changed from bastiK to wiktorn
Status: needinfoassigned

Changed 4 years ago by wiktorn

comment:7 Changed 4 years ago by wiktorn

Owner: changed from wiktorn to team
Status: assignednew
Summary: TMS simultaneous connections[PATCH] TMS simultaneous connections

patch ready

comment:8 Changed 4 years ago by bastiK

Resolution: fixed
Status: newclosed

In 8307/josm:

applied #10902 - TMS simultaneous connections (patch by wiktorn)

comment:9 Changed 4 years ago by bastiK

Great! I'm still confused by the numbers though. 25 concurrent threads for a queue of 5 elements, with a restriction that no more than 6 elements of the 5-element queue can be from the same host? Is the semaphore released when the download is finished or when it starts?

Edit: More documentation would be nice.

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

comment:10 Changed 4 years ago by Don-vip

Milestone: 15.05

comment:11 Changed 4 years ago by wiktorn

Resolution: fixed
Status: closedreopened

Indeed, this is not a fully working solution, as this only balances tasks in the queue

comment:12 Changed 4 years ago by wiktorn

Different approach to the problem attached, and some additional comments in the code.

Other possible approaches that I rejected (for reference):

  1. Create a thread-pool per host (but this will create a lot of threads, and it will be hard to control, how many threads in total we spawned.
  2. Implement all the limits in TMSCachedTileLoaderJob (submit + loadingFinished), but then limit would also cover cache accesses

Changed 4 years ago by wiktorn

comment:13 Changed 4 years ago by bastiK

Resolution: fixed
Status: reopenedclosed

In 8314/josm:

applied #10902 - TMS simultaneous connections (2nd patch by wiktorn)

comment:14 Changed 4 years ago by aceman

I think something is wrong with this patch. Panning the map (to download area) when the tiles are not yet downloaded OR panning the editor when background imagery needs to be downloaded is very slow and takes 100% of CPU. I have 10 downloads maximum set and the new downloads per host is at the default of 6.

comment:15 Changed 4 years ago by wiktorn

@aceman:

I can't reproduce the problem. Are you sure, that this is the patch, that introduced the problem?

Indeed, when downloading the tiles, I notice high CPU usage, but profiling discloses, that most of the time is spent in ImageIO.read (conversion JPEG form to native bitmap) and in painting the tiles.

I do not notice also any slowdown of the GUI itself. Can you please provide:

  • java version you're running JOSM on
  • operating system

comment:16 Changed 4 years ago by aceman

NO, I am not sure this is the culprit, but I just upgraded from 8297 to 8315 and this started happening. So it is the most logical candidate for the problem. I think the CPU load is higher when the network is congested. Maybe the threads are dropping some downloads and the resubmitting of requests is too costly?

I am on Linux kernel 3.19, Java 8b45.

comment:17 Changed 4 years ago by wiktorn

Ok, when I limited available bandwidth I see, that AWT thread consumes a lot of CPU, and SlippyMabBoxChoser even more. I'll check if 8297 (and ealier) did had the same behaviour. But I'd like to track this in separate defect.

comment:18 Changed 4 years ago by wiktorn

@aceman:
See #11404

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.