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

Speed up the slow font-size gatherer #6386

Closed
paulirish opened this issue Oct 24, 2018 · 3 comments · Fixed by #10200
Closed

Speed up the slow font-size gatherer #6386

paulirish opened this issue Oct 24, 2018 · 3 comments · Fixed by #10200
Assignees

Comments

@paulirish
Copy link
Member

paulirish commented Oct 24, 2018

The font-size gatherer is fairly slow (500ms - 1500ms), especially on text heavy pages.

from #6347

@hoten:

Got some timings. https://gist.github.com/Hoten/de38179b2d941b6d195c3c4383d0bd8d
.... Everything that took longer than 1s seems related to font stuff (CSS.getComputedStyleForNode), which I think we are getting rid of soon? If not, we might consider 1) increasing the timeout for those commands or 2) instead of sending them all at once, send them in batches.

@brendankenny

That gatherer sends all those CSS.getComputedStyleForNode commands at once, up to 500, depending on the page's node count. It's possible command overhead/contention causes issues, especially on an older phone. We could try doing them in batches instead of all at once to see if that's a solution.

It's also remotely possible we hit some gross style-recalc issue, depending on the page and how the commands are coming in. It's possible it would be more efficient to do all the lookups in the page at once with evaluateAsync, since we could guarantee running them all in the same turn of the event loop/without reforcing layout. We'd have to resync node info and debugger nodeIds, though. Maybe there's a similar coalesced solution, though.


talked with dgozman and appear to be ~3 options, two of which brendan outlined above.

  1. batch the current CSS.getComputedStyleForNode calls. there can be up to 500. batching we'd avoid the timeout more safely, but this gatherer would still take a LONG time (~300-1000ms). this doesn't seem very compelling.
  2. evaluateAsync and do our own getComputedStyle inside the page. would be very fast to collect (~25ms). but matching up nodeId's might be a dealbreaker. There's not a good way to do this.
  3. Use DOMSnapshot.captureSnapshot. It collects more data than we need, and its payload is like 300kb. But it gives us computed style for the entire tree (including iframes) in 60ms. Has some potential, but need to explore it more. It's data structure is wild, but it looks like it has the data we need.

To try this out, do devtools-on-devtools and try:

await Main.sendOverProtocol('DOMSnapshot.captureSnapshot', {computedStyles: ['font-size']})
@connorjclark connorjclark self-assigned this Oct 25, 2018
@patrickhulce
Copy link
Collaborator

I dig the captureSnapshot direction, all should be much easier now with the rewrite! :D 🎉

@connorjclark
Copy link
Collaborator

Going with door number 3. The funky data structure is basically a Blink Document, but all the values are deduped and stored in .strings.

@connorjclark
Copy link
Collaborator

AFAICT you cannot get the nodeId from DOMSnapshot.captureSnapshot. Will need to call DOM.getFlattenedDocument, which will provide all Nodes + their backendId, which can be used to map the output of the DOM snapshot to a specific node.

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

Successfully merging a pull request may close this issue.

3 participants