This looks good to me, if I understand correctly that we now create
the face on first use and cache it fin Java or as long as the Font2D is
alive.
And the JNIEnv is always found on
I think you are right that we don't need the caching of the tables since
now the face will do it. The unfortunate thing is that the removal of
this code is
well over half the changes and as such it greatly muddied finding the
changes
that make the performance difference so my review was harder and less
certain
because of that.
It could have been separated into a follow-on change I think so that it
would have
been easier to review the important change.
The pScaler parameter looks like it is unused these days which is why I
expect you removed it but also not directly relevant.
I have run builds + some tests - but I'm not in a position to run more tests
for a couple of weeks.
A (mild) stress test re-using the same font from multiple threads eachmaking
multiple calls into layout would be a good addition here. That should
help tell
us if there are any MT or re-entrancy problems. Can you provide such a
test ?
It will be a good thing to have automatically run to catch any problems
introduced later either on the Java side or by an update to harfbuzz.
-phil.
On 3/6/19, 5:45 PM, Dmitry Batrak wrote:
Hello,
I'd like to submit a patch for JDK-8220231. I'm not a Committer, so
I'll need someone to sponsor this change.
The proposed approach is used without known issues in OpenJDK-based
JetBrains Runtime for almost three years now. I've mentioned it
previously on this mailing list
(https://mail.openjdk.java.net/pipermail/2d-dev/2017-August/008497.html).
The change has been refactored as compared to the version mentioned
above (the logic has been moved to SunLayoutEngine), and includes the
removal of font tables caching (JDK-8186317). The latter, I believe,
becomes redundant with this fix.
Issue: https://bugs.openjdk.java.net/browse/JDK-8220231
Webrev: http://cr.openjdk.java.net/~dbatrak/8220231/webrev.00/
<http://cr.openjdk.java.net/%7Edbatrak/8220231/webrev.00/>
Best regards,
Dmitry Batrak
