protomaps / protomaps-leaflet

Lightweight vector map rendering + labeling and symbology for Leaflet
https://protomaps.com/docs/frontends/leaflet
BSD 3-Clause "New" or "Revised" License
757 stars 43 forks source link

redundant requests #6

Closed michaelkirk closed 3 years ago

michaelkirk commented 3 years ago

I noticed a series of requests, seemingly unnecessarily, requesting the same byte ranges.

Screen Shot 2021-04-22 at 12 12 57 PM

And cool project! I had no problems going through the demo and getting a local example up and running.

bdon commented 3 years ago

Do you have the link (it's a UUID) of the page where you downloaded it, or can you screenshot for me the area? If you were looking at an area with lots of ocean, those redundant tiles will point to the same offset in the archive, so it relies on the browser disk cache: see pt3 here: https://github.com/protomaps/pmtiles#notes

michaelkirk commented 3 years ago

I have since closed the download page, so I don't know the URL/UUID. I don't see it anywhere obvious in my downloaded bundle.

here's a screenshot of the area:

Screen Shot 2021-04-23 at 11 17 39 AM

There's no ocean, though the lower schuylkill is tidal. =)

Here's my bundle if you want it: MapBundle-West Philly et al.zip

(heads up that I made a couple changes to the server script to log the requested Range).

One thing I notice is that the requests are initiated by separate scripts:

Screen Shot 2021-04-23 at 11 21 25 AM Screen Shot 2021-04-23 at 11 21 30 AM
michaelkirk commented 3 years ago

One thing I notice is that the requests are initiated by separate scripts:

Oh, and just to be clear, though more than one script was making the "redundant" request, sometimes even the same script was responsible, e.g. see below for the 3rd and 4th request of that byte range:

Screen Shot 2021-04-23 at 11 24 43 AM Screen Shot 2021-04-23 at 11 24 48 AM
bdon commented 3 years ago

In the demo code, there are two relevant scripts loaded: the pmtiles script, and the protomaps.js script. the PMTiles script is loaded even though the PMTiles library is bundled as a dependency of PM.JS. The reason is that the PMJS LeafletLayer API does not expose a way to get at the PMTiles metadata object, I am thinking about the right API to do this, since I expect a common use case is "Load this map and zoom to the center using the PMTiles metadata".

For now, each of the two scripts will instantiate a PMTiles instance and have their own cache, resulting in duplication of some header and directory requests. That still doesn't address the duplication of requests you're seeing within the same script, which I will need to take a closer look at.

bdon commented 3 years ago

Related issue: https://github.com/protomaps/PMTiles/issues/11

bdon commented 3 years ago

Current plan is to:

  1. Export the bundled PMTiles dependency in protomaps.js, something like protomaps.PMTiles
  2. Allow passing a PMTiles instance for the url value in LeafletLayer options

That way, you can access PMTiles metadata for finding the center of a map before creating the layer, while reusing the cache.

The reason I don't want to implement this as something like LeafletLayer.metadata() is that it would be meaningless/idiosyncratic for /{z}/{x}/{y}.pbf tile URLs (what should be returned?); although most of the tile APIs I run myself have an equivalent /metadata endpoint, this is arbitrary and vendor-specific.

So if you want your code to depend on metadata being available, create a PMTiles instance first and pass it to LeafletLayer. This should solve the issue of redundant requests across scripts; haven't thoroughly investigated the same-script case yet.

bdon commented 3 years ago

@michaelkirk I've updated https://protomaps.com/bundles with the new versions of PMTiles and protomaps.js, so that the directory cache is reused. I haven't observed any redundant requests for the header bytes (0-511999).

The case of redundant requests for ranges that are 1) not the header and 2) not redundant ocean or land tiles is still a mystery to me. I will keep an eye out for this happening as I develop, but if I don't see anything for a while I will mark this issue as resolved for now.

bdon commented 3 years ago

Marking as closed for now, please reopen if you notice this behavior.

michaelkirk commented 3 years ago

I just checked the demo https://protomaps.github.io/PMTiles/examples/leaflet.html and am still seeing redundant requests.

I'm only opening the network inspector and going to that URL - no clicking or moving the map around or anything required.

Safari: Version 14.1 (16611.1.21.161.6)

Screen Shot 2021-05-12 at 10 33 46 AM

Chrome: Version 90.0.4430.93 (Official Build) (arm64)

The same 5 redundant requests appear in chrome - here's two of them:

Screen Shot 2021-05-12 at 10 33 21 AM Screen Shot 2021-05-12 at 10 33 28 AM
michaelkirk commented 3 years ago

Maybe that's expected due to there being lots of water tiles though?

michaelkirk commented 3 years ago

I ran a new export for approximately the same area as before, and was not able to reproduce any redundant requests. 👍

bdon commented 3 years ago

@michaelkirk kind of: those redundant requests are actually specific to the view starting at zoom 0, and Leaflet wrapping the map horizontally by default, so you're requesting the zoom 0 single tile five times. The renderer treats those as separate instances of a tile, so the behavior here mirrors what would happen if it was a raster image: five HTTP requests are made for the image, and it relies on the browser cache.

You can see there's no redundant requests if I set noWrap:true on the leaflet layer:

Screen Shot 2021-05-13 at 1 43 18 AM