ecmarkup
ecmarkup copied to clipboard
tc39
[DO NOT MERGE YET] Improve rendering performance by chunking layout
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
Demo with deep link:
- Before: https://mathiasbynens.github.io/ecma262-layout-chunking/before.html#sec-string.prototype.replaceall
- After: https://mathiasbynens.github.io/ecma262-layout-chunking/after.html#sec-string.prototype.replaceall
This is supposed to improve load-time performance by only requiring layout for the visible top-level emu-clauses where applicable, in browsers that support content-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
@mathiasbynens I see a bit of flickering when scrolling through the after.html demo, at least while it's loading - is this expected?
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.)
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.
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!
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.
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.
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.
Note that the scrollbar jumping around already happens on mobile platforms even without this patch.
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.
IMHO it's a small price to pay for the massive load time improvements.
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.
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.
True, but we could compute a few and interpolate with calc (assuming that's legal). It would be an improvement, at least.
(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)
(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?
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 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?
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 https://github.com/tc39/ecmarkup/pull/263#issuecomment-739142690)
One option I've considered (but haven't implemented) is a placeholder element that grows in height as one accumulates visible sections. The IntersectionObserver would allow this to be modified after layout, preventing another reflow as you scroll, and letting you keep content-visibility: auto on the content itself. Window resizing is trickier, and if you were to grow the window in width, I presume you'd want to remove the scroll tentpole element and start over, but I think it's possible to make things feel stable post-resize. Maybe I'll play with this approach a little.
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 contain-intrinsic-size, I'm sure it's something @tabatkins and @chrishtr are collecting examples of to evaluate. I anticipate many uses of these display locking features, and hopefully they'll use the popular patterns to figure out what to add next. In the mean time, this appears to work well and I'd love feedback to see if it makes things feel snappier on your document.
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!
@mathiasbynens, would you be up for dropping that snippet into your demo page so we can see how it feels?
Done. In particular, check the deep link demo:
- Before: https://mathiasbynens.github.io/ecma262-layout-chunking/before.html#sec-string.prototype.replaceall
- After: https://mathiasbynens.github.io/ecma262-layout-chunking/after.html#sec-string.prototype.replaceall
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.