763703 - optimize gfxScriptRunItemizer::Next

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Jonathan Kew [:jfkthame]]

Attachment: patch, optimize Unicode property lookup and gfxScriptItemizer::Next

We can improve text performance by optimizing gfxScriptRunItemizer::Next, which shows up surprisingly high in the profiles in bug 762710 and bug 763119. This is used to scan every piece of text (except on the 8-bit-only codepath) and split into script runs before we can do (script-specific) font matching and shaping.

A significant part of the problem is the binary search used to find paired characters (opening and closing delimiters) in order to assign the same script to both characters of a matched pair. We can avoid this binary search by using character properties from nsUnicodeProperties instead.

In addition, the Unicode property lookups perform significantly better if we make the nsCharProps() functions return const references instead of actually copying the entire record, and add #pragma pack(1) before the nsCharProps structs so that nsCharProps2 is packed into 4 bytes (as originally intended - oops).

This patch makes gfxScriptItemizer::Next drop from about 15% of the total gfxFontGroup::InitTextRun time to around 5%, when loading the tinderbox log example from bug 762710.

Comment 1

[Nathan Froyd [:froydnj]]

(In reply to Jonathan Kew (:jfkthame) from comment #0)
> In addition, the Unicode property lookups perform significantly better if we
> make the nsCharProps() functions return const references instead of actually
> copying the entire record, and add #pragma pack(1) before the nsCharProps
> structs so that nsCharProps2 is packed into 4 bytes (as originally intended
> - oops).

Is this #pragma necessary on Windows only?  Did you test with just the #pragma and not the return type switching?

Comment 2

[Jonathan Kew [:jfkthame]]

The #pragma is needed for gcc 4.2 on OS X, at least (that's where I ran across the problem - currently, it turns out that sizeof(nsCharProps2) is 5.)

And yes, just adding the pragma was not as effective as changing the return type to a const reference.

Comment 3

[Simon Montagu :smontagu]

Comment on attachment 632024 [details] [diff] [review]
patch, optimize Unicode property lookup and gfxScriptItemizer::Next

Review of attachment 632024 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxScriptItemizer.cpp
@@ +66,2 @@
>  void
> +gfxScriptItemizer::push(PRUint32 closer, PRInt32 scriptCode)

Nit: here and throughout I don't like the name "closer" because I keep misreading it as "more close". Maybe "matching"?  Looks good apart from that

Comment 4

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0fd12ab9899a

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/0fd12ab9899a