Closed Bug 1685744 Opened 4 years ago Closed 4 years ago

Scrolling on some pages hits the glyph cache limit, and the resulting churn causes unnecessary growth of the texture cache

Categories

(Core :: Graphics: WebRender, defect)

All
macOS
defect

Tracking

()

RESOLVED FIXED
86 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox84 --- wontfix
firefox85 --- wontfix
firefox86 --- fixed

People

(Reporter: mstange, Assigned: mstange)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

URL: https://ko.wikipedia.org/wiki/%EB%B2%84%EB%9D%BD_%EC%98%A4%EB%B0%94%EB%A7%88

If I short-circuit evict_items_from_cache_if_required so that it doesn't do any eviction, and keep scrolling up and down on this Korean wikipedia page, the texture cache keeps growing as more and more things are allocated.

You'd expect that it eventually has allocated all glyphs in the page and can reuse them. However, we never seem to reach that state.

In contrast, the Italian page reaches a stable state easily and stops growing the texture cache.

I figured it out.

There are two caches here, and their interplay causes unexpected behavior.
The GlyphCache has a limit of 6MB and the TextureCache has a limit of 64MB.

On this page, caching all the glyphs requires 11MB. However, 11MB is more than the glyph cache limit of 6MB, so the glyph cache starts evicting glyphs. However, it doesn't tell the texture cache about it - glyphs use Eviction::Auto in the texture cache, so the texture cache will evict them on its own, later. As we re-rasterize the glyphs that were evicted by the glyph cache, the texture cache grows, until it hits 64MB.

On the other hand, if we increase the glyph cache limit to, say, 20MB, then the entire page fits in the glyph cache. The glyph cache no longer evicts anything. As a result, the texture cache stops growing once all the glyphs are present, at around 23MB (= 11MB of glyphs, plus some images).

I'll repeat that, because it's very non-obvious:

  • With a glyph cache limit of 20MB, the texture cache tops out at 23MB.
  • With a glyph cache limit of 6MB, the texture cache grows until it hits its soft limit of 64MB.

I can think of two options to fix this:

  1. When the glyph cache decides to evict glyphs, we could actually evict the corresponding entries from the texture cache. This would keep the texture cache small.
  2. Or we could remove the glyph cache limit entirely, and just rely on the texture cache eviction. This would eliminate the "growth from churn".

I would prefer 2 because it's simpler. Glenn, do you have opinions on this?

Flags: needinfo?(gwatson)
Summary: Something is wonky with glyph reuse in the texture cache on this page → Scrolling on some pages hits the glyph cache limit, and the resulting churn causes unnecessary growth of the texture cache

I think a good question to ask is, "What's the point of the glyph cache eviction if it doesn't actually free up space in the texture cache?" I guess it allows deleting the native font handles, if all the glyphs of a font end up being evicted?

Lee, Glenn mentioned that you added the glyph cache limit and may have opinions on this.

Flags: needinfo?(lsalzman)

It looks like the glyph cache eviction in the past did evict the entries from the texture cache. But this code was removed in bug 1641751, in part 4, here.

Keywords: regression
Regressed by: 1641751
Has Regression Range: --- → yes
Flags: needinfo?(gwatson)

So the glyph cache limit was originally added in bug 1560520, for this testcase: attachment 9196608 [details]

I think a separate limit for glyphs does make sense, so that we don't evict images when we're only churning glyphs. But we can implement that limit in the texture cache itself, rather than in the glyph cache, so that we don't need to coordinate eviction between the two caches in both directions.

Flags: needinfo?(lsalzman)
Blocks: 1681339

I ran some old builds to see how much difference the original patch made. I couldn't detect a difference; builds from just before bug 1560520 and just after bug 1560520 (build IDs 20190717093640 and 20190718095458) both get around 40fps on the testcase. (This was on macOS 10.15.) With today's Nightly, which has broken glyph eviction, I get up to 55fps but the numbers are also jumping around wildly because of bug 1686810.
Anyway, since the glyph eviction code isn't doing anything useful at the moment, let's remove it. And in the near future we'll hopefully have equivalent (but working) eviction in the texture cache itself.

The limit wasn't doing anything useful anymore, because one of the recent texture
cache refactorings made it so that we weren't actually evicting these glyphs from
the texture cache. In the future, we can implement a similar limit in the texture
cache itself, by giving it per-cache-type limits rather than a global limit.

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: