Last Comment Bug 763703 - optimize gfxScriptRunItemizer::Next
: optimize gfxScriptRunItemizer::Next
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Layout: Text (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: Jonathan Kew (:jfkthame)
:
:
Mentors:
Depends on:
Blocks: 762710
  Show dependency treegraph
 
Reported: 2012-06-11 14:25 PDT by Jonathan Kew (:jfkthame)
Modified: 2012-06-21 04:05 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch, optimize Unicode property lookup and gfxScriptItemizer::Next (19.12 KB, patch)
2012-06-11 14:25 PDT, Jonathan Kew (:jfkthame)
smontagu: review+
Details | Diff | Splinter Review

Description Jonathan Kew (:jfkthame) 2012-06-11 14:25:50 PDT
Created attachment 632024 [details] [diff] [review]
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] 2012-06-11 18:22:19 PDT
(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) 2012-06-12 01:00:14 PDT
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 2012-06-20 08:33:09 PDT
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 5 Ed Morley [:emorley] 2012-06-21 04:05:07 PDT
https://hg.mozilla.org/mozilla-central/rev/0fd12ab9899a

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