font selection depends on language of first text viewed rather than current document language

RESOLVED FIXED

Status

()

Core
Graphics
P3
normal
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

({testcase})

Trunk
x86
Linux
testcase
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.9.1 +
blocking1.9 -
wanted1.9.0.x -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

9 years ago
Created attachment 314989 [details]
testcase

This happens when the page requests only generic fonts.

It would affect ja users, for example, when a particular (including default) font size is used in a Japanese page, as well as a subsequently viewed Latin page.  The Latin page would be displayed with Japanese fonts.

Bug 416725 comment 16 has more information.
The patch in bug 424622 would fix this.
Flags: blocking1.9?
(Assignee)

Comment 1

9 years ago
Created attachment 314990 [details]
current behavior
(Assignee)

Comment 2

9 years ago
Created attachment 314991 [details]
expected behavior
Flags: wanted1.9.0.x?
Flags: blocking1.9?
Flags: blocking1.9-
Flags: wanted1.9.1+
Flags: wanted1.9.0.x?
Flags: wanted1.9.0.x-
Priority: -- → P3
(Assignee)

Updated

9 years ago
Assignee: nobody → mozbugz
Depends on: 449356
(Assignee)

Comment 3

9 years ago
I threw the quick fix in attachment 305300 [details] [diff] [review] at the tryserver.  This includes
the language in the hash used to key the gfxPangoFontCache.

Interestingly, although this means that fewer family/style combinations map to
the same PangoFont, it reduces Tp by 1.3% (449->339, which looks statistically
significant).  The speed up probably results from reducing the need to
fallback to non-primary fonts for the gfxFontGroup.

There was a small increase in tp_RSS from 131 to 132 MB.  (More fonts are
stored in the cache.)

If were wanting to fix this bug in this way, then we should key the cache by
PangoFontDescription and language rather than just a hash, but that would make
the cache consume more memory, and I don't think we want to keep the
gfxPangoFontCache.
(Assignee)

Comment 4

9 years ago
(In reply to comment #3)
> it reduces Tp by 1.3% (449->339, which looks statistically significant).

Bad math there.

It reduces Tp by 2% (449 -> 439).
(Assignee)

Comment 5

9 years ago
Removing the gfxPangoFontCache altogether also fixes this bug.

A tryserver run shows that removing this cache can save 5-6% on memory use
(131/132 -> 124 MB).
However, the loss of caching has a big effect on page load time (449 -> 719).

The gfxFontCache serves a very similar purpose to the gfxPangoFontCache and
similar talos Tp numbers can be achieved without the gfxPangoFontCache by
increasing the expiration time on the gfxFontCache such that it is about the
time required for one talos iteration through the pageset.  On the tryserver,
this is about 3 minutes.

A tryserver run with no gfxPangoFontCache and a gfxFontCache timeout of 3
minutes gives a 5-6% improvement in Tp over the current situation (449 -> 424),
a 3% improvement over attachment 305300 [details] [diff] [review].
(Assignee)

Comment 6

9 years ago
Using the gfxFontCache instead of the gfxPangoFontCache increased the talos
Tp_RSS number by 3%.  Possibly this is due to gfxFontCache also keeping a
reference to the cairo scaled font whereas gfxPangoFontCache only keeps a
reference to the PangoFont.

However I don't think many users look at 400 pages in 3 minutes.  The user is
much more likely to see the 5% reduction in memory use, when the fonts expired
from the cache.
(Assignee)

Comment 7

9 years ago
The effect of increasing the gfxFontCache expiry time from 10 seconds to 3
minutes was also measured on other platforms.

On Windows, Tp improved only 1% (445 -> 441 ms),
and reported Tp_memset increased 1% (77 -> 78 MB).

On Mac, Tp improved by 4-5% (313 -> 299 ms),
and reported Tp_RSS *decreased* 2% (325 -> 319 ?units?).
The Tp_RSS decrease looks statistically significant but is hard to explain.

I don't know the effects on the memory usage of the system font servers
(guessing there are system font servers on these platforms).
(Assignee)

Comment 8

9 years ago
Created attachment 332880 [details] [diff] [review]
Use gfxFontCache instead of gfxPangoFontCache

It's clear we need to change the timeout with Pango to drop gfxPangoFontCache.
Perhaps we should change the timeout on Mac also, but I'll let someone who
knows the Mac code better make that decision.

This differs from attachment 314097 [details] [diff] [review] in that it doesn't try to do any rounding
of sizes.  Perhaps we should investigate that (bug 424622), but that's really
a separate issue.  For now, i want to get gfxPangoFontCache out of the way.
Attachment #332880 - Flags: review?(vladimir)
(Assignee)

Comment 9

9 years ago
Comment on attachment 332880 [details] [diff] [review]
Use gfxFontCache instead of gfxPangoFontCache

Withdrawing review request for now because a gfxFontGroupCache looks like it would be a better solution, because the (time consuming) font selection is the role of the gfxFontGroup and so that seems a better place to cache the results.
Attachment #332880 - Flags: review?(vladimir)
(Assignee)

Comment 10

9 years ago
Fixed in bug 449356.

In testsuite:
http://hg.mozilla.org/mozilla-central/rev/707da21015e7
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Attachment #332880 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.