Closed Bug 763703 Opened 9 years ago Closed 9 years ago

optimize gfxScriptRunItemizer::Next

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: jfkthame, Assigned: jfkthame)

References

Details

Attachments

(1 file)

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.
Attachment #632024 - Flags: review?(smontagu)
Assignee: nobody → jfkthame
(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?
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 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
Attachment #632024 - Flags: review?(smontagu) → review+
https://hg.mozilla.org/mozilla-central/rev/0fd12ab9899a
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.