Last Comment Bug 334064 - cairo-gtk2 builds are unusably slow due to font code
: cairo-gtk2 builds are unusably slow due to font code
Status: RESOLVED FIXED
: perf
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86 Linux
: -- normal with 2 votes (vote)
: ---
Assigned To: Mark Steele
:
Mentors:
http://www.mozilla.com/
Depends on:
Blocks: 334720
  Show dependency treegraph
 
Reported: 2006-04-14 16:37 PDT by David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch)
Modified: 2010-09-17 19:36 PDT (History)
26 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Xft gfxTextRun fallback for the ASCII path (6.33 KB, patch)
2006-08-10 16:23 PDT, Mark Steele
pavlov: review+
vladimir: superreview+
Details | Diff | Splinter Review
Update the previous patch to handle more cases. (11.71 KB, patch)
2006-08-24 16:26 PDT, Mark Steele
vladimir: superreview+
Details | Diff | Splinter Review
Fix stupid whitespace droppings. (10.18 KB, patch)
2006-08-24 17:55 PDT, Mark Steele
pavlov: review+
pavlov: superreview+
Details | Diff | Splinter Review
pangocairo-lite (212.25 KB, application/x-bzip2)
2006-09-12 16:27 PDT, Behdad Esfahbod
no flags Details

Description David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2006-04-14 16:37:03 PDT
In a few days of using Cairo builds, I've noticed that, despite bug 333251's dependencies being fixed, the builds are still unusably slow on many Web sites.

It probably doesn't help that I have a lot of fonts installed (see below) -- basically all the truetype font packages that come with FC5, plus a few others.

One example of this is the following steps to reproduce:
 1. load http://www.mozilla.com/ in one tab
 2. load http://www.mozilla.com/firefox/all.html in another tab
 3. switch between the tabs using Ctrl-PgUp/PgDn

It takes a few seconds to switch from tab to tab.

A realtime jprof profile (which includes some idle time) shows that:
 * 76% of the time is spent painting
   + presumably mostly the HTML documents
 * 13% of the time is spent reflowing
   + entirely the XUL document for the browser

 * 82.5% of the time is spent within gfxPangoTextRun::EnsurePangoLayout
   + this happens both during painting and reflow
   + 64.5% of the total time (78% of EnsurePangoLayout) is in FcFontSort
 * 11% of the time is spent within XftLockFace
   + all of that is within painting

So I'd make the observation that there are at least two dependencies to making this perform acceptably:

 1. fixing whatever rearchitecture needs to happen so that we don't need to redo all of this work for every paint

 2. porting bryner's work in bug 223813, which avoids unneeded calls to FcFontSort (plus followup regression fixes, of course), to whatever's doing our interaction with fontconfig (currently pango)

I'm switching back to non-cairo builds because of this, and I seriously question whether cairo should be turned on for our GTK2 builds as long as these performance problems are present.
Comment 1 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2006-04-14 16:42:42 PDT
Oh, a few more details:
 + I should have been clearer that XftLockFace is also within EnsurePangoLayout (but not within FcFontSort -- I originally had listed only FcFontSort there)
 + 98.5% of the time within reflow is in EnsurePangoLayout
   + 89.5% of the time within reflow is in FcFontSort
 + 91.4% of the time within painting is in EnsurePangoLayout
   + 69.2% of the time within painting is in FcFontSort
   + 14.3% of the time within painting is in XftLockFace
Comment 2 Dennis Jacobfeuerborn 2006-04-14 16:55:15 PDT
I cannot reproduce the behaviour described above but then I only have the regular fonts that come with a normal FC5 installation. Can adding some fonts really have an impact of a few seconds in this case?
Comment 3 Vladimir Vukicevic [:vlad] [:vladv] 2006-04-14 17:31:23 PDT
So I put a patch in bug 330064 that was fixing something similar; it hasn't gone anywhere yet, but I'm thinking about just checking it in.  You might want to try with that patch, and see how much of a difference it makes.

The freedesktop.org bug that I've filed, https://bugs.freedesktop.org/show_bug.cgi?id=6227 , has been ignored since I filed it (which is pretty typical).
Comment 4 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2006-07-26 11:23:38 PDT
I filed a bug on pango:
http://bugzilla.gnome.org/show_bug.cgi?id=348825 to make the same optimization we made in bug 223813.
Comment 5 Mark Steele 2006-08-10 16:23:31 PDT
Created attachment 233172 [details] [diff] [review]
Xft gfxTextRun fallback for the ASCII path

This still abuses pango for font selection but avoids it for measuring.
Comment 6 Stuart Parmenter 2006-08-23 15:38:42 PDT
Comment on attachment 233172 [details] [diff] [review]
Xft gfxTextRun fallback for the ASCII path

remove the printfs.  we should get this in to the tree.
Comment 7 Vladimir Vukicevic [:vlad] [:vladv] 2006-08-23 15:50:51 PDT
Comment on attachment 233172 [details] [diff] [review]
Xft gfxTextRun fallback for the ASCII path

> 
>+#define XFT_MEASURE 1
>+
>+gfxTextRun*
>+gfxPangoFontGroup::MakeTextRun(const nsACString& aString)
>+{
>+#ifdef XFT_MEASURE
>+    return new gfxXftTextRun(aString, this);
>+#else
>+    return new gfxPangoTextRun(NS_ConvertASCIItoUTF16(aString), this);
>+#endif
>+}

