anomalyco
Diffs Performance Improvements
Alright, so this PR is an attempt to fix some of the performance issues that occurred in Safari.
Before I go into what I changed, here's a quick comparison of before and after this PR:
| BEFORE | AFTER |
|---|---|
Probably best way is to review on a per-commit basis since I grouped things logically like that.
SSR Fixes
One issue that I cannot provide a video of for a test the SSR fixes in the first commit (since it requires a specific cloudflare api key which I can't really get and test with). Essentially the fix here that I noticed is that SSR components were not being hydrated with the worker pool, which ends up kicking off a render cache for these diffs that will run shiki on the main thread, locking things up if when it tries to pre-hydrate the render cache.
I should probably look to improve this up stream in the library, but until then, the best fix here is to ultimately hydrate these components with the worker threads which should get pre-renders ready without locking up the main thread.
Furthermore, I saw a quick opportunity to better update the types for this to depend on our globally exported constant, making things more future proof if we ever decide to update the web-component tag name.
Strategic use of CSS Contain
In the second commit I tackled some CSS/DOM performance issues. Essentially these diffs are pretty gnarly in terms of dom node count, there's not a whole lot we can do here until I build some virtualization APIs. It does appear that this large dom size (probably coupled with the use of css variables as well) results in some pretty heavy css/layout/compositing costs. Using contain: layout paint on the diff components directly, and littering a few contain: strict on various app layout nodes makes a dramatic difference here.
Noisy Scroll Handler
And in my final commit, I found a scroll handler that was dynamically updating a CSS variable which seems to really push Safari through it's paces. It's unclear to me whether this is safe to remove, however from a quick grep, it did not appear to be a variable that was ever used in the first place, so removed that.
If you guys do depend on this CSS variable for something I am not aware of, well, then a fix here might be far more complicated as the noisy nature of this CSS variable was really hurting some CSS performance path.
And while there is still some blanking issues on content in the chat area (i did test a very extreme scenario where a of diffs were being rendered), it's vastly better than what it was before and i assume probably good for most normal scenarios.
In Summary
I don't know this app well so definitely not an expert in checking if any or all of these changes are safe, so would depend on some of you all to let me know if there are issues. One thing to keep in mind, using strict for contain may not always be safe if you guys ever have elements within these containers that need to extend outside, so this is something I would need to hear from ya'll if it's safe or not.
