Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve css performance #1644

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

marcustyphoon
Copy link
Collaborator

This pull request courtesy of the "CSS selector stats" feature in Chrome Devtools' performance profiler.

Description

I've been encountering some slow rendering on the Patio page, which is particularly noticeable with #1291 (not currently sure if this is due to a bug or just quirks in what virtual scroller code it results in triggering a lot). Looking into it, the virtual scroller/long post collapsing/tag collapsing code does a number of things that cause style recalculation on a per-post basis, which does finally result in a situation where CSS selector performance actually kind of matters. (If one has posts per column * column count posts on screen, each one requires a style recalc, and the style recalc involves running selectors on posts per column * column count * some number elements, we are in some vague sense O(n^2) on each.)

Therefore, it's worth profiling XKit's selectors and optimizing the few that have the most impact. Short answer: anything that ends in ~ * is bad, because that pattern is—sort of, in a vague sense—quadratic in complexity again if elements have many siblings. Changing the final selector to something that only has to be checked once rather than once-per-preceding element in these few instances significantly reduces the cost of CSS on the entire page.

I saw a ~2 second decrease in the time to navigate to Patio with profiling on, but it's less without the profiler running and much less without #1291 merged in at the moment for whatever reason (or, obviously, without the large number of columns I have enabled mostly to see how the performance is).

Note: for Vanilla Video I didn't actually change the pattern, but only running the ~ on divs and video elements is enough to drop it down to performing like its peers. Fun, potentially somewhat surprising fact: the vast majority of elements on a Tumblr page aren't divs!

const elements = [...document.querySelectorAll('*')];
console.log('total', elements.length);

const counts = {};
elements.forEach(({ nodeName }) => {
  counts[nodeName] ??= 0;
  counts[nodeName]++;
});
Object.keys(counts)
  .sort((a, b) => counts[b] - counts[a])
  .forEach(key => console.log(key, counts[key]));

// total 26302
// SPAN 6759
// DIV 5649
// path 4710
// symbol 3020
// svg 1629
// IMG 933
// A 891
// BUTTON 809
// use 706
// ...

Testing steps

  • Confirm that Trim Reblogs' UI, the footer tooltips tweak, and Vanilla Video still work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant