[DO NOT MERGE YET] Improve rendering performance by chunking layout #263
Conversation
This patch adds `content-visibility: auto` as described here: - https://web.dev/content-visibility/ - https://www.youtube.com/watch?v=FFA-v-CIxJQ Demo: - Before: https://mathiasbynens.github.io/ecma262-layout-chunking/before.html - After: https://mathiasbynens.github.io/ecma262-layout-chunking/after.html
|
@mathiasbynens I see a bit of flickering when scrolling through the |
|
Lol I completely broke it. Here I just scrolled around and stopped in a single position and I'm not doing anything anymore, but it just keeps flickering: https://drive.google.com/file/d/1DlZSMBaztjISBtetL3FCmj_w2IjVxrGa/view?usp=sharing |
|
@RReverser Not sure what you're referring to exactly. Some brief "white flashes" while scrolling are expected when you scroll to a new area that hasn't gone through layout yet, but "flickering" definitely sounds unexpected/bad. Could you please file a crbug? (I did file https://bugs.chromium.org/p/chromium/issues/detail?id=1138128 for the issue I described above.) |
Yeah sorry, just updated with a video to demonstrate. |
|
@RReverser Oh wow, thanks for providing that video. I had not run into that! Please file a crbug! |
|
This greatly improves the rendering on Chrome Android on Pixel 3 XL. 👍 |
As described in [1], this prevents a flickering issue. [1]: https://bugs.chromium.org/p/chromium/issues/detail?id=1138233#c3
|
The issue @RReverser reported is now addressed as part of this patch! |
|
I notice that this makes the scrollbar jump around as you scroll, since the browser doesn't know how big the document actually is. I wonder if it would be practical to do better (not necessarily before landing this PR) - e.g. as part of the build we could render the document without this property, open it in a browser, compute the actual sizes for each section, and then write those down in the CSS. Still wouldn't be perfect, but it would be a lot better. |
|
It seems bad if the scrollbar jumps around - bad enough to make the feature itself not worth it at all, imo. If it eventually (like, the 20-30s that loading would normally take) computed the correct scrollbar size, that seems reasonable, but the scrollbar is a progress bar, and a progress bar that jumps around randomly thus becomes useless. |
|
In particular, if you drag the scroll thumb with the mouse, it stops being attached to the cursor as you scroll - it looks really broken, and I'm very confused how this is acceptable to Chrome itself from a UX perspective :-/ |
|
One thing we can do is to make content-visibility a mobile-only style to aid Android readers, where the scrollbar discontinuity is less jarring. |
|
Note that the scrollbar jumping around already happens on mobile platforms even without this patch. IMHO it's a small price to pay for the massive load time improvements.
The "actual sizes" depend on viewport width, whatever values we hardcode at build time still wouldn't be perfect indeed — we’d be optimizing for 1 particular viewport size. |
|
Destroying the usefulness of the scrollbar on desktop does not seem a worthy price to pay to me. That mobile platforms already have broken scrollbars means those should be fixed - not that desktops should be treated to the same broken experience. |
Hm, I can get that to happen occasionally on my Android phone (a Pixel 3), but it's way more dramatic after this patch. Before it's jumping by perhaps a couple percent of the height of the scrollbar; after it's jumping more like 30% - enough that I lose track of the thumb while scrolling. It makes the scrollbar pretty much unusable while scrolling on my phone, instead of being an approximation with a very small amount of jitter. I don't think the two behaviors are really comparable.
I agree it might be worth it, but it does make scrolling really jarring, especially on desktop. Particularly the thing where, if you click and drag on the thumb and cross a section boundary, it jumps out from under your cursor. Breaking a part of the browser's UI seems like a pretty high cost to me.
True, but we could compute a few and interpolate with |
|
(Drive by comment: You could even grab these heights via puppeteer at a bunch of different screen widths and set them accordingly with media queries) |
I thought the problem was also that the ecma262 clauses are all very different sizes. What's the rough workaround there, bucket the clause classes by size? |
If we're using automation to compute sizes, we can just assign a different intrinsic size to each individual clause. |
I thought you’d set the intrinsic size per element via inline styles. |
|
I talked to a few frontend devs of my acquaintance, all of whom expressed severe reservations about the damage to the scrollbar: people are used to large pages loading slowly, but pages where the scrollbar doesn't work as expected stand out as broken. As such, I'd be really hesitant to accept this as-is. I'm still interested in exploring ways of making this work, such as those suggested above. But that said, I am not convinced this feature will see adoption in its current state: I suspect that most devs responsible for the large documents for which this is relevant will find the performance benefit not to be worth the cost of breaking part of the browser's UI. So I'm curious if Chrome or the CSS working group is looking into tweaking the design so that the benefit can be had without that cost, or if the expectation is that devs will need to try to patch over the scrollbar issues with client-side JS or other tricks. |
|
In this post Alex Russell describes using an intersection observer to keep things marked as visible once they've ever been within the viewport, which could be an improvement - the scrollbar would still jump around while moving to new content, but at least if you scroll back to a part you've already been to it'll stay put. Though it would come at the cost making resizing the page slow again once you'd loaded enough sections. |
|
(cc @slightlyoff given #263 (comment)) |
|
One option I've considered (but haven't implemented) is a placeholder element that grows in height as one accumulates visible sections. The |
|
Ok, I've been playing with a solution for my blog that's better in the face of resizing and the like. Here's the rough code, where the document structure of the elements I'd like to make cheaply available is: <html>
<head>
<!-- stuff -->
<style>
body > main > article {
content-visibility: auto;
}
</style>
</head>
<body>
<!-- stuff -->
<main>
<article>...</article>
<article>...</article>
<article>...</article>
...And here's the bit of script that targets those articles for efficient painting: <script type="module">
let articles = Array.from(
document.querySelectorAll("body > main > article")
);
let eqIsh = (a, b, fuzz=1) => {
return (Math.abs(a - b) <= fuzz);
};
let rectNotEQ = (a, b, fuzz=2) => {
return (!eqIsh(a.width, b.width, fuzz) ||
!eqIsh(a.height, b.height, fuzz));
};
let added = new WeakMap();
// Only call this when it's known cheap; post layout
let setHeight = (el, rect=el.getBoundingClientRect()) => {
let old = added.get(el);
added.set(el, rect);
// Set intrinsic size to prevent jumping on un-painting:
// https://drafts.csswg.org/css-sizing-4/#intrinsic-size-override
if (!old || rectNotEQ(old, rect)) {
el.attributeStyleMap.set(
"contain-intrinsic-size",
`${parseInt(rect.width)}px ${parseInt(rect.height)}px`
);
}
};
let iobs = new IntersectionObserver((entries, o) => {
entries.forEach((entry) => {
if (entry.intersectionRatio > 0) {
setHeight(entry.target, entry.boundingClientRect);
}
});
},
{ rootMargin: "50px 0px 100px 0px"}
);
let robs = new ResizeObserver((entries, o) => {
entries.forEach((entry) => {
if (entry.contentRect.height) {
setHeight(entry.target, entry.contentRect);
}
});
});
articles.forEach((el) => {
iobs.observe(el);
robs.observe(el);
});
</script>Basically, this sets up intersection and resize observers to create "set aside" space on each of the elements in question. and updates that geometry on resize so that if/when they scroll offscreen, their contents won't continue to get layout/paint passes applied until they "snap back". Have verified that this works as intended in Chrome. |
|
Is perhaps all this code something for which a standardized api for “efficiently paint this selector” might be warranted, since it seems the current solution in the PR requires a bit of effort to avoid footguns? |
|
This is 50 non-comment lines, including the CSS, so while I think there might be space for a "please reserve the last seen space for this element" attribute value for |
|
Thanks, @slightlyoff! @mathiasbynens, would you be up for dropping that snippet into your demo page so we can see how it feels? (Incidentally I suspect we'll end up wanting to put this in the ecma262-specific CSS, rather than ecmarkup itself, since it's aimed at larger documents rather than stuff like proposals, but we can keep iterating on it here for now.) |
|
@slightlyoff Nice!
Done. In particular, check the deep link demo: |
|
Hm, on Chrome 87 on Mac I don't get a scrollbar at all anymore in the after (https://mathiasbynens.github.io/ecma262-layout-chunking/after.html#sec-string.prototype.replaceall) version. Is that supposed to be the case? |
|
On Chrome 86.0.4240.193 the infinite loading behavior still happens, and the scrollbar never shows up, and on Canary (89.0.4349.2) the page hangs outright, so I can't assess it properly. (The hangs on Canary seem to happen on the "before" page as well, so it's probably a bug in Chrome rather than your code.) I'll wait a bit for a new version of Chrome, I guess. |
|
There was at least one Chrome bug (on Android) discovered in the snippet I posted above, and so I've updated it in a blog post about this approach: https://infrequently.org/2020/12/resize-resilient-deferred-rendering/ For Chrome bugs, tagging in @vmpstr (who I pinged out of band). On Cr87/Mac, the demo branch seems to be working as advertised for me (no hangs, and I see scrollbars). Will try other platforms. Update: Looks good on Windows! Also confirmed that a great deal of layout work is being skipped when resizing, e.g. Trace shows original (lower left) and updated (upper right) documents being resized. Layouts go from ~360ms -> ~80ms. |
@slightlyoff Thanks — demo pages updated accordingly. The trace looks 🔥🔥🔥 !
@bakkot I tested on the exact same Canary version (89.0.4349.2) on macOS, with a clean browser profile, and couldn't reproduce any hangs on either page. Are you sure you're not accidentally using some browser extension that interferes with the result somehow?
@syg That's definitely not the intention! I still get a scrollbar on 87.0.4280.88 on macOS. If you can reproduce this reliably, could you please file a crbug? |
Yup, it's a completely clean profile. Opening the demo page pegs the CPU (in fact it pegs two cores) for at least a minute (at which point I just killed it). I'm on macOS 10.14; maybe that's the diifference?
|
|
@bakkot: can you perhaps copy/paste the full contents of |
|
@slightlyoff Sure:
|
This patch adds
content-visibility: autoas described here:Demo:
Demo with deep link:
This is supposed to improve load-time performance by only requiring layout for the visible top-level
emu-clauseswhere applicable, in browsers that supportcontent-visibility. While recording traces to quantify the improvement I bumped into https://bugs.chromium.org/p/chromium/issues/detail?id=1138128, which we’ll probably want to have fixed upstream before landing this.cc @una @surma @jakearchibald