Reduce workload for web browser by removing all inline style #2125
Conversation
|
IMO we should just switch to Rust based ANSI coloring and get rid of ansiup. |
|
As wasm or in the server source code? |
|
Normally in the backend in Rust, Urgau even implemented a Rust crate for this, it just hasn't replaced ansi-up yet. I'm sure Urgau will have some thoughts/comments here :) |
|
I would prefer this approach a lot. Waiting for @Urgau to confirm before I actually implement it. |
|
@Urgau confirmed it by message so doing that instead. |
GuillaumeGomez
force-pushed
the
reduce-workload
branch
4 times, most recently
from
cbe26ef to
59e9ff6
Compare
|
Now all the rendering is done in the backend (in Rust, yeay). Sadly, the |
|
And CI passed, yeay. \o/ |
|
(back at my computer) Agree, doing in Rust is better. I started doing it but got side-tracked into doing it in JS when we wanted to do the download in the browser as well, but since we don't do that, let's do all the processing in Rust. Thanks for doing it. |
GuillaumeGomez
force-pushed
the
reduce-workload
branch
from
59e9ff6 to
9ed2643
Compare
|
Fixed merge conflict (although I think a regex might need an update). |
GuillaumeGomez
force-pushed
the
reduce-workload
branch
from
9ed2643 to
41ac875
Compare
| @@ -234,6 +227,93 @@ async fn process_logs( | |||
| } | |||
| }; | |||
|
|
|||
| let tree_roots = tree_roots.iter().map(|p| regex::escape(p)).join("|"); | |||
There was a problem hiding this comment.
Since the logs can be quite large, doing this processing might not be super cheap (the triagebot server isn't very powerful). What about computing all of this only once, and then caching the whole HTML page output? We'd have to check how does its size compare to the size of the raw logs though, it could be much larger than the log size.
I couldn't benchmark it locally because the regex crashed for me.
GuillaumeGomez
force-pushed
the
reduce-workload
branch
from
41ac875 to
64a802d
Compare
|
To be noted that I'm currently improving things in |
GuillaumeGomez
force-pushed
the
reduce-workload
branch
from
64a802d to
5be13e8
Compare
|
I tried it locally and the render time is ~900ms in release mode, that's quite a lot (on a relatively powerful Zen3 notebook). So I really think that we should cache the whole HTML output, rather than just the logs. The only problem with that is that for the experiment I tested, the logs were ~2.2 MiB, while the HTML was ~7.8 MiB, so almost a 4x increase. It also means that triagebot has to transfer 4x more data over the network (and it's a lot of data, for a web page). I wonder if this won't be actually slower in practice; doing the ANSIfication on the client at least had the advantage that we didn't transfer so much (useless) data over the wire. That being said, I think that compression could help here a lot. I tried to compress the data with zstd; after that the size is just 642759 (and the compression took just 13ms). So I would use some commonly supported compression format, e.g. brotli or something, ideally something that also has a pure Rust implementation if possible, and compress the result. That way we will not pay for the network transfer, just for RAM cost for the in-memory cache. |
|
I'm used to having the proxy handle the compression, didn't think it wouldn't be the case. If not we should add one for sure. So yeah, big downside is that page size is bigger, however the time needed to get the page fully rendered is shorter (as long as you don't have a slow internet connection). |
I'm not fully sure of the "is shorter" part, at least in the current state. I timed the JS portion of the original code (ANSIfy logs + replace links) and it takes just ~100ms on my PC... |
|
Ah indeed. Not sure why I remembered it took longer for me. Well, in that case, I can go back to what I was working on originally: improving the JS by removing all inline style. :) |
|
This PR already includes all your anstyle improvements? If not, we could still wait for them. |
|
Not yet. Also I didn't even start benchmarking anstyle which I think can be improved. |
|
Yeah just the anstyle part took 800ms, so that's the bottleneck. |
That is nuts, I don't remember seeing such bottleneck when testing out anstyle for our use case. If we can improve that on anstyle part that would be great. |
|
The primary objective was achieved by #2129. Let's close this one. |
|
Absolutely! Thanks for closing it, completely forgot. |
Since we now have our own version ofansi_up, we can iterate from there as I saw quite a few things that could be improved.In the meantime, this first iteration removes all inline style which is quite bad for web browsers.Now all the rendering is done in the backend (in Rust, yeay).
Sadly, the anstyle-svg crate I used was generating way too much tags, making the log pages extremely slow to render. So I opened rust-cli/anstyle#261 and rust-cli/anstyle#262 to fix that. Now it's super smooth and faster than it was before and nice side-bonus: no more JS. \o/
r? @Urgau