Use a custom servlet for serving files to support caching of files #1521
Conversation
digitaldan
changed the title
Job #583: Bundle Size — 15.93MiB (0%).Metrics (no changes)
Total size by type (no changes)
|
|
Hi @ghys, i have longed noticed that the browser does not seem to do a good job caching assets from our UI and static files. This is somewhat noticeable on on a local LAN, more so if the hardware is a little slow, and on an iffy cellular connection this can be quite noticeable. This replaces the simple file serving we did have, which did not have a concept of caching, with a servlet who will either A) use the startup time of the bundle as the "last-modified" time, which ensures all bundled files get cached appropriately or B) use the last modified time of the local file in the user's 'html' directory, which is especially nice when dealing with large images. I also added a hash to the webpack app bundles to ensure uniqueness when loading, with the above caching this may not be absolutely necessary, but seems like a good idea in any case. |
|
@digitaldan that's great! A new servlet will help a lot for sure! I'd like to have this PR as an opportunity to discuss further improvements and fixes:
I've found an implementation of a servlet that seems to tackle both: https://balusc.omnifaces.org/2009/02/fileservlet-supporting-resume-and.html For GZip compression the example above will perform the compression on the fly (with a I've done that just now as a test, following https://github.com/webpack-contrib/compression-webpack-plugin/tree/v6.0.3#multiple-compressed-versions-of-assets-for-different-algorithm (changed ([email protected] is the last version compatible with webpack 4) The results are as follows, the potential gains are impressive:
See the app entry points alone:
With some moving files around the totals are: I remember trying this with the HABot client but it wasn't working well with the openHAB Cloud reverse proxy (it didn't forward the |
|
@ghys I'll take a look at both use cases. Regarding compression, for bundled gzip assets, does the client request something like For other file serving using gziped streams, i wonder, does something in our OSGI stack have any concept of this? is there something we could delegate to for this ? If not, its probably simple to implement, if we decide its worth doing. |
|
Also i might look at using an eTag instead of the If-Modified header. Seems like it might be easier to deal with. |
That's it.
Not sure, but it looks like Jetty has some support indeed.
|
|
Update- I've tried adding: <Set name="gzip">true</Set>
<Set name="etags">true</Set>to And stopped the main UI bundle so that the default servlet is used again for And indeed the gzip-encoded version is served and a ETag added:
On subsequent requests for this file Chrome sends a
Note that the gzip paramater is deprecated and precompressedFormats should be used instead but it needs to be configured with an array in the xml. |
|
NIce find! |
|
So i have something working based on extending the jetty default servlet, i believe it actually solves caching, serving local gzip assets and byte range support..... and its actually less code then my original servlet .. and it worked the first time i tried running it....so something is certain to be wrong with it :-) take a look at https://github.com/digitaldan/openhab-webui/blob/33dfddcd42bedb0705206fb449bb9fd0609b799f/bundles/org.openhab.ui/src/main/java/org/openhab/ui/internal/UIServlet.java , i have not tried acutally testing range or gzip files yet, but i believe they will work , i'll play some more with it this week. |
|
I was hoping you'd come up with something like this, if it indeed works that's brilliant! We have to make sure it plays well with reverse proxies in general and openHAB Cloud in particular, like if the response is compressed it should simply forward it to the client with the |
|
@ghys do you have a branch which uses the webpack compression plugin ? I am running my test branch now on my production home system, so far so good, would like to test the gzipped assets. I'll also throw in a big video file in my html folder and see if Safari can stream it ( I believe thats the best way to test byte range support?) |
|
No, but it's quite easy, just install: and add this in webpack.config.js before new CompressionPlugin({
filename: '[path][base].gz',
algorithm: 'gzip',
test: /\.js$|\.css$|\.html$/,
threshold: 0,
minRatio: Infinity,
}),
new CompressionPlugin({
filename: '[path][base].br',
algorithm: 'brotliCompress',
test: /\.(js|css|html|svg)$/,
compressionOptions: {
level: 11,
},
threshold: 0,
minRatio: Infinity,
}),(import it with |
Yes I suppose! Or you can test with curl on a text file perhaps: https://everything.curl.dev/http/ranges |
|
Success! The servlet is successfully serving br and/or gz files automatically as discussed, caching using etags as well as byte range support. I'm running this on my home system, both locally, remote and through myopenHAB and everything is working. The PR is updated with this new servlet if you want to give it a try. The code is fairly simple as well and does not stray much from your original logic of serving resources which is a nice bonus since that has been throughly tested. I'll play with it more this week to see if any tweaks are needed. I also want to double check we are passing all headers along through myopenhab to ensure optimal caching is happening. |
digitaldan
force-pushed
the
file-servlet
branch
2 times, most recently
from
2556373 to
7b21a44
Compare
|
Actually, myopenhab is respecting headers and compressed content, so a win there as well! |
Signed-off-by: Dan Cunningham <[email protected]>
digitaldan
force-pushed
the
file-servlet
branch
from
7b21a44 to
7277f35
Compare
|
A huge step forward, unfortunately there seems to be some problems:
I tried to figure out what was the logic by analyzing https://github.com/eclipse/jetty.project/blob/jetty-9.4.x/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java#L320 and It would be better to have some kind of server preference (which seems to be the case: jetty/jetty.project@aa8597c) but I haven't made sense of it. It really seems random based on the first resource it finds... Obviously if Brotli is available and supported by the browser it should be preferred.
When the content is gzipped it actually seems that the content is sent uncompressed with a
This can be confirmed with some curl commands: One quick & easy way to solve both 1. & 2. would be to only provide Brotli precompressed assets, as there doesn't seem to be any problems with them, they would be served as the only precompressed encoding available, and they have widespread support, worst case scenario the client would be served uncompressed assets.
It can probably be tracked to this code: The redirect shouldn't happen and the index.html page should be served instead for those URLs - the UI's router will pick up the server-relative URL and display the proper page. |
Ok, i did see this once as well, but then it didn't happen, figured it was something weirdly cached on my end, but obviously not. I will need to play with this some more. I'll look at the code you referenced to see i can get a better picture of whats going on. Unfortunately this is not a well documented area of jetty. FYI, If you turn up debug logging you can see how jetty calls for these files starting with the uncompressed version and moving to gz, and br.
Hmm, strange, all my files were served using
Yeah, i thought i took care of that, i even have a comment in the code specifying this very thing. So whats strange is that |
Correction, actually i did not see this exactly, but i did notice some strange behavior where sometimes it loads the single app bundle, and sometime it loads all of the fragments shown in your console. I'm not sure how that works with webpack? Does the client eventually need all the app fragments in the js directory, or can it load one big bundle ? |
|
Thanks for the debug log tip, here's a typical refresh of the about page (with the unfortunate redirect): and the network traffic as seen by the browser: I don't really see what determines that this app.9e494f674314ce5c7e79.js is served the gzip version instead of the brotli version.
hehe, that's why I told you the details. It really seems that something in the middleware stack (express or nginx?) knows how to decode gzip but not brotli and decides to decode it.
It depends on whether the service worker has successfully registered (in that case the workbox library will precache all assets, after which you will typically get a "install" or "add to home screen" prompt). Otherwise the assets will be loaded on demand depending on the needs - for example the SIP widget will load some additional assets. If you reload a simple page (for example the about page), the only UI assets that should be loaded will normally be the entry point, that is app.[hash].js and app.css (along with fonts and logos). |
Signed-off-by: Dan Cunningham <[email protected]>
|
@ghys I am not seeing 'gz' assets served when 'br' assets are present. I have tried reloading and clearing cache many times as well as using both firefox and chrome. Any thoughts why its happening in your environment and how i might reproduce ? |
|
@digitaldan Yes I think I figured out the symptom, but have no solution yet. I'm putting all 3 versions of app.9e494f674314ce5c7e79.js (uncompressed, .gz, .br) from yesterday's builds and also the new one following your last commit (works nice btw) in For yesterday's build I always get the gz version but for today's I get the br version. If I delete the gz version from yesterday's build I get the uncompressed version (so the br version is completely ignored), likewise if I delete the br version from today's build, the gz version is ignored. Turns out the critical line of code is this one: In particular: On yesterday's build this is not the case for the br asset (last modified 2s before the original): On today's build the gz asset's last modified time is 4s before the original: So that's why they are ignored. The solution would be to ensure that the compressed assets always have a last modified date after the originals, but I'm not sure how to achieve that yet! |
|
A faster way to check these last modified dates directly from the bundle is to look for the Any compressed asset older than the original will never be served as described above. |
Nice find! That would have taken me a while to figure out. I have a couple ideas, but an easy one would be to extend the Resource object when loading out of the bundle, and for the last modified time always return the same date, like the startup time of the bundle, or grab it from maybe the index.html or something well known . So for example private long startupTime startupTime = System.currentTimeMillis(); // or we get this from a well known file in the bundle.
....
class CommonTimeResource extends Resource {
private Resource baseResource;
public CustomResource(Resource baseResource) {
this.baseResource = baseResource;
}
// return the same time for all bundled files!
@Override
public long lastModified() {
return startupTime;
}
@Override
public boolean isContainedIn(@Nullable Resource r) throws MalformedURLException {
return baseResource.isContainedIn(r);
}
@Override
public void close() {
baseResource.close();
}
@Override
public boolean exists() {
return baseResource.exists();
}
@Override
public boolean isDirectory() {
return baseResource.isDirectory();
}
@Override
public long length() {
return baseResource.length();
}
@Override
public URL getURL() {
return baseResource.getURL();
}
@Override
public File getFile() throws IOException {
return baseResource.getFile();
}
@Override
public String getName() {
return baseResource.getName();
}
@Override
public InputStream getInputStream() throws IOException {
return baseResource.getInputStream();
}
@Override
public ReadableByteChannel getReadableByteChannel() throws IOException {
return baseResource.getReadableByteChannel();
}
@Override
public boolean delete() throws SecurityException {
return baseResource.delete();
}
@Override
public boolean renameTo(@Nullable Resource dest) throws SecurityException {
return baseResource.renameTo(dest);
}
@Override
public String[] list() {
return baseResource.list();
}
@Override
public Resource addPath(@Nullable String path) throws IOException, MalformedURLException {
return baseResource.addPath(path);
}
}FYI I have not actually run this yet, so not sure it works. The other option would be to do something during the node build, but i worry about platform specific time issues depending on the build host and OS. |
|
Yep the custom Resource could work!
Probably won't work because the dates in the .jar are actually all the same: I'm not sure where these different last modified times are from, probably from a cache? |
Signed-off-by: Dan Cunningham <[email protected]>
|
Give the latest push a try and let me know what you think ! |
Signed-off-by: Dan Cunningham <[email protected]>
|
It works well! Good idea to only use the resource wrapper for the bundle resources and not the user static assets (having the real time in the
Note the odd uncompressed assets, this make sense - that's because of the other condition in Jetty's code, the compressed versions are ignored when they're larger in size than the originals, which is good. |
| new CompressionPlugin({ | ||
| filename: '[path][base].br', | ||
| algorithm: 'brotliCompress', | ||
| test: /\.(js|css|html|svg)$/, |
There was a problem hiding this comment.
Maybe we could save a few more bytes by compressing additional extensions (.woff, .woff2, .ttf, .mp3, .jpg, .png) though it remains to be seen as they might not have that good a compression ratio.
Otherwise, I think we're good to merge if you don't have anything more to check!
There was a problem hiding this comment.
Awesome, let me just check the code again and make sure there's no clean up i need to do, ill ping you shortly !
There was a problem hiding this comment.
Ok, we are good to merge. Thanks for all the very in-depth testing and advice! Hopefully this makes a noticeable difference for people.
There was a problem hiding this comment.
The code above compresses SVG files for brotli, but not for gzip.
The Gzip compression can be improved with 3-8% by compressing with Zopfli: https://webpack.js.org/plugins/compression-webpack-plugin/#using-zopfli instead of the default Gzip implementation. However all browsers not supporting Brotli were released before 2018, so the GZip compression helps only to browsers released before 2018.
Maybe we could save a few more bytes by compressing additional extensions (.woff, .woff2, .ttf, .mp3, .jpg, .png) though it remains to be seen as they might not have that good a compression ratio.
JPG is already compressed. It might be possible to reduce further the jpg file size by stripping meta data, and applying lossless conversions.
SVG files can be made smaller by stripping from them whitespaces and useless elements — #1430.
PNG files can be reduced by applying to them lossless compressions/optimizations — #1431 . My experience is that to reduce the PNG files further one has to convert them to WebP format, instead of applying brotli/gz.
Signed-off-by: Dan Cunningham <[email protected]>
There was a problem hiding this comment.
Great, thank you so much, this is a great & welcome addition! Lots of bandwidth saves can be expected with the new ETags-based caching & the UI assets being sent compressed (more than 8.5MB if Brotli is used, more than 7.6MB if gzip is used).
Note that the issue with openHAB Cloud uncompressing gzip and sending them plain with the wrong header remains, but it's relatively benign now that Brotli is likely to be served to the vast majority of browsers.
ghys
changed the title
|
Thanks!
I'll be looking into that this weekend, its not clear if its the Java http client in the cloud binding who makes the final request to the servlet, something in node land, something in nginx and so on...... |
|
Great but it's not as critical in the end, a quick & easy suggestion to reproduce: |
|
Yeah, was not sure if we wanted to support compression look up on static files. Its a one or two line change to the logic if indeed we want to support it . |
|
Sorry forget my previous comment, i thought you were referencing something else. My hunch is that the Jetty HTTP client in the binding see's the the returned content encoding and decompresses it before returning its byte array, then we are passing it along but still specifying Content-Encoding: gzip. I think its an easy fix (🤞 ) |
Also, use hash in webpack builds to ensure unique names for caching.
Signed-off-by: Dan Cunningham [email protected]