etherpad-lite icon indicating copy to clipboard operation
etherpad-lite copied to clipboard

Possible page load improvements

Open webzwo0i opened this issue 5 years ago • 8 comments

I just checked the network tab and the following requests are not optimal, while https://github.com/ether/etherpad-lite/pull/4442 is the most important one:

  • socket.io has no max-age header. instead it uses if-none-match header and gets a 304. ~~- locales.json has no max-age header. instead it uses if-none-match header but still gets 200. (why?)~~
  • plugin-definitions.json has no max-age header.
  • favicon.ico has no max-age header. response uses etag, but the request does not use if-none-match.

imo the two more interesting:

  • pad_utils.js is included in pad.html to use escapeHTML (so no code duplication happens?). It results in a 307 redirect to the timeslider.js package (!). It's also packaged in pad.js, however timeslider.js is defined later in tar.json so the latter is picked. This means, whenever a normal pad is visited the timeslider package (100kb+) is downloaded after following the redirect. The timeslider package however is cached, so after a first load it's served from browser cache. However, the 307 redirect is still present. It's easy to put pad_utils.js in an own package or to put pad.js after timeslider.js in tar.json - but I think frontend tests are necessary first. (pad_utils.js is used when showing a meaningful error message) Alternatively we could inline all the code for escapeHTML and drop pad_utils from pad.html (https://github.com/ether/etherpad-lite/blob/develop/src/templates/pad.html#L446)

  • client_plugins.js also results in a 307, redirecting to ace2_common.js I am not sure from where it's included, may be https://github.com/ether/etherpad-lite/blob/develop/src/static/js/ace.js#L266

webzwo0i avatar Oct 27 '20 00:10 webzwo0i

I'm going to make something that gets timing performance (startup, page load, availability to edit etc) and push this as a test.

JohnMcLear avatar Jan 23 '21 14:01 JohnMcLear

https://github.com/ether/etherpad-lite/pull/4681 now has test coverage in for performance, obviously values in the plugin will need modifying to catch the faults you have spotted. I'm gonna consider my input on this closed off as you have the tools now for your minification/caching work :)

JohnMcLear avatar Jan 29 '21 22:01 JohnMcLear

@webzwo0i did you have a PR you wanted to land for this?

JohnMcLear avatar Feb 12 '21 11:02 JohnMcLear

Adding my findings.

socket.io has no max-age header. instead it uses if-none-match header and gets a 304. <- confirmed. - burn socket.io

locales.json has no max-age header. instead it uses if-none-match header but still gets 200. (why?) <- confirmed. -- pr available to add maxAge

plugin-definitions.json has no max-age header. <- confirmed. -- pr available to add maxAge

favicon.ico has no max-age header. response uses etag, but the request does not use if-none-match. <- confirmed. -- pr available to add maxAge

pad_utils and client_plugins.js 307s <- confirmed -- pr available but I don't think it's working as you hoped?

JohnMcLear avatar Feb 12 '21 12:02 JohnMcLear

PRs: https://github.com/ether/etherpad-lite/pull/4758 https://github.com/ether/etherpad-lite/pull/4759 https://github.com/ether/etherpad-lite/pull/4761

JohnMcLear avatar Feb 12 '21 14:02 JohnMcLear

Can we switch to webpack instead of tuning tar.json? What would it take to switch?

rhansen avatar Feb 13 '21 23:02 rhansen

See ep_webpack

I'm +1 removing tar.json

JohnMcLear avatar Feb 13 '21 23:02 JohnMcLear

ep_webpack only packs .js files from plugins. It doesn't touch core .js files.

I looked at webpack a bit and it looks like a fair amount of straightforward work, except for one issue: All of the documentation I've found tells you to set it up in a way that you have to manually run a build command before you can start the server. I would prefer to set it up so that Etherpad compiles everything (if required) on startup or on first request to match the current behavior. There does appear to be a way to run webpack from Node.js (see https://webpack.js.org/api/node/) but I don't know if there are any limitations.

rhansen avatar Feb 15 '21 06:02 rhansen