Hi,
If you read the bug here I wrote :
>Note that harfbuzz still will likely copy and sanitise the table each
time
>we create a face object but fixing that needs a deeper understanding
>(perhaps too deep) of how we can re-use harfbuzz objects across
> calls to shape. Something to consider later
So I am aware that we are taking a per-call overhead of rebuilding this
info and today's fix
is a lower-cost / risk step towards reducing that.
I needed to take a closer look at the lifecycle and sharability of these
structures before
doing anything but don't have time right now. Eg, it isn't clear to me
if your code is thread safe.
Also I intend to remove the ICU code as obsolete and the caching code I
am using in
this fix was then going to look like dead code so I wanted to make sure
it was hooked up
into HB and serving a purpose before that removal.
Seems like you must have backported the HB code to JDK 8u ?
Or are you already shipping JDK 9 based OpenJDK ?
But in summary, yes, the general idea you propose is something that is
"on the list".
Probably some lines of what you have in that webrev can be adopted but I
don't
yet have a view on whether it should all be adopted directly.
However a similar end result is the goal ..
-phil.
On 08/18/2017 02:31 AM, Dmitry Batrak wrote:
Hello,
I was going to propose a related change a bit later, but thought it
makes sense to let you know now, as you're working on a similar thing.
In our OpenJDK-based runtime, we've optimized HarfBuzz invocations, by
creating hb_face_t object only once per Font2D instance.
This can make layout 2-3 times faster (more prominent for fonts with a
lot of layout rules, e.g. Fira Code).
We have this working in production for about a year now, without issues.
I've created a corresponding webrev here
http://cr.openjdk.java.net/~dbatrak/hb-optimization/webrev.00/
<http://cr.openjdk.java.net/%7Edbatrak/hb-optimization/webrev.00/>
It's generated 'as is', it should probably be reworked, if it's going
to be included in OpenJDK. In particular, caching of hb_face_t objects
should probably be encapsulated in SunLayoutEngine, and not done in
Font2D object directly.
I'm ready to work on the change further, following the formal
procedure (raising the ticket, submitting RFRs, etc). So, please let
me know
if you're interested, and whether you have any comments or
suggestions. In any case, I'll require someone sponsoring this change,
as I don't have a Committer status.
Best regards,
Dmitry Batrak
On Thu, Aug 17, 2017 at 12:07 AM, Phil Race <philip.r...@oracle.com
<mailto:philip.r...@oracle.com>> wrote:
Bug: https://bugs.openjdk.java.net/browse/JDK-8186317
<https://bugs.openjdk.java.net/browse/JDK-8186317>
Webrev: http://cr.openjdk.java.net/~prr/8186317/
<http://cr.openjdk.java.net/%7Eprr/8186317/>
In the ICU code path, we cache the layout tables in native memory
so that
when requested by ICU we can just hand the pointer, saving the
overhead
of copying the Java array for every call to layout.
We weren't doing this for harfbuzz and this webrev fixes that.
I noticed harfbuzz always retrieves the 'head' table so I decided
to add that to the list of
cached tables.
The overall result is something like 25-30% performance gain.
-phil.
