1584000 - Migrate glyph to character association code from libThebes to graphite for sandboxed libGraphite performance

Status: RESOLVED FIXED

(Core :: Graphics: Text, enhancement, P3)

Description

[Shravan Narayan]

This bug tracks the migration of the glyph to character association code from libThebes to libGraphite.

This migration will not have an impact on performance of stock Firefox.

However, with RLBox, we are looking to use libGraphite in a sandboxed manner. The overhead of sandboxing is proportional to

1. Overhead of running sandboxed libGraphite code
2. The number of times the application transitions to/from the sandbox (i.e. the number of invocations of libGraphite APIs).

The glyph to character association code in libThebes makes several calls to Graphite in a tight loop.

Thus migrating to this code to the graphite component i.e. including this code in the sandbox reduces the number of function invocations (however, at the cost of more sandboxing overhead). In our test, we see the when running a sandboxed libGraphite, this results in a net speedup of 15% of the sandboxed graphite. Thus we are including this code in the libGraphite component.

Comment 1

[Shravan Narayan]

Attachment: Bug 1584000 - Migrate glyph to character association code from libThebes to graphite for sandboxed libGraphite performance r=jfkthame,froydnj

Comment 2

[Shravan Narayan]

Attachment: Bug 1584000 - Migrate glyph to character association code from libThebes to graphite for sandboxed libGraphite performance r=jfkthame,froydnj

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1b90fe16b26a
Migrate glyph to character association code from libThebes to graphite for sandboxed libGraphite performance r=jfkthame,froydnj

Comment 4

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/1b90fe16b26a

Comment 5

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1b90fe16b26a

Comment 6

[Thaodan]

Could this be done in a way that graphite2 doesn't get changed or the code goes upstream?

Comment 7

[Shravan Narayan]

Sorry could you clarify your question or provide some additional context?
Fwiw, this isn't actually changing any existing code to graphite. We however include some extra functions as part of the compilation unit of graphite --- some code which was previously part of libThebes is now compiled with graphite.

This was done with the aim of improving the performance of sandboxed libGraphite. On platforms where graphite is not sandboxed, all this just amounts to a minor build change which is not visible (as all objects libGraphite, libThebes etc. are ultimately included in libxul object)

Comment 8

[Thaodan]

Yes I can: because this adds code to graphite it breaks the compilation when not using the bundled version of graphite.

Comment 9

[Shravan Narayan]

Oh, strange, we had tested both scenarios, so this should work. Could you please provide a link to the build failure if building on "try servers" or the compiler error output if its a local build error? Also if its a local build error, can you retry running mach bootstrap before building?

Comment 10

[Thaodan]

It is a local error.
See here:

mozilla-unified/gfx/thebes/gfxGraphiteShaper.cpp:13:10: schwerwiegender Fehler: graphite2/GraphiteExtra.h: Datei oder Verzeichnis nicht     gefunden
   13 | #include "graphite2/GraphiteExtra.h"

Comment 11

[Nathan Froyd [:froydnj]]

(In reply to Thaodan from comment #10)

It is a local error.
See here:

mozilla-unified/gfx/thebes/gfxGraphiteShaper.cpp:13:10: schwerwiegender Fehler: graphite2/GraphiteExtra.h: Datei oder Verzeichnis nicht     gefunden
   13 | #include "graphite2/GraphiteExtra.h"

Can you provide your mozconfig and the platform you're compiling on? I don't think we have an option for building with a system graphite.