publiclab / plots2

a collaborative knowledge-exchange platform in Rails; we welcome first-time contributors! :balloon:
https://publiclab.org
GNU General Public License v3.0
961 stars 1.83k forks source link

Gradually increase default results limit for /map view #10123

Open jywarren opened 3 years ago

jywarren commented 3 years ago

In #10059, we made it possible to increase the results limit at https://publiclab.org/map using the parameter limit:

https://publiclab.org/map?limit=100

https://github.com/publiclab/plots2/blob/6e499b5dee0a2bbfac0ec83e61746db6a8ca93a4/app/views/map/map.html.erb#L95

I just adjusted the default to 25. We'll ratchet it upwards gradually and take notes:

The code is here:

https://github.com/publiclab/plots2/blob/38ab2988fbd3e63227d02bd5398d64cc957d2279/app/views/map/map.html.erb#L95

Limit 100

Manually setting the limit to 100 and browsing around a bit did show a bit of a spike in Skylight:

Screen Shot 2021-08-31 at 12 06 53 PM

nearbyPeople not bad at max of 1.8s, but taglocations worse at up to 14s:

Screen Shot 2021-08-31 at 12 07 35 PM Screen Shot 2021-08-31 at 12 07 39 PM

We optimized tagLocations in #9946/https://github.com/publiclab/plots2/pull/10028, thanks to @17sushmita. I wonder if we could go further since it's going quite slow still at up to 14s.

We could look into partitioning calls into whole-integer-latitude/longitude boxes... as mentioned also in https://github.com/publiclab/plots2/issues/9946#issuecomment-898941171

17sushmita commented 3 years ago

Yes, I think we can try some more optimizations as I mentioned here https://github.com/publiclab/plots2/pull/10028#issuecomment-899899653. Also, looking into https://github.com/publiclab/plots2/issues/9946#issuecomment-898941171

jywarren commented 3 years ago

Went to limit of 50 in https://github.com/publiclab/plots2/commit/032edf8708e779e712a3fe52416eb3cc6346f3f9 !! 🤞

jywarren commented 3 years ago

Publishing to live now.

jywarren commented 3 years ago

OK, so at default limit 50, we are seeing a lot of great content (and the MapKnitter CORS fix shows maps now too):

image

vs limit 100:

image

vs the prior limit of 5 (i think):

image

yet it's not hitting the server too hard! I'm checking Skylight now but I think this means we are OK for uptime with the default limit 50, and don't need to pull back.

image

jywarren commented 3 years ago

It appears our tagLocations endpoint is doing even better!

image

jywarren commented 3 years ago

whereas nearbyPeople does occasionally get a 23 second load time:

image

jywarren commented 3 years ago

I'm comfortable leaving things as they are for a while pending more exploration, experimentation, and input. I think there's a lot of mileage to be had from turning other default layers on, perhaps, or doing more to tune/trim and interlink MapKnitter markers nearby, and/or tune the display of people, maybe? I think limits are doing OK right now at 50.