Call this something other than XFT_MEASURE -- maybe USE_XFT_FOR_ASCII or something.


Looks fine otherwise, though.
Comment 8 Boris Zbarsky [:bz] 2006-08-23 18:35:34 PDT
I tried testing that patch, but it doesn't compile (presumably needs merging to tip?):

../../../../mozilla/gfx/thebes/src/gfxPangoFonts.cpp:180: cannot allocate an 
   object of type `gfxXftTextRun'
../../../../mozilla/gfx/thebes/src/gfxPangoFonts.cpp:180:   because the 
   following virtual functions are abstract:
../../../dist/include/thebes/gfxFont.h:202:     virtual nsrefcnt 
   gfxTextRun::AddRef()
../../../dist/include/thebes/gfxFont.h:202:     virtual nsrefcnt 
   gfxTextRun::Release()

I'd love a working version of the patch to run on some testcases...
Comment 9 Masayuki Nakano [:masayuki] (Mozilla Japan) 2006-08-24 08:31:50 PDT
Can this patch drawing the intl text? (CJK, complex scripts and RTL text)
Comment 10 Mark Steele 2006-08-24 16:26:41 PDT
Created attachment 235325 [details] [diff] [review]
Update the previous patch to handle more cases.

Strings passed down from layout as ASCII or strings passed as Unicode in the ASCII range will be handled faster with this patch. The rest are handled by the original Pango path. The updated patch adds a check to the Unicode path for compatible strings to steal to the Xft path.
Comment 11 Vladimir Vukicevic [:vlad] [:vladv] 2006-08-24 17:39:35 PDT
Comment on attachment 235325 [details] [diff] [review]
Update the previous patch to handle more cases.

>Index: gfx/thebes/src/gfxPangoFonts.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/gfx/thebes/src/gfxPangoFonts.cpp,v
>retrieving revision 1.30
>diff -u -r1.30 gfxPangoFonts.cpp
>--- gfx/thebes/src/gfxPangoFonts.cpp	9 Aug 2006 21:36:37 -0000	1.30
>+++ gfx/thebes/src/gfxPangoFonts.cpp	24 Aug 2006 23:16:56 -0000

>@@ -530,7 +597,7 @@
>     mMetrics.subscriptOffset = mMetrics.xHeight;
> 
>     pango_font_metrics_unref (pfm);
>-#endif
>+    #endif

^ broken



>+    } else {
>+        printf ("SPACE ME!\n");
>+    }
>+

I'd remove this printf, or at least make it dependant on DEBUG.  It doesn't do anything more than a // XXX fixme would do, though.
Comment 12 Mark Steele 2006-08-24 17:55:11 PDT
Created attachment 235341 [details] [diff] [review]
Fix stupid whitespace droppings.
Comment 13 Stuart Parmenter 2006-08-29 13:22:53 PDT
patch checked in.
Comment 14 dolphinling 2006-08-29 13:41:21 PDT
I love you all.

Is this actually completely fixed, or is there still more work to be done? (Or is what still needs to be done getting the Pango and XFT bugs fixed?)
Comment 15 Stuart Parmenter 2006-08-29 13:52:55 PDT
i'm sure there is more to do but this makes a huge difference. I would say it is no longer "unusably slow"
Comment 16 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2006-08-29 19:49:43 PDT
...for users with Latin-based languages only?
Comment 17 Boris Zbarsky [:bz] 2006-08-29 20:20:10 PDT
For ASCII users only.  Any text fragment that has a non-ASCII char gets the old slow code.
Comment 18 Behdad Esfahbod 2006-08-29 22:39:49 PDT
I confess I'm totally lost.  Are you using PangoXft with cairo?!
Comment 19 Mark Steele 2006-08-30 13:05:48 PDT
(In reply to comment #18)
> I confess I'm totally lost.  Are you using PangoXft with cairo?!
> 

No. The file is possibly misnamed now. The intention was to add a fast path that (at the very least) *avoids* using Pango for measuring the types of strings we need to measure. This second path was wedged in with the existing Pango code but it uses Xft alone to measure and get the glyph indices. Glyphs are then (still) rendered with the cairo font API.
Comment 20 Masayuki Nakano [:masayuki] (Mozilla Japan) 2006-08-30 13:20:36 PDT
This patch breaks Japanese text that has ASCII characters...
If the text is:
aaaaaaBBBBBBB

Assume that the 'a' is an ASCII character, and 'B' is an Japanese character.
If selecting as following:

aaaaaaBBBBBBB
  |<---->|

The first fragment is rendered by Xft. But the 'a's of the second fragment are rendered by pango. I think that we should not separate the gfxTextRun. I think that we should merge the gfxTextRuns after bug 339513.
Comment 21 Behdad Esfahbod 2006-08-31 12:18:47 PDT
(In reply to comment #19)
> (In reply to comment #18)
> > I confess I'm totally lost.  Are you using PangoXft with cairo?!
> > 
> 
> No. The file is possibly misnamed now. The intention was to add a fast path
> that (at the very least) *avoids* using Pango for measuring the types of
> strings we need to measure. This second path was wedged in with the existing
> Pango code but it uses Xft alone to measure and get the glyph indices. Glyphs
> are then (still) rendered with the cairo font API.

I perfectly understand that.  But it doesn't change a thing.  You are using PangoXft (pango_xft_font_get_font for example) and cairo, bridging in between yourself, instead of using PangoCairo.  I won't comment on the level of pain that brings, and let alone what a broken design it is, but it does mean that I can't and won't spend time making it faster as all my focus is in PangoCairo...
Comment 22 Behdad Esfahbod 2006-08-31 12:22:47 PDT
For the record, this approach means duplicate caches (in cairo and in Xft), and not enjoying the optimization work that Federico and I did in pangocairo last fall, *including* a very fast cache for glyph extents in pangocairo.
Comment 23 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2006-08-31 17:12:43 PDT
If defeating a very fast cache makes us much faster, then the very fast cache doesn't seem to be much help in this case.
Comment 24 Behdad Esfahbod 2006-08-31 19:41:55 PDT
(In reply to comment #23)
> If defeating a very fast cache makes us much faster, then the very fast cache
> doesn't seem to be much help in this case.

No, you don't get it.  From what I see and feel, you are not using PangoCairo at all.  I will feel a lot better if proved wrong.
Comment 25 Boris Zbarsky [:bz] 2006-08-31 22:10:10 PDT
You're correct that PangoCairo is not used, except on BeOS.  We'd love to use it, but aren't in a position to do so due to the requirements it has.  We'd love for those requirements to change so we can use it.

Specifically, last I checked PangoCairo needs pango 1.10 or higher.  See bug 305649 comment 8.  For reference, FC4 ships with pango 1.8.  FC3 shipped (as recently as end of 2004) with pango 1.6.  Our current code (using CairoXft) requires pango 1.6, which is still a huge jump in requirements over what we require for Gecko 1.8, but we're _really_ not in a position to require that all of our Linux users upgrade to FC5 or newer.

If you can make PangoCairo work with pango 1.6, we should be able to use it, and that would be very nice.  If it requires 1.8, we can at least think about it....  But the current runtime requirements of PangoCairo simply make it unusable for our purposes.  So we've had to work around that by using PangoXft for fonts, with resulting problems all around.
Comment 26 Mark Steele 2006-09-01 02:27:27 PDT
I would suggest that it's not a very strong position to jump in to a thread with "I confess I'm totally lost." and then follow up with a "No, you don't get it.", but that's perhaps a style choice.
I think it's a good time to reiterate something that's been mentioned several times in this thread, including once when specifically asked: the intention was to avoid using Pango.
If the gripe was "yes, but you're still (mis)using a piece of API", then perhaps something more constructive could have been added to this bug. Yet even if that were the case, the point of this patch has been missed completely since, *obviously*, that all could have been implemented (and has been!) with purely fontconfig & Xft calls. It turns out it's faster and more interesting in the short term for several reasons to use the same code paths that the other Pango code is eventually forced to use. After all, this optimization is a *special case* for only a few languages. If this "design" still bothers you, consider it an work-in-progress experiment. Instead, the post went out of its way to say "I won't comment" and "I won't make it faster".
I'll have to assume this is in reference to 
http://bugzilla.gnome.org/show_bug.cgi?id=348825 the cache-miss-as-norm behavior of the fontset cache that is very clearly exhibited with with the slightest experimentation (You've done some right? How is that sorting several times per page going?).
If this is the case, the hostile attitude is very unfortunate for users of Pango, especially obscure corner cases like, you know, web browsers.
It's especially important to remember that this particular cache is the very same cache that misses many times per page load *using* PangoCairo, which impairs page load times even WITHOUT drawing a single string. Additionally, pango_shape also stands way out. So, it turns out you're right in that point that I was NOT enjoying the optimization work done last fall!
So, one more time, there are performance wins because this patch largely *avoids* using Pango to measure. It's an unfortunate side note that it was only after this that a comment on Pango performance was made from someone working on Pango.
Since I'm such a glass-is-half-full type, I see this as a great opportunity for rapid improvement, at least for users on future platforms!
Comment 27 Boris Zbarsky [:bz] 2006-09-01 21:38:53 PDT
For what it's worth, I profiled the testcases on bug 64858 with this patch.  Overall cairo time is now only about 20% slower than GTK2/XFT time (down from 400% slower).  nsThebesFontMetrics::GetWidth is now only about 2.5x slower than nsFontMetricsXft::GetWidth (down from 100x or so).

The breakdown for nsThebesFontMetrics::GetWidth is about like so:

gfxXftTextRun::Measure takes about 20% more time than nsFontMetricsXft::GetWidth (and seems to be doing about the same things).

gfxXftTextRun::~gfxXftTextRun and gfxPangoFontGroup::MakeTextRun together take almost as much time as gfxXftTextRun::Measure.  Caching text runs in nsTextFrame or something (like we plan to, right?) would be a nice win there.

There seems to be a lot of string object creation and destruction around, with temporary nsDependentCSubstrings floating about.  If the "call gfx with char pointer" case is the common one (and I think it is), would it make sense to make gfxXftTextRun::gfxXftTextRun take an nsDependentC?String& as an arg and make the member a nsDependentC?String reference instead of an actual object?  Or give it constructors that take char* and length?  I guess the problem is that MakeTextRun takes an nsAC?String.  But maybe we should change that (e.g. add other signatures)?
Comment 28 Boris Zbarsky [:bz] 2006-09-01 21:40:58 PDT
We should file separate bugs on any concrete plans along those lines, btw.

Behdad Esfahbod, please do let me know what the dependency story of PangoCairo is; e-mail me if you don't want to clutter the bug up with it.  We'd really like to use whatever is being most worked on and most optimized, if we can.
Comment 29 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2006-09-01 23:06:38 PDT
(In reply to comment #27)
> Or give it constructors that take char* and length?  I guess the problem is
> that MakeTextRun takes an nsAC?String.  But maybe we should change that (e.g.

New APIs should not take nsAC?String.  They should take nsC?Substring (or nsC?String, if appropriate).  nsAC?String exists only for backwards compatibility with the old string API; nsC?Substring is equivalent, but without the vtable pointer magic.
Comment 30 Mark Steele 2006-09-01 23:21:02 PDT
I noticed the string object overhead myself too after the first version of patch. It was easier to see this sticking out in profiles with the patch speedup and I changed the nsCString to nsDependentCString. As these strings are just (uni)char * up in nsTextFrame, I was wondering if we couldn't just use those straight through. Should this move to a new bug?
Comment 31 Boris Zbarsky [:bz] 2006-09-01 23:27:45 PDT
Yeah.  Mark, you want to file?  If it doesn't happen by Monday, I'll do it, but I'm basically offline starting now and until then.
Comment 32 Vladimir Vukicevic [:vlad] [:vladv] 2006-09-02 17:04:29 PDT
(In reply to comment #27)
> gfxXftTextRun::Measure takes about 20% more time than
> nsFontMetricsXft::GetWidth (and seems to be doing about the same things).

I'd guess this is due to us forcing layout to do word-by-word measurement; going around pango seems to be an overall win though, and we can probably wait until the textframe rewrite is done to see what happens to the numbers.

There's some code to use pango-cairo in the tree right now, enabled by a #define; I'm not 100% sure it still works, but it would be interesting to fix it up and get performance numbers for using pango-cairo for everything.  If we can get those numbers to be in line with the Xft numbers, then we can investigate going down the somewhat painful route of doing runtime pango version checking and using pango-cairo if it's available, otherwise falling back to pango-xft.  This would mean we could have the "new-world" code in the tree, already being used and tested, letting us just jettison the old code at some point post-gecko 1.9/1.10/2.0/whatever once we're ready to increase our linux platform requirements.
Comment 33 Jeff Walden [:Waldo] (remove +bmo to email) 2006-09-02 19:05:04 PDT
(In reply to comment #30, comment #31)

The method signature changes wrt string types are bug 351135.
Comment 34 Behdad Esfahbod 2006-09-11 15:34:04 PDT
Sorry for the long delay.

(In reply to comment #25)
> You're correct that PangoCairo is not used, except on BeOS.  We'd love to use
> it, but aren't in a position to do so due to the requirements it has.  We'd
> love for those requirements to change so we can use it.
>
> Specifically, last I checked PangoCairo needs pango 1.10 or higher.  See bug
> 305649 comment 8.  For reference, FC4 ships with pango 1.8.  FC3 shipped (as
> recently as end of 2004) with pango 1.6.  Our current code (using CairoXft)
> requires pango 1.6, which is still a huge jump in requirements over what we
> require for Gecko 1.8, but we're _really_ not in a position to require that all
> of our Linux users upgrade to FC5 or newer.

Well, pangocairo is as old as cairo itself, and requires a pango version of the same age.  It may be possible to use pangocairo with older versions of pango if someone does the job of backporting it, but from the upstream point of view, pangocairo is part of the pango distribution and released with pango.  there have been three stable versions of pango shipping with it now: 1.10, 1.12, and 1.14.

I would argue that by the time that Firefox want so ship with pangocairo, FC5 is not such a huge requirement.  Earlier Fedora versions don't have cairo either afterall.  I know mozilla currently has its own cairo copy, but isn't the plan to remove that and use upstream when all the patches are merged (almost there)?

 
> If you can make PangoCairo work with pango 1.6, we should be able to use it,
> and that would be very nice.  If it requires 1.8, we can at least think about
> it....  But the current runtime requirements of PangoCairo simply make it
> unusable for our purposes.  So we've had to work around that by using PangoXft
> for fonts, with resulting problems all around.

So, if I understand it correctly the code paths in question use Pango unconditionally, right?  In the Firefox 1.5 series that Pango is optional, requiring pangocairo is not such a big deal I assume.

Anyway, basing your future rendering on the cairo+pangoxft set of libraries doesn't sound right to me.

Also, about the patch in this bug, it will make a difference in rendering of Latin.  Pango now enabled OpenType Layout processing for Latin fonts.  This means, the Xft measurements don't take into account things like ligatures, while the pango path does.


(In reply to comment #26)
> I would suggest that it's not a very strong position to jump in to a thread
> with "I confess I'm totally lost." and then follow up with a "No, you don't get
> it.", but that's perhaps a style choice.

I didn't know that expressing my confusion may be offensive.  Put it on me being totally shocked.


> I think it's a good time to reiterate something that's been mentioned several
> times in this thread, including once when specifically asked: the intention was
> to avoid using Pango.
> If the gripe was "yes, but you're still (mis)using a piece of API", then
> perhaps something more constructive could have been added to this bug. Yet even
> if that were the case, the point of this patch has been missed completely
> since, *obviously*, that all could have been implemented (and has been!) with
> purely fontconfig & Xft calls. It turns out it's faster and more interesting in
> the short term for several reasons to use the same code paths that the other
> Pango code is eventually forced to use. After all, this optimization is a
> *special case* for only a few languages.

But even for those few languages, it makes a difference.  For 'ff', 'fi', 'ffi', and similar ligatures that many decent fonts have.


> If this "design" still bothers you,
> consider it an work-in-progress experiment.

Ok.


> Instead, the post went out of its
> way to say "I won't comment" and "I won't make it faster".
> I'll have to assume this is in reference to 
> http://bugzilla.gnome.org/show_bug.cgi?id=348825

I said: "I can't and won't spend time making it faster as all my focus is in PangoCairo".  I don't think that needs more explanation.  And this is not in response to any other bug.  Chris Aillon sent me this bug after seeing I'm working on making firefox+pango faster.

> the cache-miss-as-norm
> behavior of the fontset cache that is very clearly exhibited with with the
> slightest experimentation (You've done some right? How is that sorting several
> times per page going?).

You know that I'm working on that problem and have already increased the cache size for example.


> If this is the case, the hostile attitude is very unfortunate for users of
> Pango, especially obscure corner cases like, you know, web browsers.

Which hostile attitude?  That my focus is on pangocairo?


> It's especially important to remember that this particular cache is the very
> same cache that misses many times per page load *using* PangoCairo, which
> impairs page load times even WITHOUT drawing a single string.

"WITHOUT drawing a single string" doesn't mean much, as measurement and layout is harder than drawing.

> Additionally,
> pango_shape also stands way out. So, it turns out you're right in that point
> that I was NOT enjoying the optimization work done last fall!

pango_shape is very hard to optimize.  Most of the time it takes (in pango >= 1.12) is in the Open Type code.


> So, one more time, there are performance wins because this patch largely
> *avoids* using Pango to measure. It's an unfortunate side note that it was only
> after this that a comment on Pango performance was made from someone working on
> Pango.

What is surprising though, is that no one ever reported any bug about "pango is slow for firefox in here and here" earlier than this past July (ie, three months ago.)


> Since I'm such a glass-is-half-full type, I see this as a great opportunity for
> rapid improvement, at least for users on future platforms!

Agreed.  So, what should I do?  Open a bug "please use pangocairo"?
Comment 35 Stuart Parmenter 2006-09-11 16:00:20 PDT
(In reply to comment #34)
> I would argue that by the time that Firefox want so ship with pangocairo, FC5
> is not such a huge requirement.  Earlier Fedora versions don't have cairo
> either afterall.  I know mozilla currently has its own cairo copy, but isn't
> the plan to remove that and use upstream when all the patches are merged
> (almost there)?
> 
afaik, the plan is for us to ship our own version of cairo with our builds.  What upstream distributors do is up to them and the Firefox trademark guidelines.

> ... ligature stuff ...
While having support for ligatures from pango would be nice, we simply can't take a 500% slowdown for them.  We want to special case any non-complex text to avoid pango entirely because it is so slow for so little gains.  The slowness is  pango_shape and pango_itemize.  I realize these certainly aren't the easiest functions to optimize, but they are where the performance problems live.  I don't want to speculate how to fix them as I haven't looked at the code in a long long time and haven't looked at recent performance profiles.

> ... pangocairo vs pango+xft stuff ...
The pangocairo vs pango+xft argument should really go in a seperate bug.  As far as I know it has no real performance implications.  We'll move to using pangocairo once at least our developers have machines that have pangocairo installed on them which wasn't the case when we stopped using pangocairo.  I would like to see distributors ship updated pango versions to their older releases so that we can use pangocairo on them.  Dropping some % (not going to speculate on numbers here) of potential linux users for something we can work around seems silly, even if pangocairo is clearly what we want.
Comment 36 Boris Zbarsky [:bz] 2006-09-11 18:09:50 PDT
> I would argue that by the time that Firefox want so ship with pangocairo

This would be in 6 months, right?  And all developers would need to have such machines right now?

> FC5 is not such a huge requirement. 

Ah.  You're one of those people who're so common in the Linux distro world who assumes everyone upgrades their OS as soon as the next version comes out.  Sorry, but that's just not the case.  There's no way we can ship in 6 months requiring FC5, imho.

> isn't the plan to remove that and use upstream when all the patches are
> merged

We'll continue shipping cairo libs with our product until such a time as they're widely available as part of OSes, as I understand.  I think pavlov covered the details here.

> the code paths in question use Pango unconditionally

Yes.

> requiring pangocairo is not such a big deal I assume.

If it gets backported, sure.  There's a big difference between requiring an OS that shipped sometime in the last 3 years (what we plan to do now) and an OS that shipped sometime in the last 9 months (what you're proposing we do).  So yes, as things stand requiring pangocairo would be a big deal.

> basing your future rendering 

We'd be happy to base our rendering on the "right thing" if the "right thing" actually existed in users' OSes.  If it doesn't, we can't until such a time as it does.  It's really that simple.

> This means, the Xft measurements don't take into account things like
> ligatures

For what it's worth, last time I profiled the pango measurement path (just the measurement) was about 120 times slower than the Xft path.  This is on FC4 here, with PangoXft.  Pageload performace was 400% slower with pango than without.  I clearly can't test PangoCairo because I'd need to upgrade my OS first.

Ligatures are nice (in fact, anything that gets us closer to what TeX does is generally nice), but not with this sort of perf hit....

Again, it's unfortunate that as things stand we're unable to profit from your PangoCairo performance work.  Anything that would change that situation would be welcome.

> earlier than this past July (ie, three months ago.)

That's about when we started actually trying to figure out why the builds using pango were so slow.  We didn't start using pango much before that; and before that we were using Xft directly.

> So, what should I do?  Open a bug "please use pangocairo"?

Get pangocairo backported into pango versions that are actually shipped to an overwhelming majority of Linux users so we don't have to give people the "to use our browser you MUST update your operating system" crap that will make them simply not use our browser?

Comment 37 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-09-12 03:15:01 PDT
A few extra points...

Distributors (e.g., Redhat and SuSE) have in the past been forced to update Firefox to new major versions even on their stable (supported for several years) product lines, in order to keep up to date with security fixes. This may or may not happen again, but it seems wise to keep the dependency requirements at a minimum just in case it does.

Let's not lose sight of vlad's comment #32. We can use pangocairo if available at runtime, if there's a benefit.

Just in case some people aren't aware, the Redhat Pango patch for FF1.5/2, and our current trunk cairo/Pango code, abuse Pango badly. They create Pango objects every time we draw a string or measure text *word by word*. The new textframe code should help a lot, by letting us create Pango objects just once, for large chunks of text (per text node), and thus considerably reduce the 400% pageload hit that Boris quoted. So it's premature to draw specific conclusions about the whole-browser performance impact of Pango ... but it will still be significant, for sure.

I would really really really like to have a solution for non-complex text that gives us ligatures and kerning with good performance, and I don't care what it's called or who owns it. As an eternal optimist, I refuse to believe it's impossible.
Comment 38 Behdad Esfahbod 2006-09-12 10:26:59 PDT
(In reply to comment #35)
> (In reply to comment #34)
> > I would argue that by the time that Firefox want so ship with pangocairo, FC5
> > is not such a huge requirement.  Earlier Fedora versions don't have cairo
> > either afterall.  I know mozilla currently has its own cairo copy, but isn't
> > the plan to remove that and use upstream when all the patches are merged
> > (almost there)?
> > 
> afaik, the plan is for us to ship our own version of cairo with our builds. 
> What upstream distributors do is up to them and the Firefox trademark
> guidelines.
> 
> > ... ligature stuff ...
> While having support for ligatures from pango would be nice, we simply can't
> take a 500% slowdown for them.  We want to special case any non-complex text to
> avoid pango entirely because it is so slow for so little gains.

I appreciate it if you point me to benchmarks such that I can reproduce the 500%.  The Pango-based Firefox shipped in Fedora is definitely not 500% slower.  Not from benchmarks I've seen.  There's the fontset caching problem, but that's been improved and I'm still working on it.  And like Robert pointed, the pango backend in Firefox 1.5 is doing really bad things with Pango.

(In reply to comment #36)
> > I would argue that by the time that Firefox want so ship with pangocairo
> 
> This would be in 6 months, right?  And all developers would need to have such
> machines right now?

Is it?  http://www.mozilla.org/projects/firefox/roadmap.html says "Q1 2007?", but then the same page also says "2  	 	late Q2/early Q3 2006  	Final Release".  Do you expect me to believe that page?

> > FC5 is not such a huge requirement. 
> 
> Ah.  You're one of those people who're so common in the Linux distro world who
> assumes everyone upgrades their OS as soon as the next version comes out. 

I'm not.  I'm just a developer.  If you expect someone like me to fetch and compile the developmental Firefox versions (which takes a one or two gigs) to test and optimize it, why can't Firefox developers install a three tiny libraries that are glib, cairo, and pango?


> Sorry, but that's just not the case.  There's no way we can ship in 6 months
> requiring FC5, imho.

You can ship with internal copies of glib and pango and gtk+.

> > This means, the Xft measurements don't take into account things like
> > ligatures
> 
> For what it's worth, last time I profiled the pango measurement path (just the
> measurement) was about 120 times slower than the Xft path.  This is on FC4
> here, with PangoXft.

That's such a bold statement.  Do you have any benchmarks?  I'm very interested in checking them out.


> Pageload performace was 400% slower with pango than
> without.  I clearly can't test PangoCairo because I'd need to upgrade my OS
> first.

If you, a developer, need to upgrade your OS first to get a non-ancient Pango, why do you think all your users install Firefox on ancient operating systems?


> > So, what should I do?  Open a bug "please use pangocairo"?
> 
> Get pangocairo backported into pango versions that are actually shipped to an
> overwhelming majority of Linux users so we don't have to give people the "to
> use our browser you MUST update your operating system" crap that will make them
> simply not use our browser?

Suppose that I backported pangocairo to work with pango-1.8.  How are your users going to get that pangocairo library?  Included in firefox?  Then why not include pango too?
Comment 39 Stuart Parmenter 2006-09-12 10:33:20 PDT
(In reply to comment #38)
> 
> Suppose that I backported pangocairo to work with pango-1.8.  How are your
> users going to get that pangocairo library?

Ideally, the distros would ship updates to their users.
Comment 40 Behdad Esfahbod 2006-09-12 10:38:06 PDT
(In reply to comment #39)
> (In reply to comment #38)
> > 
> > Suppose that I backported pangocairo to work with pango-1.8.  How are your
> > users going to get that pangocairo library?
> 
> Ideally, the distros would ship updates to their users.

Distros just don't do that.  Not to include a new library.
Comment 41 Carl Worth 2006-09-12 10:41:45 PDT
Am I understanding the approach correctly:

"Mozilla must bundle cairo because not all users of interest have it already."

"Mozilla must not use pangocairo because not all user of interest have it already."

If that's an accurate statement of the current thinking, then what's the distinction between cairo and pangocairo here?

As for performance bugs, I'm quite sure we all just want to get them fixed. Let's continue working together on that.

-Carl
Comment 42 Stuart Parmenter 2006-09-12 10:46:06 PDT
(In reply to comment #41)
> Am I understanding the approach correctly:
> 
> "Mozilla must bundle cairo because not all users of interest have it already."
> 
> "Mozilla must not use pangocairo because not all user of interest have it
> already."
> 
> If that's an accurate statement of the current thinking, then what's the
> distinction between cairo and pangocairo here?
> 

The distinction is that we _have_ to ship with cairo.  We don't have to ship with pangocairo.  In fact, I'm not sure what it buys us except for perhaps slightly easier to deal with and more headaches.

Again, pangocairo nonsense talk should be moved to another bug.
Comment 43 Carl Worth 2006-09-12 11:01:10 PDT
(In reply to comment #42)
> The distinction is that we _have_ to ship with cairo.  We don't have to ship
> with pangocairo.  In fact, I'm not sure what it buys us except for perhaps
> slightly easier to deal with and more headaches.

I would think it would be an obvious win to work together with pango upstream  for doing pango/cairo integration as efficiently as possible.

I think that that's all that Behdad meant when saying he "can't and won't spend time making it faster". If mozilla does its own custom integration of pango with cairo, then mozilla owns it, and there's obviously little that pango maintainers can do about that.

-Carl
Comment 44 Boris Zbarsky [:bz] 2006-09-12 11:24:58 PDT
(In reply to comment #38)
> I appreciate it if you point me to benchmarks such that I can reproduce the
> 500%.

Sure.  Take trunk builds from before the patch for this bug landed, both cairo and non-cairo.  Load the testcases in bug 64858.  You might need to edit out the JProfStartProfiling/JProfStopProfiling calls unless you're actually profiling with jprof.  Look at the reported times.  I get about 4x better performance with the non-cairo builds, with the vast majority of the time difference being text measurement performance.

As Robert said, a lot of this is impedance mismatch between what pango wants to do and what current trunk is trying to get it to do.  But that doesn't help the fact that right now trunk was unusable for daily browsing.  The more text, the less usable.  Once we rearchitect our layout code around what pango and such want, we'll have to remeasure, of course; at that point we should revisit the changes made in this bug.

>  The Pango-based Firefox shipped in Fedora is definitely not 500% slower.

I can't speak to what changes Fedora made and what they use pango for.  Our real concern is trunk going forward.

> Is it?  http://www.mozilla.org/projects/firefox/roadmap.html says "Q1 2007?",

That would be in like 3 months.  I'm giving it some slack here.  ;)

> Release".  Do you expect me to believe that page?

Not at all.  I don't believe it myself.   ;)

> I'm not.  I'm just a developer.  If you expect someone like me to fetch and
> compile the developmental Firefox versions

When have I ever asked you to do that?

> (which takes a one or two gigs)

Non-debug builds don't one or two gigs...  And those are all you need for optimization (profiling) purposes.  But that's neither here nor there.

> why can't Firefox developers install a three tiny
> libraries that are glib, cairo, and pango?

In brief, because the package systems on Linux suck.  For example, I can't install an updated pango without updating hundreds of other packages, due to the RPM dependency mess.  Worse yet, users would have to have the updated pango too, which means either the distros need to ship it or we need to.

> You can ship with internal copies of glib and pango and gtk+.

We could, but that's something we'd like to avoid.  Given that all the paths forward seem to suck, we're going to got with what we see as the lesser evils.

> That's such a bold statement.  Do you have any benchmarks?  I'm very
> interested in checking them out.

See beginning of this comment.

> If you, a developer, need to upgrade your OS first to get a non-ancient
> Pango,

I'm not a pango developer.  I'm a pango user.  I'm developing on my own machine, part-time, and the machine is used for a whole lot of things other than Firefox development.

> why do you think all your users install Firefox on ancient operating systems?

First of all, a year old is not exactly ancient.  Second of all, when did I say "all"?  I'm not saying all Linux users use FC3 or FC4 or equivalent.  I'm saying not all use FC5.  Please don't put made-up words in my mouth.

> Suppose that I backported pangocairo to work with pango-1.8.  How are your
> users going to get that pangocairo library? 

Like I said, "that are actually shipped to an overwhelming majority of Linux users".  You clearly can't control that, but if there's an option to do it we could work with distros to ship such updates.  Right now, all distros can tell us is "can't do that".

Carl, I don't think anyone's asking Behdad to spend significant time optimizing parts of pango that he considers deprecated.  But then I'm not sure why he's being so defensive about us trying to avoid those parts of pango....
Comment 45 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2006-09-12 12:17:48 PDT
(In reply to comment #38)
> You can ship with internal copies of glib and pango and gtk+.

There may be legal obstacles to doing that.  glib, pango, and gtk+ are not licensed under the MPL, as far as I am aware.
Comment 46 Behdad Esfahbod 2006-09-12 14:31:52 PDT
Ok, going to shut up here.  Just:

  - I've been defensive, because I was attacked upon.  Doesn't matter though.

  - Unless you use Pango the way it is supposed to be used, please stop saying "pango is slow".
Comment 47 Boris Zbarsky [:bz] 2006-09-12 14:47:28 PDT
>   - Unless you use Pango the way it is supposed to be used,

The problem is that the way it's "supposed" to be used isn't very compatible with rendering web content.  Consider, for example, trying to render the following simple HTML + CSS with pango:

  <div> This is 
    <span style="letter-spacing: 0.5em">so<span
       style="font-size: large">me</span> text</span>
    that has simple styling.
  </div>

(this is not particularly contrived markup; in the "real" world you get stuff like this all the time, esp. with font changes).

Pango seems to assume that what you're rendering are large paragraphs of text, all with the same styling.  For a typical website, that's not what's being rendered....
Comment 48 Behdad Esfahbod 2006-09-12 14:59:34 PDT
(In reply to comment #47)
> >   - Unless you use Pango the way it is supposed to be used,
> 
> The problem is that the way it's "supposed" to be used isn't very compatible
> with rendering web content.  Consider, for example, trying to render the
> following simple HTML + CSS with pango:
> 
>   <div> This is 
>     <span style="letter-spacing: 0.5em">so<span
>        style="font-size: large">me</span> text</span>
>     that has simple styling.
>   </div>
> 
> (this is not particularly contrived markup; in the "real" world you get stuff
> like this all the time, esp. with font changes).
> 
> Pango seems to assume that what you're rendering are large paragraphs of text,
> all with the same styling.  For a typical website, that's not what's being
> rendered....

Not at all.  First, Pango has two levels of API.  1) PangoLayout, 2) bare pango_itemize, pango_shape, pango_break.   And both of these support all the styling in your example.  Slightly different perhaps, but quite possible to match.  Pango supports that using attributes.  It even has a convenience markup language.  In Pango markup speak, your example can be rendered using:

  pango-view --markup --text 'This is <span letter_spacing="6144">so<span size="large">me</span> text</span> that has simple styling.'

The letter-spacing attribute in Pango takes an absolute size, but that's something that can be fixed if need be.
Comment 49 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2006-09-12 15:05:14 PDT
(In reply to comment #46)
>   - Unless you use Pango the way it is supposed to be used, please stop saying
> "pango is slow".

Are you saying that pango isn't supposed to be used without pangocairo, or that pango isn't supposed to be used without holding on to pango objects, or something else?  We're planning to fix the latter, but even with that change some of the performance problems (e.g., font sorting) are probably still going to be a problem.
Comment 50 Boris Zbarsky [:bz] 2006-09-12 15:15:22 PDT
> And both of these support all the styling in your example.

In a way that is 100% compatible with the CSS specification?
Comment 51 David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) 2006-09-12 15:50:01 PDT
(In reply to comment #50)
> > And both of these support all the styling in your example.
> 
> In a way that is 100% compatible with the CSS specification?

I'd be hesitant to leave such support to OS libraries in any case.  Having the same bugs across platforms makes us an easier target for Web developers, and being able to advance faster than operating systems gives us the ability to move the Web forward faster than the OS-upgrade cycle (which is probably significantly slower on operating systems that people pay for).
Comment 52 Behdad Esfahbod 2006-09-12 16:20:09 PDT
(In reply to comment #50)
> > And both of these support all the styling in your example.
> 
> In a way that is 100% compatible with the CSS specification?

I don't know.  But you can set the attributes whatever way you want, to match whatever you want.
Comment 53 Behdad Esfahbod 2006-09-12 16:27:55 PDT
Created attachment 238095 [details]
pangocairo-lite

Attaching a ripped off pangocairo.  It's dead easy to get it working with pango 1.8.  The only new API it uses that is not in 1.8 is pango_font_get_font_map, but if you can track what it's used for, it's very easy to work around it.
Comment 54 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-09-12 18:59:32 PDT
We'd be lucky if Pango's implementation of letter-spacing matches CSS's in terms of exactly where spacing is allowed and where it's placed (not really dictated in CSS yet, but will be). Ditto for word spacing, justification spacing, tab expansion. There's also non-text elements in lines, relative positioning, CSS borders, hyphenation ... Even if we have lots of luck, we can't expect Pango and CSS to always match for all time, and designing for that would be foolish. Even more so when you bring other platforms into the equation.

Note You need to log in before you can comment on or make changes to this bug.