search

maplibre / maplibre-gl-js

MapLibre GL JS - Interactive vector tile maps in the browser
https://maplibre.org/maplibre-gl-js/docs/
Other
6.31k stars 688 forks source link

Google PageSpeed score #1876

Open myalban opened 1 year ago

myalban commented 1 year ago

Motivation

Is it possible to improve the JS parsing time ? Google page speed test is bellow 50% for smartphone.

https://pagespeed.web.dev/report?url=https%3A%2F%2Fmaplibre.org%2Fmaplibre-gl-js-docs%2Fexample%2Fdisable-rotation%2F

Design Alternatives

Split package with limited functions ?

HarelM commented 1 year ago

One of the main problems related to this code base is that it uses workers. In order to simplify the usage of this library the workers are rigged into the library code. This creates a huge problem for splitting, tree shaking etc. Feel free to research this and propose a solution. See also #977

rotu commented 1 year ago

I think this is fairly easy. Using the CSP build instead of the main one with custom Javascript loaders sped things up significantly. Also allows us to use modulepreload to get the worker to start loading before the main script is even started.

lighthouse-before.pdf lighthouse-after.pdf

I suggest we just replace the main build with the CSP one!

HarelM commented 1 year ago

Replacing the main with the CSP and shifting the configuration to the developer is problematic. If CSP can solve performance issue I suggest to better document it, but I don't think it should be the main build. Unless I'm missing something obvious here, when I looked at the CSP docs I got scared and decided to avoid it, might be just me though...

rotu commented 1 year ago

CSP itself is not solving performance issues - I think that wrangling the script into a blob is creating performance issues. (The script can't be parsed ahead of time and still has to be parsed in both the main and worker thread anyway). It just so happens that the blob approach also causes issues with CSP. Also, it means the worker script cannot be stored in browser cache. (in my above numbers, the cache is disabled anyway, so I think the new approach is winning even fetching the worker script 4 times!)

Additionally, CSP should not shift configuration to the developer - this package should definitely run "out of the box" without specifying a worker URL. I'm thinking that we should just set the worker URL to a sane default (e.g. new URL('./worker.js', import.meta.url) and release the worker script in the dist folder.

It's still necessary to have a non-esm-compatible worker script (because Firefox does not support ESM in workers).

rotu commented 1 year ago

Fyi @birkskyum.

birkskyum commented 1 year ago

Regarding firefox, there has been done more work on it in the past half year, than in the preceding 7 years link, so I hope we can finally have this someday.

I would love to see a demonstration of this with ESM only in say Chrome or safari to validate that our path ahead is clear.

clementmas commented 1 year ago

In terms of "time to first paint", I find that loading the local GeoJSON first before the base style makes it seem much faster. It might not improve performance. Or possibly make it worse since the GeoJSON has to be repainted over the base style once loaded. But that could be something to explore.

Canoir commented 1 year ago

hi, I don't actully understand the CSP build? what is that? i have such a problem too with greate core web vitals but 10 sec and 7 sec TBT and Speed index. without map everything is good.

rotu commented 1 year ago

Let’s start with “CSP”. It’s security model based around restricting where executable code can come from. Certain CSP options can forbid running code from a string via eval or from a blob: URL (aka object url).

Now in order to keep the size of this library down, some of the code which is used in both the main process and background workers is essentially kept as a string until late in the game and then constructs the worker code around it after the library has loaded. This is slow (and I think also prevents certain optimizations even when the page is already cached).

The CSP build does not do this - it ships the main library and the worker code as two separate scripts, duplicating the code but (I believe) running much faster.

I’ll see if I can put some solid measurements to my above wishy-washy description and speculation.

rotu commented 1 year ago

UPDATE: I can't seem to find the performance discrepancy I expected. Then again I'm testing on the test/examples/ pages, not the previous debug pages. Is this issue maybe resolved?

Canoir commented 1 year ago

But still my website has so much TBT and speed index because of maplibre-gl. website address is : https://behtarino.com

rotu commented 1 year ago

I couldn't tell ya. I ran the performance profiling on the latest version.

One thing that could be an impact is that your site is using multiple workers and maplibre uses a single worker as of https://github.com/maplibre/maplibre-gl-js/pull/2354

Another thing is that some web requests are curiously slow and have caching disabled (e.g. behtarino-manifest.json seems to take like 600 ms for some reason)