optimize gfxScriptRunItemizer::Next

RESOLVED FIXED in mozilla16

Status

()

Core
Layout: Text
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jfkthame, Assigned: jfkthame)

Tracking

(Blocks: 1 bug)

unspecified
mozilla16
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
Attachment #632024 - Flags: review?(smontagu)
(Assignee)

Updated

5 years ago
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?
(Assignee)

Comment 2

5 years ago
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+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0fd12ab9899a
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/0fd12ab9899a
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.