[OpenJDK 2D-Dev] Question regarding the integration of harfbuzz (JEP 258)

Wed Dec 9 18:49:14 UTC 2015

Hmm. Should JEP 254 really have " removed the handling of empty strings 
from jni_GetStringCritical()."
without tracking down uses and updating them ?

I didn't mean IBM already ship harfbuzz in their JDK but I thought
they were already using it in some other IBM products.

-phil.

On 12/9/15, 10:37 AM, Volker Simonis wrote:
> Hi Phil,
>
> thanks a lot for your answer. Please find my comments inline:
>
> On Wed, Dec 9, 2015 at 6:02 PM, Philip Race<philip.race at oracle.com>  wrote:
>> Hi Volker,
>>
>> Running with ICU should stop harfbuzz use completely so if
>> you are still seeing a crash, perhaps it is due to something else entirely.
> Yes, it was indeed something completely different. It was "8145015:
> jni_GetStringCritical asserts for empty strings"
> (https://bugs.openjdk.java.net/browse/JDK-8145015) which I've opened
> (and fixed :) today. The problem was that
> sun.awt.X11.XlibWrapper.XSetLocaleModifiers() was called with an empty
> string from sun.awt.X11.XToolkit.init() which crashed in the debug
> build for the ISO8859-1 locale:
>
> V  [libjvm.so+0xffffffff]  VMError::report_and_die(Thread*,const
> char*,int,const char*,const char*,char*)+0x68
> V  [libjvm.so+0xffffffff]  report_vm_error(const char*,int,const
> char*,const char*,...)+0x8c
> V  [libjvm.so+0xffffffff]  typeArrayOopDesc::char_at_addr(int) const+0x74
> V  [libjvm.so+0xffffffff]  jni_GetStringCritical+0x2d4
> C  [libjava.so+0xffffffff]  getString8859_1Chars+0x70
> C  [libjava.so+0xffffffff]  JNU_GetStringPlatformChars+0xc4
> C  [libawt_xawt.so+0xffffffff]
> Java_sun_awt_X11_XlibWrapper_XSetLocaleModifiers+0x4c
> j  sun.awt.X11.XlibWrapper.XSetLocaleModifiers(Ljava/lang/String;)Ljava/lang/String;+0
> j  sun.awt.X11.XToolkit.init()V+9
> j  sun.awt.X11.XToolkit.<init>()V+97
>
> During debugging I also found out that "ISO-8859-1", which is the name
> this locale has on Linux, isn't correctly detected as 'FAST_8859_1'
> encoding. That's probably why nobody saw this error before. And that's
> why it's always good to double check the code on AIX :)
>
> I'll open a bug for the encoding problem on Linux tomorrow...
>
>> Especially since a "simple AWT program" should not load layout anyway.
>> There was a fair amount of new and changed code pushed recently ...
>>
>> Yes, we need to run harfbuzz in multiple concurrent threads in the JDK.
>> A single threaded lock on harfbuzz would throttle text operations.
>> I suppose it is possible that we do not strictly need it as we currently
>> create a
>> new harfbuzz "instance" each time we go to invoke layout but that
>> is something that at least theoretically could change.
>>
>> The approach to synchronization is all from up-stream harfbuzz.
>> The harfbuzz mailing list might be the place to discuss that.
>> I'd expect it does work on AIX as a principal driver here is that
>> the IBM sponsored ICU project has deprecated its layout engine
>> and IBM/ICU are moving to harfbuzz so I'd be somewhat astonished
>> if they haven't yet tried it on AIX. Did you try just getting a copy
>> of the full hotspot library and running its configure on AIX to see if
>> harfbuzz can work out which options it wants there ? Obviously
>> I did not try that since I don't have AIX. Steven Loomis of IBM
>> who submitted the JEP should be able to help with a lot of this.
> That's strange, because I've looked at the up-stream harfbuzz project
> and they definitely don't have the atomic instructions for AIX.
>
> So either IBM completely dropped AIX or they are only using it from a
> single thread or they haven't contributed their changes back to the
> OpenJDK - which would be sad :( So it would be definitely interesting
> to hear the actual internals from Steven :)
>
> That said, I've now implemented the three atomics myself by using the
> same compiler intrinsics like in the hotspot. I'll post a webrev
> tomorrow...
>
> Regards,
> Volker
>
>> -phil.
>>
>>
>> On 12/8/15, 10:50 AM, Volker Simonis wrote:
>>> Hi,
>>>
>>> the integration of harfbuzz broke our AIX build because there's no
>>> implementation available for the hb_atomic macros in
>>> hb-atomic-private.hh. I've fixed that locally but still get a crash
>>> when running a simple AWT program (even with
>>> -Dsun.font.layoutengine=icu).
>>>
>>> I'm curretnly debugging the problem, but I have some general questions:
>>>
>>> 1. Do we really need the multi-threaded features of harfbuzz in OpenJDK?
>>>
>>> 2. Why does hurbuzz require these synchronization macros? From a first
>>> glance it doesn't seem to use multiple threads by itself. Does it have
>>> some global data structures which it needs to protect if harfbuzz is
>>> called from various threads?
>>>
>>> 3. Is there a way to test the hurfbuzz engine on a new platform? I saw
>>> the new test TestLayoutVsICU.java but it is marked as "manual" and it
>>> seems to require external fonts (at least some of which seem to be
>>> freely available). Could you please provide at least a minimal
>>> description on how this test can be run?
>>>
>>> 4. Is there a way to test that the implementation of the hb_atomic
>>> macros in hb-atomic-private.hh is correct (i.e. a way to stress
>>> harfbuzz from multiple threads)? I think this is important as I
>>> couldn't find a good description of the requirements for the atomic
>>> macros and on weak memory model architectures we should really get
>>> this right. For example there's no description of which fences are
>>> required around the various atomic operations. Getting this right
>>> across all platforms in the HotSpot (see
>>> hotspot/src/os_cpu/../vm/atomic_os_cp.inline.hpp) is already hard
>>> enough so I'm a little concerned that we are now introducing similar
>>> primitives in the class library.
>>>
>>> Thank you and best regards,
>>> Volker
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20151209/54301120/attachment.html>