Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Google PageSpeed score #1876

Open
myalban opened this issue Nov 26, 2022 · 12 comments
Open

Google PageSpeed score #1876

myalban opened this issue Nov 26, 2022 · 12 comments
Labels
enhancement New feature or request PR is more than welcomed Extra attention is needed

Comments

@myalban
Copy link

myalban commented Nov 26, 2022

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
Copy link
Collaborator

HarelM commented Nov 27, 2022

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

@HarelM HarelM added enhancement New feature or request PR is more than welcomed Extra attention is needed labels Nov 27, 2022
@rotu
Copy link
Contributor

rotu commented Jan 3, 2023

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
Copy link
Collaborator

HarelM commented Jan 3, 2023

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
Copy link
Contributor

rotu commented Jan 3, 2023

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
Copy link
Contributor

rotu commented Jan 4, 2023

Fyi @birkskyum.

@birkskyum
Copy link
Member

birkskyum commented Jan 4, 2023

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
Copy link

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
Copy link

Canoir commented Jul 1, 2023

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
Copy link
Contributor

rotu commented Jul 1, 2023

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
Copy link
Contributor

rotu commented Jul 2, 2023

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
Copy link

Canoir commented Jul 2, 2023

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

@rotu
Copy link
Contributor

rotu commented Jul 2, 2023

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 #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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request PR is more than welcomed Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants