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

Bad performance when rendering long list of links separated with commas. #2831

Closed
kasper93 opened this issue Jun 20, 2018 · 17 comments
Closed

Bad performance when rendering long list of links separated with commas. #2831

kasper93 opened this issue Jun 20, 2018 · 17 comments

Comments

@kasper93
Copy link

@kasper93 kasper93 commented Jun 20, 2018

Hi,

I noticed that one page works really slow with webrender enabled. It turned out to be a list of links. It tanks compositor performance and CPU backed to a lesser degree. With 5000 links I get 45ms mean when scrolling on decent CPU. And it spikes to 160ms when selecting text. You can see the attached screenshot for profiler view after little bit of scrolling the page.

Click for screenshot

webrender

It happens when links are separated with commas. Probably matter of having alternating text color or something.

Here is the page that you can reproduce the issue.

<!DOCTYPE html>
<html lang="en">
<head>
</head>
<body>
	<script>
		for (let i = 0; i < 5000; ++i) {
			const str = (Math.random() + 1).toString(36).substring(7);
			document.body.insertAdjacentHTML('beforeend', `<a href="https://example.com/">${str}</a>, `);			
		}
	</script>
</body>
</html>

Regards,
Kacper

@kvark
Copy link
Member

@kvark kvark commented Jun 20, 2018

I'd guess that we are switching the batches too match here, and transitioning towards a single image brush draw call for all transparent things would help. More investigation to follow.

@kasper93
Copy link
Author

@kasper93 kasper93 commented Oct 24, 2018

Hi,

Any news about this? I'm asking because recent version is actually 3 times slower than one tested in original report. Which makes even smaller lists to produce lags. Original testcase now takes 130 ms per frame in CPU compositor. Should I create issue on bugzilla, not sure where you track issues more?

Click for screenshot

webrender

@kvark
Copy link
Member

@kvark kvark commented Oct 25, 2018

Taking another look. We are making 30K draw calls, which is the major problems. Expected draw call count should be within 100. And the reason for this is because the text underline is a solid primitive, and we end up interlacing it with text primitives, breaking all the batches.

@Gankra
Copy link
Contributor

@Gankra Gankra commented Oct 25, 2018

Is this something picture caching can/should solve? (browsers do indeed observably draw these things interlaced, not sure if doing anything at the display-list level to deinterlace them would be strictly web compatible)

@kvark
Copy link
Member

@kvark kvark commented Oct 25, 2018

Picture caching is sort of a universal answer to anything:

  1. look, there is something stupid we do, and it costs us!
  2. oh, but with picture caching we'll only do it once!

I'd like to see an orthogonal solution here. For instance, what's stopping us to render all the underline (given that it's solid rects) in the opaque pass?

@Gankra
Copy link
Contributor

@Gankra Gankra commented Oct 25, 2018

I expect we could add an optimization that checks the line style and determines if it's opaque (so in this case checking that the style is "solid" should be sufficient). The fact that it's interleaved with definitely-non-opaque text isn't a problem for that?

@kvark
Copy link
Member

@kvark kvark commented Oct 25, 2018

Not at all. The problem only occurs when some semi-transparent stuff is interleaved with another. Shoving anything into the opaque pass is a huge win.

@kvark kvark self-assigned this Oct 25, 2018
@gw3583
Copy link
Collaborator

@gw3583 gw3583 commented Oct 25, 2018

The underlines, if they are line-style: solid, should be getting drawn as rectangles in the opaque pass. If they're not, that's a bug!!

@kvark
Copy link
Member

@kvark kvark commented Oct 25, 2018

@gw3583 I'm on it ;)

@kvark
Copy link
Member

@kvark kvark commented Oct 25, 2018

@gw3583 btw, would you see any issues drawing the dashes/dotted decorations in opaque pass, given that they need to be snapped?

@gw3583
Copy link
Collaborator

@gw3583 gw3583 commented Oct 25, 2018

There is AA on at least the dots and wavy styles, so they probably need to be alpha pass. The dashes are done via image repeat, so we'd probably need to introduce discard into brush_image, which doesn't seem ideal?

bors-servo added a commit that referenced this issue Oct 25, 2018
Render solid line decorations in the opaque pass

Fixes #2831
The draw call count drops from 30000 to 60 :)
When replaying the WR capture on my Geforce 1050 Ti, the refresh rate raises from 9fps to full 60fps.
Due to the batching changes, it may also (positively!) affect other primitives.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/3232)
<!-- Reviewable:end -->
@kvark
Copy link
Member

@kvark kvark commented Oct 25, 2018

Note that my #3232 only addresses the rendering and scrolling issues. There is more to this case: display lists are humongous, which is seen every time the selection changes. We spend > 50ms just sending the DLs and then about 30ms in the WR backend processing them...

@kasper93
Copy link
Author

@kasper93 kasper93 commented Oct 28, 2018

Note that my #3232 only addresses the rendering and scrolling issues

Improvement is huge. On 2560x1400 resolution I get around 30fps when scrolling. (i7 5820k and R9 390) but it is no longer visible on first glance in real websites. Still pure CPU rendering is faster tho.

There is more to this case: display lists are humongous, which is seen every time the selection changes. We spend > 50ms just sending the DLs and then about 30ms in the WR backend processing them...

On my end it takes 1800-2200 ms on DL IPC. Basically when anything is selected on page it tanks the performance. Maybe we should keep this issue open for further improvements?

@Gankra
Copy link
Contributor

@Gankra Gankra commented Oct 29, 2018

I think the "IPC" time might just be us choking on https://bugzilla.mozilla.org/show_bug.cgi?id=1435931#c4 which is largely an upstream gecko bug

@Gankra
Copy link
Contributor

@Gankra Gankra commented Oct 29, 2018

Yeah selecting text on my mac spends 24 seconds stuck computing text selection indices: http://bit.ly/2zbWKcq

You see this as a massive spike in the "DisplayList IPC" graph because that graph covers the time from starting to construct the display list to having fully consumed the display list in the webrender backend. The time we get stuck computing selections is done inside CreateWebrenderCommands for text display items (to determine style changes and whether to draw a box around that item), so it gets charged against display list construction time (which is in fact an accurate claim).

@Gankra
Copy link
Contributor

@Gankra Gankra commented Oct 31, 2018

this was improved a lot by @mattwoodrow in the lastest nightly, but it still takes multiple seconds for a selection change to appear on the screen in webrender

@Gankra
Copy link
Contributor

@Gankra Gankra commented Oct 31, 2018

@kvark kvark added the bugzilled label Nov 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.