OpenClinica / enketo-oc

OpenClinica's fork of the Enketo web forms monorepo
Apache License 2.0
0 stars 1 forks source link

Performance improvements #102

Open MartijnR opened 3 years ago

MartijnR commented 3 years ago

Start with Lighthouse recommendations

pbowen-oc commented 3 years ago

@MartijnR - Internal feedback on resource caching: Static resources like css, js and font files served by the form engine are not cached. I believe this is because we have a "cache-control" header set with max-age=0. These resources should be allowed to be cached by the browsers.

If there is a newer version of the file, it needs to be loaded form the network. Consider also setting a max age to force reload of resources periodically.

MartijnR commented 3 years ago

test form ee: http://localhost:8005/widgets

Screen Shot 2021-05-06 at 11 45 00 AM
const perfData = window.performance.timing;
const elapsed  = {
    pageLoadTime: perfData.loadEventEnd - perfData.navigationStart,
    connectTime: perfData.responseEnd - perfData.requestStart,
    renderTime: perfData.domComplete - perfData.domLoading
};
for (const [key, value] of Object.entries(elapsed)) {
  console.log(`${key}: ${value}`);
}

baseline (FAST 3G): pageLoadTime: 7374, 7369, 7367 connectTime: 617, 613, 614 renderTime: 6780, 6780, 6793

baseline (normal, but serving from localhost): pageLoadTime: 534, 374, 403 connectTime: 284, 131, 157 renderTime: 245, 221, 238

test form ee-oc:

MartijnR commented 3 years ago

HTTP/2 instead of HTTP/1.1

This would be the lowest-hanging fruit, it seems.

https://enke.to/widgets (live, real server)

https://www.digitalocean.com/community/tutorials/how-to-set-up-nginx-with-http-2-support-on-ubuntu-18-04

baseline:

Screen Shot 2021-05-06 at 11 51 40 AM

New (with http/2):

Screen Shot 2021-05-06 at 2 34 37 PM
svadla-oc commented 3 years ago

Thanks @MartijnR. We will look into switching to HTTP/2. In addition to this, could we also allow resource caching?

MartijnR commented 3 years ago

yes @svadla-oc, I think we should. I'm still figuring out whether that should be added to the app or if it should be done by NGINX/Apache. If you have an opinion about that please let me know.

svadla-oc commented 3 years ago

@MartijnR I think it is better to add the cache header within the app instead of Nginx/Apache to avoid external dependency. It seems like the app is currently setting "cache-control" header with max-age=0 and that is preventing the browsers from caching these resources.

MartijnR commented 3 years ago

The question is whether the Service Worker (offline app cache) can be thrown off by any type of app/NGINX HTTP caching.

I moved this to a separate EE issue: https://github.com/enketo/enketo/issues/1076

MartijnR commented 3 years ago

Forcing the client to verify whether a file has been updated, reduces the performance improvement (still very significant), but eliminates any issues with not immediately serving new versions of files after updating Enketo Express.

MartijnR commented 3 years ago

hmm... It looks like https://enke.to/widgets, already has the caching I was implementing (when turning off 'disable cache' in the network tab of dev tools). E.g. the JS bundle returns a 304 response. Also for css, fonts, translations and HTML page.

Screen Shot 2021-05-18 at 2 31 03 PM

It's a fairly slow request though.

It uses Cache-control: public, max-age=0 and etag. Even though max-age=0, browsers may cache it because of 'public'. Chrome seems to do so.

More aggressive (and risky) caching (without checking for newer version) may be easier in NGINX. KoBo seems to think so.

Wherever that aggressive caching would be done, we'd need to either bust that cache or perhaps have only a short (1 day) maxAge. Not sure if the latter could still cause a problem with mismatched resources.

pbowen-oc commented 3 years ago

Some resources are already being cached less aggressively (returning 304 when the file is checked). Use Lighthouse in Chrome Developer Tools to see what else is possible.

Consider doing caching in NGinx based on Enketo version.

MartijnR commented 3 years ago