2008619 - [meta] Text shaping word cache affects performance on several SP3 tests (particularly TipTap)

Status: ASSIGNED

(Core :: Graphics: Text, enhancement)

Description

[Denis Palmeiro [:denispal]]

The word cache splits text into individual words and shapes each separately which creates a lot of overhead in the Editor-TipTap test during Speedometer3, even with good cache hit rates. When I run 100 iterations of TipTap on my macbook pro, we spend about 2240 ms text shaping when the word cache is enabled and 1193 ms when I skip it. Skipping the word cache leads to a 10% improvement in Editor-TipTap's performance..

Comment 1

[Denis Palmeiro [:denispal]]

Here is a profile: https://share.firefox.dev/4bqWI42

Comment 2

[Denis Palmeiro [:denispal]]

Since a lot of the overhead is in locking and realloc, maybe we can get away with a smaller fixed size MRU cache instead of the current implementation with mozilla::HashMap.

Comment 3

[Denis Palmeiro [:denispal]]

(In reply to Denis Palmeiro [:denispal] from comment #2)

Since a lot of the overhead is in locking and realloc, maybe we can get away with a smaller fixed size MRU cache instead of the current implementation with mozilla::HashMap.

I prototyped this but it doesn't seem to help much.

Comment 4

[Denis Palmeiro [:denispal]]

Attachment: Bug 2008619: WIP: Skip word cache for very long text runs.

For text runs longer than 1024 characters, skip the word cache and shape the entire run directly. This avoids per-word overhead (i.e. locking, reallocs, etc) that becomes inefficient in cases that have large text blocks. Improves Editor-TipTap by about 10%.

Comment 5

[Jonathan Kew [:jfkthame]]

The performance observation here is interesting. It used to be that the word cache gave us a (smallish but significant) perf boost on most text-heavy content, but I wonder if that's still true. In general, harfbuzz shaping performance has been steadily improving thanks to lots of good upstream work by Behdad; it may be that it's reached a point where the cache isn't really gaining us much.

So I guess I'm suggesting that before we do a threshold as per the patch here (which may be fine -- I'm not opposed to the idea), I'd be interested to know what happens (for perf tests in general, not just Editor-TipTap) if we disable the word cache altogether. It should be pretty simple to do that, by hacking gfxFont::SplitAndInitTextRun to just call ShapeTextWithoutWordCache directly. Would you be up for giving that a try?

Comment 6

[Worcester12345]

PERF key word?

Comment 7

[Denis Palmeiro [:denispal]]

(In reply to Jonathan Kew [:jfkthame] from comment #5)

The performance observation here is interesting. It used to be that the word cache gave us a (smallish but significant) perf boost on most text-heavy content, but I wonder if that's still true. In general, harfbuzz shaping performance has been steadily improving thanks to lots of good upstream work by Behdad; it may be that it's reached a point where the cache isn't really gaining us much.

So I guess I'm suggesting that before we do a threshold as per the patch here (which may be fine -- I'm not opposed to the idea), I'd be interested to know what happens (for perf tests in general, not just Editor-TipTap) if we disable the word cache altogether. It should be pretty simple to do that, by hacking gfxFont::SplitAndInitTextRun to just call ShapeTextWithoutWordCache directly. Would you be up for giving that a try?

Justin was interested in trying out different caching implementations so I transferred this bug to him. Justin, is this something you can also try out? Thanks!

Comment 8

[Justin Link]

Sorry, I was out sick yesterday so I haven't done anything here yet. I plan to start taking a look today and will try out Jonathan's suggestion.

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:lsalzman, could you have a look please?

For more information, please visit BugBot documentation.

Comment 10

[Justin Link]

I have tried four different ways of disabling the word cache (comparisons here here here and here).

Except on Windows non-ref hardware where anything that causes us to use the word cache less is a win, simply disabling the word cache results in a clear regression. In all of the cases, there are huge regressions in Editor-TipTap. On Windows, those huge Editor-TipTap regressions are sometimes offset by wins in other tests, although those seem less significant (even though the effect is that the overall SP3 score gets pulled upward).

I was surprised to see such a strong regression after Denis' earlier results so I re-implemented his change and tried a few different thresholds here and still see the improvements that he saw. Using a lower threshold than what he picked (1024) might also be better.

There are a few take-aways that I see from this:

1. The word cache is useful and valuable but we are definitely over-using it.
2. Non-ref Windows hardware seems to have a distinctly difference experience with the word cache. Perhaps the shaping itself is less expensive there for some reason so there is less benefit in caching the results? Maybe something aspect of the word cache executes less efficiently on that hardware which affects when it should be used?

I'm taking a few action items out of this:

1. Study how the tests that benefit from the word cache are shaping words vs the tests that don't and use that to determine the appropriate conditions for when we should try to skip it.
2. Look more closely into what's happening on non-ref Windows hardware. I triggered some performance profiles on these machines in CI and have started looking at the profiles. These are the profiles from the slowest of these runs. So far it's surprising because we aren't actually spending much time in the main code related to shaping and caching.
3. Try using a simpler data structure (without need for allocations and possibly without need for locking?) like the cache that I previously implemented for JS atoms.

Comment 11

[Justin Link]

It turns out that the word cache is still very helpful when it comes to performance. The reason that Denis' patch to restrict when we use the word cache made things faster was because it avoided doing something that sabotages the word cache. See bug 2030147 for more details.

I'm turning this into a meta-bug because there are still a few more avenues to follow-up on:

1. Pre-allocate the word cache with a larger size to avoid some initial re-allocs. A quick test of this seemed to indicate that this was a significant win on Linux and neutral on other desktop platforms.
2. Consider using a different data structure for the cache. If the "working set" is actually quite large, then the current data structure is probably well-suited. If not, an MRU cache or hash-based cache (similar to a CPU cache) might be a better choice.
3. Take a look at the locking that is used. In my tests, it seems like we're always doing the shaping from the main thread so maybe the locking isn't really necessary but, then again, maybe it's also not really harming us if there's no contention.

Comment 12

[Jonathan Kew [:jfkthame]]

The cache size/strategy is a tricky one, because it is so dependent on the nature of the content we're dealing with. Not to say it can't be improved, but we should be wary of focusing too much on just a few examples.

Regarding locking, for HTML content we always shape on the main thread, but since we implemented text rendering for offscreen canvas, it's also possible for us to do shaping from DOM worker threads. So that's why we had to add locking.

An alternative would be to avoid ever sharing font instances between threads, but that potentially has other downsides (increased memory usage, and losing the possibility of the word cache being usefully shared across threads -- e.g. if several workers are using the same font, they don't all have to shape the same words independently).