1163488 - replace pref lang lookup arrays with preprocessor generated arrays

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[John Daggett (:jtd)]

There's code in gfxPlatform.cpp for mapping language strings used in all.js to a pref lang enum. There are various arrays that must be maintained in an associated order:

http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxPlatform.cpp#353
http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxPlatform.h#75

Karl thinks this is too brittle and that we should use lookups that rely on case statements and simply let the compiler optimize the lookup code.

https://bugzilla.mozilla.org/show_bug.cgi?id=1056479#c79

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Another option would be to use the C preprocessor (like nsCSSPropList.h, nsCSSPseudoClassList.h, nsCSSFontDescList.h, etc.).

Comment 2

[Nicholas Nethercote [inactive]]

> Karl thinks this is too brittle and that we should use lookups that rely on
> case statements and simply let the compiler optimize the lookup code.

Large case statements usually cause lots of mispredicted branches. A lookup table is usually better.

> Another option would be to use the C preprocessor (like nsCSSPropList.h,
> nsCSSPseudoClassList.h, nsCSSFontDescList.h, etc.).

+1 to this. It would allow you to keep the lookup table, while also guaranteeing the different parts of the code follow the same ordering.

Comment 3

[John Daggett (:jtd)]

Attachment: patch, generate pref font arrays from shared list

Consolidate the data arrays maintained for use with pref font handling into a shared list. Use preprocessor to populate arrays and maintain ordering between enum and lang atom/strings.

We can probably eliminate the string from the list but that requires a certain amount of refactoring within the pref font code, so that's a bigger job.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ca220c8e9883

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ca220c8e9883